feat(modkit-coord): PRD + DESIGN for distributed coordination primitives crate#9
Open
diffora wants to merge 2 commits into
Open
feat(modkit-coord): PRD + DESIGN for distributed coordination primitives crate#9diffora wants to merge 2 commits into
diffora wants to merge 2 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces Changesmodkit-coord Library Skeleton & Specification
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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.
Inline comments:
In `@libs/modkit-coord/Cargo.toml`:
- Line 2: The crate name in libs/modkit-coord/Cargo.toml is missing the
workspace prefix; change the name field from "modkit-coord" to "cf-modkit-coord"
so it matches the other modkit-* crates and can be referenced as package =
"cf-modkit-coord" from the root workspace; update any workspace dependency
entries (e.g., the root Cargo.toml entry for modkit-coord) to use package =
"cf-modkit-coord" if not already done.
- Around line 17-18: Add a [features] table to Cargo.toml that declares the
empty placeholder feature names required by DESIGN.md: declare lease = [],
semaphore = [], barrier = [], and counter = [] so consumers can pin features
before implementations land; keep the existing [lints] workspace = true intact.
In `@libs/modkit-coord/docs/DESIGN.md`:
- Around line 195-222: LeaseGuard is missing a ttl: Duration field required by
with_ack_in_tx and cadence recommendations; add ttl: Duration to the LeaseGuard
struct, populate it at LeaseManager::acquire (capture the acquire-time TTL value
you already pass/decide there), expose a public read accessor (e.g. ttl()) and
update any code paths that set locked_until (with_ack_in_tx, renew logic,
Drop/release tasks) to use guard.ttl when computing now() + ttl; also update the
DESIGN.md domain model table to list ttl under LeaseGuard and the value-objects
table so the spec matches the implementation.
In `@libs/modkit-coord/docs/PRD.md`:
- Around line 522-530: The PRD dependency table incorrectly lists `chrono` as
the TTL/time dependency while DESIGN.md §3.5 mandates the `time` crate; update
the PRD.md table entry to remove `chrono` and instead list `time` (mentioning
`time::OffsetDateTime` / `time::Duration` as the used types) so it matches the
design decision and removes ambiguity for implementers.
In `@libs/modkit-coord/docs/SHORT_DESCRIPTION.md`:
- Around line 92-104: The fenced pseudo-code block labelled "Worker A:" in
SHORT_DESCRIPTION.md triggers markdownlint MD040 because it lacks a language
tag; update that fence from ``` to ```text (or another appropriate tag) so the
block becomes a tagged code block (e.g., ```text) to silence MD040 and keep the
existing contents and indentation intact.
- Around line 121-138: The SHORT_DESCRIPTION example uses outdated guard-centric
calls; update the example to the DESIGN.md LeaseManager-centric API: replace
modkit_coord::install_migration(...) with install_lease_table(manager,
table_name), stop calling guard.spawn_renewal(...) and instead call
manager.spawn_renewal(&guard, cadence, token), replace guard.with_ack_in_tx(|tx|
...) with manager.with_ack_in_tx(&guard, &tx, F) (pass the guard and tx into the
manager method), and replace guard.release() with manager.release(guard) (which
consumes the guard); adjust the example code and parameter names accordingly so
all four call-sites (install, spawn_renewal, with_ack_in_tx, release) match the
symbols install_lease_table, LeaseManager::spawn_renewal,
LeaseManager::with_ack_in_tx, and LeaseManager::release defined in DESIGN.md.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: bca74586-2fc8-4d6f-aeba-43f6e12cd203
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
.cypilot/config/artifacts.tomlCargo.tomllibs/modkit-coord/Cargo.tomllibs/modkit-coord/README.mdlibs/modkit-coord/docs/DESIGN.mdlibs/modkit-coord/docs/PRD.mdlibs/modkit-coord/docs/SHORT_DESCRIPTION.mdlibs/modkit-coord/src/lib.rs
| @@ -0,0 +1,18 @@ | |||
| [package] | |||
| name = "modkit-coord" | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Missing cf- package name prefix — inconsistent with all other modkit-* workspace crates.
Every other modkit-* crate in this workspace declares name = "cf-modkit-<name>" (e.g. cf-modkit-db, cf-modkit-auth), and the root Cargo.toml references them via modkit-db = { package = "cf-modkit-db", ... }. With name = "modkit-coord" here, the future workspace dependency entry will need package = "cf-modkit-coord" but won't find a match — this requires a crate rename before any consumer PR can land, and renaming after consumers take a direct dependency is a breaking change.
🔧 Proposed fix
-name = "modkit-coord"
+name = "cf-modkit-coord"And the corresponding future workspace dep entry in root Cargo.toml:
modkit-coord = { package = "cf-modkit-coord", version = "0.1.0", path = "libs/modkit-coord" }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| name = "modkit-coord" | |
| name = "cf-modkit-coord" |
🤖 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 `@libs/modkit-coord/Cargo.toml` at line 2, The crate name in
libs/modkit-coord/Cargo.toml is missing the workspace prefix; change the name
field from "modkit-coord" to "cf-modkit-coord" so it matches the other modkit-*
crates and can be referenced as package = "cf-modkit-coord" from the root
workspace; update any workspace dependency entries (e.g., the root Cargo.toml
entry for modkit-coord) to use package = "cf-modkit-coord" if not already done.
Comment on lines
+17
to
+18
| [lints] | ||
| workspace = true |
There was a problem hiding this comment.
Missing [features] block — contradicts the DESIGN.md specification for the skeleton.
DESIGN.md §4.1 explicitly states that v1 declares the lease, semaphore, barrier, and counter feature names in Cargo.toml as empty placeholders, so that consumers pinning features = ["lease"] before the implementation PR lands don't hit an "unknown feature" build error. Without this, the skeleton deviates from its own spec.
🔧 Proposed addition
[lints]
workspace = true
+
+[features]
+default = ["lease"]
+lease = []
+semaphore = []
+barrier = []
+counter = []📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| [lints] | |
| workspace = true | |
| [lints] | |
| workspace = true | |
| [features] | |
| default = ["lease"] | |
| lease = [] | |
| semaphore = [] | |
| barrier = [] | |
| counter = [] |
🤖 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 `@libs/modkit-coord/Cargo.toml` around lines 17 - 18, Add a [features] table to
Cargo.toml that declares the empty placeholder feature names required by
DESIGN.md: declare lease = [], semaphore = [], barrier = [], and counter = [] so
consumers can pin features before implementations land; keep the existing
[lints] workspace = true intact.
Comment on lines
+92
to
+104
| ``` | ||
| Worker A: | ||
| 1. acquire("outbox", ttl=30s) → got a LeaseGuard | ||
| 2. spawn renewal_task (renews every 10s) | ||
| 3. in a loop: | ||
| with_ack_in_tx(|tx| { | ||
| // reads its work | ||
| // writes the result | ||
| // library checks locked_by == uuid_A inside | ||
| }) | ||
| 4. cancel(renewal_task) | ||
| 5. release() → attempts = 0, lease is free | ||
| ``` |
There was a problem hiding this comment.
Missing language identifier on fenced code block (MD040).
The pseudo-code block on line 92 has no language tag, triggering the markdownlint MD040 warning flagged by static analysis. Use text or a similar tag.
🔧 Proposed fix
-```
+```text
Worker A:
1. acquire("outbox", ttl=30s) → got a LeaseGuard📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ``` | |
| Worker A: | |
| 1. acquire("outbox", ttl=30s) → got a LeaseGuard | |
| 2. spawn renewal_task (renews every 10s) | |
| 3. in a loop: | |
| with_ack_in_tx(|tx| { | |
| // reads its work | |
| // writes the result | |
| // library checks locked_by == uuid_A inside | |
| }) | |
| 4. cancel(renewal_task) | |
| 5. release() → attempts = 0, lease is free | |
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 92-92: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@libs/modkit-coord/docs/SHORT_DESCRIPTION.md` around lines 92 - 104, The
fenced pseudo-code block labelled "Worker A:" in SHORT_DESCRIPTION.md triggers
markdownlint MD040 because it lacks a language tag; update that fence from ```
to ```text (or another appropriate tag) so the block becomes a tagged code block
(e.g., ```text) to silence MD040 and keep the existing contents and indentation
intact.
Comment on lines
+121
to
+138
| ```rust | ||
| // in the module's migrator — once | ||
| modkit_coord::install_migration(&mut migrator, "outbox_coord_leases"); | ||
|
|
||
| // in the worker | ||
| let manager = LeaseManager::new(db, "outbox_coord_leases"); | ||
| let guard = manager.acquire("outbox", Duration::from_secs(30)).await?; | ||
| let _renewal = guard.spawn_renewal(Duration::from_secs(10)); | ||
|
|
||
| guard.with_ack_in_tx(|tx| async move { | ||
| // your work here — plain SeaORM/SQL | ||
| Ok(()) | ||
| }).await?; | ||
|
|
||
| guard.release().await?; // or just drop — best-effort release will run | ||
| ``` | ||
|
|
||
| **You write zero SQL related to the lease.** That's the whole point of the library — to replace ~150 lines of duplicated SQL in every module with a single typed API. |
There was a problem hiding this comment.
Four API call-sites differ from the LeaseManager-centric signatures defined in DESIGN.md.
The example code uses a guard-centric API that doesn't match the DESIGN.md §3.3 public surface:
| SHORT_DESCRIPTION | DESIGN.md |
|---|---|
modkit_coord::install_migration(…) |
install_lease_table(manager, table_name) |
guard.spawn_renewal(…) |
manager.spawn_renewal(&guard, cadence, token) |
guard.with_ack_in_tx(|tx| …) |
manager.with_ack_in_tx(&guard, &tx, F) |
guard.release() |
manager.release(guard) (consumes guard) |
These inconsistencies will mislead implementers in the follow-up PR.
🔧 Proposed fix to align with DESIGN.md
-modkit_coord::install_migration(&mut migrator, "outbox_coord_leases");
+modkit_coord::install_lease_table(&mut migrator, "outbox_coord_leases").await?;
// in the worker
let manager = LeaseManager::new(db, "outbox_coord_leases");
let guard = manager.acquire("outbox", Duration::from_secs(30)).await?;
-let _renewal = guard.spawn_renewal(Duration::from_secs(10));
+let cancel = CancellationToken::new();
+let _renewal = manager.spawn_renewal(&guard, Duration::from_secs(10), cancel.clone());
-guard.with_ack_in_tx(|tx| async move {
- // your work here — plain SeaORM/SQL
- Ok(())
-}).await?;
+let tx = db.begin().await?;
+manager.with_ack_in_tx(&guard, &tx, |tx| async move {
+ // your work here — plain SeaORM/SQL
+ Ok(())
+}).await?;
+tx.commit().await?;
-guard.release().await?; // or just drop — best-effort release will run
+manager.release(guard).await?; // or just drop — best-effort release will run🤖 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 `@libs/modkit-coord/docs/SHORT_DESCRIPTION.md` around lines 121 - 138, The
SHORT_DESCRIPTION example uses outdated guard-centric calls; update the example
to the DESIGN.md LeaseManager-centric API: replace
modkit_coord::install_migration(...) with install_lease_table(manager,
table_name), stop calling guard.spawn_renewal(...) and instead call
manager.spawn_renewal(&guard, cadence, token), replace guard.with_ack_in_tx(|tx|
...) with manager.with_ack_in_tx(&guard, &tx, F) (pass the guard and tx into the
manager method), and replace guard.release() with manager.release(guard) (which
consumes the guard); adjust the example code and parameter names accordingly so
all four call-sites (install, spawn_renewal, with_ack_in_tx, release) match the
symbols install_lease_table, LeaseManager::spawn_renewal,
LeaseManager::with_ack_in_tx, and LeaseManager::release defined in DESIGN.md.
diffora
force-pushed
the
modkit-coord/01-prd-and-design
branch
from
May 7, 2026 13:17
cde6238 to
fdfd796
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@libs/modkit-coord/docs/SHORT_DESCRIPTION.md`:
- Line 86: The text uses the wrong method name `retry_release`; update this
occurrence to `release_with_retry` so it matches PRD.md and DESIGN.md and the
other usage in SHORT_DESCRIPTION.md (ensure the symbol `release_with_retry` is
used consistently in the sentence describing the behavior and any inline code
spans).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7bcb151b-0c11-44b5-ba7d-87893c549b18
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
.cypilot/config/artifacts.tomlCargo.tomllibs/modkit-coord/Cargo.tomllibs/modkit-coord/README.mdlibs/modkit-coord/docs/DESIGN.mdlibs/modkit-coord/docs/PRD.mdlibs/modkit-coord/docs/SHORT_DESCRIPTION.mdlibs/modkit-coord/src/lib.rs
✅ Files skipped from review due to trivial changes (3)
- libs/modkit-coord/src/lib.rs
- libs/modkit-coord/README.md
- Cargo.toml
diffora
force-pushed
the
modkit-coord/01-prd-and-design
branch
from
May 7, 2026 13:37
fdfd796 to
86064fe
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
libs/modkit-coord/docs/SHORT_DESCRIPTION.md (2)
86-86:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winWrong method name: use
release_with_retryinstead ofretry_release.Line 82 and the PRD both use
release_with_retryas the method name.✏️ Proposed fix
-`retry_release` is for scenarios like "I didn't crash, but processing didn't succeed — let the next one retry, and let's keep the failure-streak counter." +`release_with_retry` is for scenarios like "I didn't crash, but processing didn't succeed — let the next one retry, and let's keep the failure-streak counter."🤖 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 `@libs/modkit-coord/docs/SHORT_DESCRIPTION.md` at line 86, The doc uses the wrong method name `retry_release`; update the text to use the correct API name `release_with_retry` everywhere (replace `retry_release` with `release_with_retry` in the sentence and any other occurrences) so it matches the PRD and line 82; ensure references to the method in examples and descriptions (e.g., mentions of `retry_release`) are renamed to `release_with_retry`.
92-104:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing language identifier on fenced code block.
The pseudo-code block still lacks a language tag, triggering MD040. Add
textor similar.🔧 Proposed fix
-``` +```text Worker A: 1. acquire("outbox", ttl=30s) → got a LeaseGuard🤖 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 `@libs/modkit-coord/docs/SHORT_DESCRIPTION.md` around lines 92 - 104, The fenced pseudo-code block labeled "Worker A" is missing a language identifier and triggers MD040; update the opening fence from ``` to ```text (or another plain tag) so the block becomes ```text ... ``` ensuring the closing triple backticks remain, which will satisfy the linter and keep the "Worker A:" pseudo-code intact.
🤖 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.
Inline comments:
In `@libs/modkit-coord/docs/SHORT_DESCRIPTION.md`:
- Around line 13-20: SHORT_DESCRIPTION.md's table omits the library-owned
worker_label column; update the table in SHORT_DESCRIPTION.md to include a fifth
row for `worker_label` (or add a clear footnote) indicating it's written by the
library for diagnostics, matching PRD references to `worker_label` and the
acquire behavior; ensure the table header/formatting remains consistent and that
the description text references PRD.md for full schema details.
---
Duplicate comments:
In `@libs/modkit-coord/docs/SHORT_DESCRIPTION.md`:
- Line 86: The doc uses the wrong method name `retry_release`; update the text
to use the correct API name `release_with_retry` everywhere (replace
`retry_release` with `release_with_retry` in the sentence and any other
occurrences) so it matches the PRD and line 82; ensure references to the method
in examples and descriptions (e.g., mentions of `retry_release`) are renamed to
`release_with_retry`.
- Around line 92-104: The fenced pseudo-code block labeled "Worker A" is missing
a language identifier and triggers MD040; update the opening fence from ``` to
```text (or another plain tag) so the block becomes ```text ... ``` ensuring the
closing triple backticks remain, which will satisfy the linter and keep the
"Worker A:" pseudo-code intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7ddb5b49-b774-4233-b8d8-dc10b332dfcd
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
.cypilot/config/artifacts.tomlCargo.tomllibs/modkit-coord/Cargo.tomllibs/modkit-coord/README.mdlibs/modkit-coord/docs/DESIGN.mdlibs/modkit-coord/docs/PRD.mdlibs/modkit-coord/docs/SHORT_DESCRIPTION.mdlibs/modkit-coord/src/lib.rs
✅ Files skipped from review due to trivial changes (5)
- libs/modkit-coord/src/lib.rs
- libs/modkit-coord/README.md
- Cargo.toml
- libs/modkit-coord/Cargo.toml
- libs/modkit-coord/docs/DESIGN.md
Comment on lines
+13
to
+20
| In a DB table (let's call it `outbox_coord_leases`) we store 4 columns: | ||
|
|
||
| | Column | Meaning | | ||
| |---|---| | ||
| | `key` | Job identifier (e.g. `"outbox"` or `"partition-7"`) | | ||
| | `locked_by` | UUID of the worker currently holding the lease | | ||
| | `locked_until` | Until what time the lease is considered alive | | ||
| | `attempts` | Counter of attempts (for crash diagnostics) | |
There was a problem hiding this comment.
Column count mismatch with PRD: SHORT_DESCRIPTION lists 4 columns but schema has 5.
This document lists 4 columns (key, locked_by, locked_until, attempts) but PRD.md consistently documents 5 library-owned columns, adding worker_label (see PRD lines 97, 265, 382). The PRD explicitly justifies worker_label as library-owned because the library writes to it during acquire.
While this SHORT_DESCRIPTION is informal, omitting a library-owned column could mislead readers about the actual table schema.
📝 Suggested addition
Consider adding a note or fifth row:
| Column | Meaning |
|---|---|
| `key` | Job identifier (e.g. `"outbox"` or `"partition-7"`) |
| `locked_by` | UUID of the worker currently holding the lease |
| `locked_until` | Until what time the lease is considered alive |
| `attempts` | Counter of attempts (for crash diagnostics) |
+| `worker_label` | Optional human-readable diagnostic label (e.g. replica ID) |Or add a footnote: "The actual schema includes a 5th column worker_label for diagnostics; see PRD for details."
🤖 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 `@libs/modkit-coord/docs/SHORT_DESCRIPTION.md` around lines 13 - 20,
SHORT_DESCRIPTION.md's table omits the library-owned worker_label column; update
the table in SHORT_DESCRIPTION.md to include a fifth row for `worker_label` (or
add a clear footnote) indicating it's written by the library for diagnostics,
matching PRD references to `worker_label` and the acquire behavior; ensure the
table header/formatting remains consistent and that the description text
references PRD.md for full schema details.
…mitives crate Introduces libs/modkit-coord crate skeleton with PRD, DESIGN, and an explainer doc for a distributed-lease primitive — v1's first member of a multi-primitive home (semaphore/barrier/counter gated behind opt-in cargo features in future versions). Replaces the per-module lease implementations duplicated in modkit-db::outbox and account-management/integrity_check_runs with a single typed Rust API (LeaseManager + RAII LeaseGuard) backed by raw dialect-aware SQL across PostgreSQL, SQLite, and MySQL. The four library-owned columns (key, locked_by, locked_until, attempts) form a fixed contract; consumers MAY co-locate their own state columns (processed_seq, last_error, etc.) in the same coordination table or use a separate table. Drop always performs clean release; consumers needing failure-streak preservation across panics call release_with_retry explicitly. The lease-guarded transactional ack (with_ack_in_tx) validates ownership inside the consumer's transaction so a lost-lease holder atomically rolls back its work writes. Both PRD and DESIGN pass cpt validate + validate-toc cleanly. Crate skeleton compiles (cargo check -p modkit-coord). Implementation lands in a follow-up PR per the standard cypilot SDLC flow. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
diffora
force-pushed
the
modkit-coord/01-prd-and-design
branch
from
May 7, 2026 13:54
86064fe to
9a908f2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
libs/modkit-coord/docs/SHORT_DESCRIPTION.md (4)
86-86:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winWrong method name:
retry_release→release_with_retry.PRD §5.1 (
cpt-modkit-coord-fr-release-with-retry) and line 82 of this same file use the correct namerelease_with_retry; line 86 uses the inconsistent alias.🤖 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 `@libs/modkit-coord/docs/SHORT_DESCRIPTION.md` at line 86, The docs use the wrong method name alias; replace the inconsistent `retry_release` occurrence with the canonical `release_with_retry` so the documentation matches PRD §5.1 and the rest of the file; update any stray references or examples in the same document that use `retry_release` to `release_with_retry` to ensure consistency with the `cpt-modkit-coord-fr-release-with-retry` spec.
121-138:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winConsumer usage sketch uses guard-centric calls that diverge from the
LeaseManager-centric API in DESIGN.md §3.3.Four call-sites are inconsistent:
This file DESIGN.md modkit_coord::install_migration(…)install_lease_table(manager, table_name)guard.spawn_renewal(…)manager.spawn_renewal(&guard, cadence, token)guard.with_ack_in_tx(|tx| …)manager.with_ack_in_tx(&guard, &tx, F)guard.release().await?manager.release(guard)(consumes guard)These inconsistencies will mislead the implementation PR author.
🤖 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 `@libs/modkit-coord/docs/SHORT_DESCRIPTION.md` around lines 121 - 138, The sample usage in SHORT_DESCRIPTION.md diverges from the API described in DESIGN.md; update the call sites to use the LeaseManager-centric API: replace modkit_coord::install_migration(&mut migrator, "outbox_coord_leases") with install_lease_table(manager, table_name) (or the exact manager-based helper), change let _renewal = guard.spawn_renewal(Duration::from_secs(10)) to manager.spawn_renewal(&guard, cadence, token), change guard.with_ack_in_tx(|tx| …) to manager.with_ack_in_tx(&guard, &tx, F) (passing the guard and tx into the manager helper), and replace guard.release().await? with manager.release(guard) so the examples match DESIGN.md’s function names and signatures.
13-20:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing
worker_label— table shows 4 columns but the library owns 5.PRD §1.5 (Glossary), §5.4 (
cpt-modkit-coord-fr-migration-helper), and §7.2 (cpt-modkit-coord-contract-table-schema) all document five library-owned columns, withworker_labelexplicitly justified as library-owned because the library writes to it during acquire.🤖 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 `@libs/modkit-coord/docs/SHORT_DESCRIPTION.md` around lines 13 - 20, The table in SHORT_DESCRIPTION.md for outbox_coord_leases omits the library-owned worker_label column; update the table to include a fifth row for `worker_label` describing it as the human-readable worker identifier the library writes during acquire (used for diagnostics/observability), ensuring the docs match the PRD and the library behavior when acquiring leases.
92-92:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMD040: fenced code block has no language tag.
markdownlint-cli2 flags this block. Add
text(or similar) to silence the warning.🤖 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 `@libs/modkit-coord/docs/SHORT_DESCRIPTION.md` at line 92, The fenced code block in the markdown lacks a language tag causing MD040; update the trailing or opening triple-backticks in SHORT_DESCRIPTION.md to include a language specifier (e.g., add "text" after the opening ``` or another appropriate language) so the block becomes ```text and the markdownlint warning is silenced.
🤖 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.
Inline comments:
In `@libs/modkit-coord/docs/PRD.md`:
- Around line 233-238: The FR cpt-modkit-coord-fr-ack-in-tx currently omits TTL
semantics and conflicts with the Glossary and Acceptance Criteria; update the FR
to state that with_ack_in_tx reuses the LeaseGuard's captured acquire-time TTL
to extend locked_until (like renew) and document an explicit per-call TTL
override variant (e.g., with_ack_in_tx_with), also update the Glossary entry for
LeaseGuard to explicitly list renew and with_ack_in_tx as reusing the captured
TTL and modify the Acceptance Criteria (line referencing per-call TTL override)
to require *_with variants for both renew and with_ack_in_tx; reference
functions/operations: with_ack_in_tx, with_ack_in_tx_with, renew, LeaseGuard,
and cpt-modkit-coord-fr-ack-in-tx.
---
Duplicate comments:
In `@libs/modkit-coord/docs/SHORT_DESCRIPTION.md`:
- Line 86: The docs use the wrong method name alias; replace the inconsistent
`retry_release` occurrence with the canonical `release_with_retry` so the
documentation matches PRD §5.1 and the rest of the file; update any stray
references or examples in the same document that use `retry_release` to
`release_with_retry` to ensure consistency with the
`cpt-modkit-coord-fr-release-with-retry` spec.
- Around line 121-138: The sample usage in SHORT_DESCRIPTION.md diverges from
the API described in DESIGN.md; update the call sites to use the
LeaseManager-centric API: replace modkit_coord::install_migration(&mut migrator,
"outbox_coord_leases") with install_lease_table(manager, table_name) (or the
exact manager-based helper), change let _renewal =
guard.spawn_renewal(Duration::from_secs(10)) to manager.spawn_renewal(&guard,
cadence, token), change guard.with_ack_in_tx(|tx| …) to
manager.with_ack_in_tx(&guard, &tx, F) (passing the guard and tx into the
manager helper), and replace guard.release().await? with manager.release(guard)
so the examples match DESIGN.md’s function names and signatures.
- Around line 13-20: The table in SHORT_DESCRIPTION.md for outbox_coord_leases
omits the library-owned worker_label column; update the table to include a fifth
row for `worker_label` describing it as the human-readable worker identifier the
library writes during acquire (used for diagnostics/observability), ensuring the
docs match the PRD and the library behavior when acquiring leases.
- Line 92: The fenced code block in the markdown lacks a language tag causing
MD040; update the trailing or opening triple-backticks in SHORT_DESCRIPTION.md
to include a language specifier (e.g., add "text" after the opening ``` or
another appropriate language) so the block becomes ```text and the markdownlint
warning is silenced.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: efaab1bf-58aa-4984-b97f-7d55614eca52
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
.cypilot/config/artifacts.tomlCargo.tomllibs/modkit-coord/Cargo.tomllibs/modkit-coord/README.mdlibs/modkit-coord/docs/DESIGN.mdlibs/modkit-coord/docs/PRD.mdlibs/modkit-coord/docs/SHORT_DESCRIPTION.mdlibs/modkit-coord/src/lib.rs
✅ Files skipped from review due to trivial changes (5)
- Cargo.toml
- libs/modkit-coord/src/lib.rs
- libs/modkit-coord/README.md
- libs/modkit-coord/Cargo.toml
- libs/modkit-coord/docs/DESIGN.md
Comment on lines
+233
to
+238
| - [ ] `p1` - **ID**: `cpt-modkit-coord-fr-ack-in-tx` | ||
|
|
||
| The system **MUST** provide an operation that runs a caller-supplied work closure inside a consumer-owned database transaction and atomically validates lease ownership inside the same transaction before the transaction commits. When the lease has been lost to a contender (the in-tx ownership check finds `locked_by` no longer matches the holder's worker identity), the operation **MUST** return a distinct lease-lost error. The operation **MUST NOT** itself commit or roll back the transaction; the rollback path is realised structurally by the consumer's call shape — the consumer invokes the operation from inside `Db::transaction_ref(|tx| async { ... })` (or `transaction_ref_mapped`) and propagates the lease-lost error via `?` out of that closure, at which point `transaction_ref` observes the error and rolls back the underlying `DatabaseTransaction` it owns. The operation **MUST NOT** require the caller to write or inspect any lease-related SQL. The API documentation **MUST** spell out this consumer-owned rollback contract so a consumer that swallows the lease-lost error and returns `Ok` from the outer closure cannot accidentally trigger a commit-on-lost-lease. | ||
|
|
||
| - **Rationale**: Lease-guarded write atomicity is the second core correctness invariant: without it, a holder that lost the lease mid-cycle could still commit stale work concurrently with the new holder, defeating exclusive semantics. Pushing the ownership check into the same transaction as the work writes is the property that makes the lease useful for "exactly one consumer commits per cycle". The library cannot itself force a rollback because `modkit-db`'s `DbTx<'_>` is a borrow-only wrapper over `&DatabaseTransaction`; the actual `DatabaseTransaction` is owned by `Db::transaction_ref`, and only the closure-returns-`Err` path triggers the rollback. Anchoring the contract on `transaction_ref` (rather than telling consumers "drop the tx") ties correctness to a structural API boundary the consumer cannot trivially bypass. | ||
| - **Actors**: `cpt-modkit-coord-actor-consumer-worker`, `cpt-modkit-coord-actor-database` |
There was a problem hiding this comment.
cpt-modkit-coord-fr-ack-in-tx is missing TTL semantics that the Glossary and Acceptance Criteria require.
The FR body says with_ack_in_tx validates ownership inside a transaction — no TTL is mentioned. But two other sections contradict this:
- Glossary (line 94): "The acquire-time TTL is captured by the returned
LeaseGuardand reused by every subsequentrenewandwith_ack_in_txcall against that guard" - Acceptance Criteria (line 553): "An explicit per-call TTL override variant is available for both operations" (i.e. both
renewandwith_ack_in_tx).
This means the implementation PR needs to answer a question the FR leaves open: does with_ack_in_tx also extend locked_until (similar to renew) or not?
- If yes → the FR needs a TTL-extension clause (otherwise the implementer only builds an ownership check and the
*_withoverride variant forwith_ack_in_txis never designed). - If no → the Glossary and AC need to drop the
with_ack_in_txreferences to TTL, and AC line 553 must confine the*_withvariant torenewonly.
Either resolution prevents an ambiguous implementation, but as written the three sections are in direct contradiction.
🔧 Example fix if `with_ack_in_tx` does extend TTL
Add to the FR body (after the ownership-validation sentence):
The operation MUST NOT itself commit or roll back the transaction; the rollback path is realised
structurally by the consumer's call shape ...
+
+When the ownership check succeeds, the operation MUST also extend `locked_until` by the
+acquire-time TTL captured on the guard (or a per-call TTL override supplied via the
+`with_ack_in_tx_with` variant) so that a long-running transaction cannot expire the lease
+between the ownership check and the commit. An explicit-TTL variant
+(`*_with`) MUST be exposed for consumers that need a different window per call.🔧 Example fix if `with_ack_in_tx` does NOT extend TTL
-The acquire-time TTL is captured by the returned `LeaseGuard` and reused by every subsequent
-`renew` and `with_ack_in_tx` call against that guard ...
+The acquire-time TTL is captured by the returned `LeaseGuard` and reused by every subsequent
+`renew` call against that guard ...And in AC line 553:
-An acquire-time TTL is captured by the returned `LeaseGuard` and reused by default for `renew` /
-`with_ack_in_tx`; an explicit per-call TTL override variant is available for both operations
+An acquire-time TTL is captured by the returned `LeaseGuard` and reused by default for `renew`;
+an explicit per-call TTL override variant (`renew_with`) is available and exercised by a unit test📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - [ ] `p1` - **ID**: `cpt-modkit-coord-fr-ack-in-tx` | |
| The system **MUST** provide an operation that runs a caller-supplied work closure inside a consumer-owned database transaction and atomically validates lease ownership inside the same transaction before the transaction commits. When the lease has been lost to a contender (the in-tx ownership check finds `locked_by` no longer matches the holder's worker identity), the operation **MUST** return a distinct lease-lost error. The operation **MUST NOT** itself commit or roll back the transaction; the rollback path is realised structurally by the consumer's call shape — the consumer invokes the operation from inside `Db::transaction_ref(|tx| async { ... })` (or `transaction_ref_mapped`) and propagates the lease-lost error via `?` out of that closure, at which point `transaction_ref` observes the error and rolls back the underlying `DatabaseTransaction` it owns. The operation **MUST NOT** require the caller to write or inspect any lease-related SQL. The API documentation **MUST** spell out this consumer-owned rollback contract so a consumer that swallows the lease-lost error and returns `Ok` from the outer closure cannot accidentally trigger a commit-on-lost-lease. | |
| - **Rationale**: Lease-guarded write atomicity is the second core correctness invariant: without it, a holder that lost the lease mid-cycle could still commit stale work concurrently with the new holder, defeating exclusive semantics. Pushing the ownership check into the same transaction as the work writes is the property that makes the lease useful for "exactly one consumer commits per cycle". The library cannot itself force a rollback because `modkit-db`'s `DbTx<'_>` is a borrow-only wrapper over `&DatabaseTransaction`; the actual `DatabaseTransaction` is owned by `Db::transaction_ref`, and only the closure-returns-`Err` path triggers the rollback. Anchoring the contract on `transaction_ref` (rather than telling consumers "drop the tx") ties correctness to a structural API boundary the consumer cannot trivially bypass. | |
| - **Actors**: `cpt-modkit-coord-actor-consumer-worker`, `cpt-modkit-coord-actor-database` | |
| - [ ] `p1` - **ID**: `cpt-modkit-coord-fr-ack-in-tx` | |
| The system **MUST** provide an operation that runs a caller-supplied work closure inside a consumer-owned database transaction and atomically validates lease ownership inside the same transaction before the transaction commits. When the lease has been lost to a contender (the in-tx ownership check finds `locked_by` no longer matches the holder's worker identity), the operation **MUST** return a distinct lease-lost error. The operation **MUST NOT** itself commit or roll back the transaction; the rollback path is realised structurally by the consumer's call shape — the consumer invokes the operation from inside `Db::transaction_ref(|tx| async { ... })` (or `transaction_ref_mapped`) and propagates the lease-lost error via `?` out of that closure, at which point `transaction_ref` observes the error and rolls back the underlying `DatabaseTransaction` it owns. The operation **MUST NOT** require the caller to write or inspect any lease-related SQL. The API documentation **MUST** spell out this consumer-owned rollback contract so a consumer that swallows the lease-lost error and returns `Ok` from the outer closure cannot accidentally trigger a commit-on-lost-lease. | |
| When the ownership check succeeds, the operation MUST also extend `locked_until` by the acquire-time TTL captured on the guard (or a per-call TTL override supplied via the `with_ack_in_tx_with` variant) so that a long-running transaction cannot expire the lease between the ownership check and the commit. An explicit-TTL variant (`*_with`) MUST be exposed for consumers that need a different window per call. | |
| - **Rationale**: Lease-guarded write atomicity is the second core correctness invariant: without it, a holder that lost the lease mid-cycle could still commit stale work concurrently with the new holder, defeating exclusive semantics. Pushing the ownership check into the same transaction as the work writes is the property that makes the lease useful for "exactly one consumer commits per cycle". The library cannot itself force a rollback because `modkit-db`'s `DbTx<'_>` is a borrow-only wrapper over `&DatabaseTransaction`; the actual `DatabaseTransaction` is owned by `Db::transaction_ref`, and only the closure-returns-`Err` path triggers the rollback. Anchoring the contract on `transaction_ref` (rather than telling consumers "drop the tx") ties correctness to a structural API boundary the consumer cannot trivially bypass. | |
| - **Actors**: `cpt-modkit-coord-actor-consumer-worker`, `cpt-modkit-coord-actor-database` |
🤖 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 `@libs/modkit-coord/docs/PRD.md` around lines 233 - 238, The FR
cpt-modkit-coord-fr-ack-in-tx currently omits TTL semantics and conflicts with
the Glossary and Acceptance Criteria; update the FR to state that with_ack_in_tx
reuses the LeaseGuard's captured acquire-time TTL to extend locked_until (like
renew) and document an explicit per-call TTL override variant (e.g.,
with_ack_in_tx_with), also update the Glossary entry for LeaseGuard to
explicitly list renew and with_ack_in_tx as reusing the captured TTL and modify
the Acceptance Criteria (line referencing per-call TTL override) to require
*_with variants for both renew and with_ack_in_tx; reference
functions/operations: with_ack_in_tx, with_ack_in_tx_with, renew, LeaseGuard,
and cpt-modkit-coord-fr-ack-in-tx.
…er example Replace the abstract outbox-dispatcher framing with account-management's hierarchy integrity reconciler as the running example throughout. The reconciler is the first real consumer of modkit-coord and exhibits every property the library is designed for — long-running work (5-15 minutes on large hierarchies), repair-phase SQL writes that demand fence-token correctness under VM-suspend / GC-pause, and singleton-key semantics distinct from outbox's per-partition pattern. Each section is grounded in concrete numbers (LEASE_TTL=15min, RENEW_INTERVAL=5min, ~1M closure rows for 100k tenants at depth 10) and maps directly to the planned domain/integrity_check/service.rs flow: acquire -> spawn_renewal -> snapshot+classify (read-only) -> with_ack_in_tx(repair) -> release. Adds an explicit comparison block explaining why same-DB lease-guarded writes are impossible with Redis-locks or cluster's DistributedLockV1, addressing the question that was raised when comparing primitives. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
libs/modkit-coord/docs/SHORT_DESCRIPTION.md (1)
151-151:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a language tag to the fenced block at Line 151.
This block still triggers markdownlint MD040; please change the opening fence to something like
```text.🔧 Minimal fix
-``` +```text AM scheduler tick (every 30 minutes per hierarchy):🤖 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 `@libs/modkit-coord/docs/SHORT_DESCRIPTION.md` at line 151, The fenced code block that contains "AM scheduler tick (every 30 minutes per hierarchy):" is missing a language tag and triggers markdownlint MD040; change the opening fence from ``` to a tagged fence such as ```text so the block becomes a text-fenced code block (e.g., update the block surrounding that exact line in SHORT_DESCRIPTION.md).
🤖 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.
Duplicate comments:
In `@libs/modkit-coord/docs/SHORT_DESCRIPTION.md`:
- Line 151: The fenced code block that contains "AM scheduler tick (every 30
minutes per hierarchy):" is missing a language tag and triggers markdownlint
MD040; change the opening fence from ``` to a tagged fence such as ```text so
the block becomes a text-fenced code block (e.g., update the block surrounding
that exact line in SHORT_DESCRIPTION.md).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2c81ff90-e84a-4dc6-abcc-c41e6ae26841
📒 Files selected for processing (1)
libs/modkit-coord/docs/SHORT_DESCRIPTION.md
diffora
added a commit
that referenced
this pull request
May 13, 2026
…able test-only repo seam (#5 #9) * Introduce ConversionScope newtype + ConversionScopeKind discriminator (UrlBinding / SystemSweep). System-driven seams (ConversionService::expire_pending, soft_delete_resolved) now take &ConversionScope instead of raw &AccessScope; module.rs wires the cleanup loop with ConversionScope::system_sweep(). The discriminator is debug-asserted so a regression that wires a URL-bound scope into the reaper / retention path surfaces in debug/test before the actor_kind audit envelope drifts. The wrapped AccessScope keeps flowing through to the TenantRepo lookups and to the conversion repo so the InTenantSubtree (#1813) plumb is a single-site change later. * Rename ConversionRepo::transition_pending_to_approved → __transition_pending_to_approved_test_only with #[doc(hidden)] and a docstring spelling out why it's dangerous in production (bypasses type re-eval, self_managed flip, and closure barrier rewrite). Production callers MUST go through apply_conversion_approval. There are no production callers today — the rename is the grep-discoverable signal at every future call site that this path is test-only. All 450 unit tests + 10 SQLite integration tests + clippy-clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
libs/modkit-coordcrate skeleton with PRD, DESIGN, and an explainer doc for a distributed-lease primitive — v1's first member of a multi-primitive home (semaphore/barrier/countergated behind opt-in cargo features in future versions).modkit-db::outboxandaccount-management/integrity_check_runswith a single typed Rust API (LeaseManager+ RAIILeaseGuard) backed by raw dialect-aware SQL across PostgreSQL, SQLite, and MySQL.What's in this PR
libs/modkit-coord/docs/PRD.md(564 lines) — problem statement, FRs, NFRs, use cases, acceptance criteria; ~30 uniquecpt-modkit-coord-*IDslibs/modkit-coord/docs/DESIGN.md(778 lines) — architecture overview, principles, domain model, component design, API contracts (informal signatures, no code blocks per cypilot DESIGN rules), 4 mermaid sequence diagramslibs/modkit-coord/docs/SHORT_DESCRIPTION.md— informal explainer for engineers new to the primitivelibs/modkit-coord/Cargo.toml+src/lib.rs— minimal crate skeleton;cargo check -p modkit-coordis cleanCargo.toml.cypilot/config/artifacts.tomlregisters the new systemmodkit-libswith autodetect rule forlibs/$system/docs/Architectural decisions captured
modkit-coordis the platform's home for distributed coordination primitives; v1 ships onlylease. Future primitives land as additional opt-in cargo features within this crate, not as separate crates.Entitybinding; per-backend dialect SQL emitted from a private internal module.(key, locked_by, locked_until, attempts). Consumers MAY co-locate their own state columns (e.g. outbox'sprocessed_seq,last_error) in the same coordination table or use a separate table.release_with_retryexplicitly. Panic-payload introspection rejected as design choice.release()(resetsattempts) +release_with_retry()(preservesattempts).^[A-Za-z_][A-Za-z0-9_]{0,62}$atLeaseManagerconstruction time.Test plan
cpt --json validate --artifact libs/modkit-coord/docs/PRD.md→ PASS, 0 errors, 0 warningscpt --json validate-toc libs/modkit-coord/docs/PRD.md→ PASS, 0 errors, 0 warningscpt --json validate --artifact libs/modkit-coord/docs/DESIGN.md→ PASS, 0 errors, 0 warningscpt --json validate-toc libs/modkit-coord/docs/DESIGN.md→ PASS, 0 errors, 0 warningscargo check -p modkit-coordcleanOut of scope (follow-up PRs)
account-management/integrity_check_runsmigration to modkit-coord (closes Risk 2C from the AM PR feat(am): hierarchy integrity + auto-repair + bootstrap saga #8 residual-risks memorandum)modkit-db::outboxmigration to modkit-coord (eliminates ~150 LOC of dialect SQL duplication)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Chores