docs(platform): durable-delivery ACL provisioning design#3
docs(platform): durable-delivery ACL provisioning design#3mattwilkinsonn wants to merge 5 commits into
Conversation
Design record for closing the cotal_acl_<space> provisioning gap: recommends bring-up-owned ACL provisioning (cotal up / provision-acl) plus spawn's provision-when-daemon-live completeness fix. Co-Authored-By: seal <noreply@sealedsecurity.com>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 12 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a design document describing durable-delivery ACL provisioning, recommending provisioning during ChangesDesign Document
Estimated code review effort: 1 (Trivial) | ~5 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Greptile SummaryAdds a design-only record for the durable-delivery ACL provisioning gap: mint and spawn never write the
Confidence Score: 5/5Design-only document with no implementation changes; safe to merge. The record is a documentation-only addition with no code changes. The approach is thoroughly reasoned, the per-task interfaces are precise, and the Global Constraints section explicitly handles the one intentional deviation from the AGENTS.md No fallbacks convention. The only finding is a minor factual inconsistency in the live-evidence paragraph (0→6 rows backfilled against a stated 7-agent population), which does not affect the design correctness or the implementer's work. No files require special attention; the single file is a documentation-only design record. Important Files Changed
Reviews (5): Last reviewed commit: "docs(platform): tighten Task 2 — durable..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@docs/designs/platform/durable-delivery-acl-provisioning.md`:
- Around line 94-99: Treat ACL provisioning as a required startup step, not a
log-and-continue task: in the `cotal up` flow after `postStart`, make the
idempotent ACL routine fail the bring-up when provisioning does not succeed, or
explicitly mark the process/status unhealthy instead of returning success. Keep
`cotal provision-acl` as the re-run entrypoint, but ensure the same underlying
routine used by `up` and the standalone command propagates failure so the broker
cannot appear healthy while ACLs are missing.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c530814d-f815-4f11-8678-f82ab5591be5
📒 Files selected for processing (1)
docs/designs/platform/durable-delivery-acl-provisioning.md
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 1 file
Heads up: you’re close to your included review allowance. Set a flex budget so reviews don’t pause.
Fix all with cubic | Re-trigger cubic
Drop the ephemeral "this session" anchoring (flagged by greptile P2 + cubic P3) so the durable design record is self-contained: the mint.ts zero-match search and the 0->8 self-heal evidence read as standing facts, not session-scoped observations. Co-Authored-By: seal <noreply@sealedsecurity.com>
Two freeze-prep clarifications from PR #3 review (mercator's supervisor rulings; substantive design forks stay open for Matt): - Soft-fail posture: mark the cotal-up auto-provision hook as the one deliberate, scoped exception to the 'No fallbacks' constraint (up orchestrates many agents; re-runnable; mirrors up.ts:284-285), with the standalone command staying hard-fail. Cross-referenced both sections so the spec isn't ambiguous (cubic P2, greptile P2). - 'Row is the whole gate' caveat: true only for the spawn/manager-provisioned population (the 7-agent evidence set already held dm/dlv durables); a mint+exec-omp agent has neither, so the provisioning step writes the full footprint (dm+dlv+acl). Corroborated by Codex on PR #4 and cubic's creds-only finding here; the DLV fix landed on PR #4 (f40ab37). Co-Authored-By: seal <noreply@sealedsecurity.com>
…aveat PR #3 re-review internal-consistency fixes (greptile 4/5 + cubic P2, both bot-only): - Task 2 step 3 said commitAcl-only, contradicting the scope caveat (a mint+exec-omp agent needs dm_<id> + dlv_<id> + the ACL row). Expand it to the full durable footprint (provisionDmInbox + provisionDlvInbox + commitAcl), all idempotent — matching what provisionAgent writes and what the DLV fix landed on PR #4 (f40ab37). Interfaces 'produces' line updated to match. - The fork's option (ii) claimed the standalone command covers a persona-file-less out-of-band agent 'via its creds file', but both paths enumerate listPersonas (agent-files only, no creds/ scan). Corrected to state it honestly as a known, deferred gap rather than false coverage. Co-Authored-By: seal <noreply@sealedsecurity.com>
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Heads up: you’re close to your included review allowance. Set a flex budget so reviews don’t pause.
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
… assertions Two review-accuracy fixes on the frozen contract (cubic on #3): - Step 3 said 'exactly what provisionAgent writes for a spawned agent', but a bare cotal spawn passes durableMembership:false and skips the ACL row (provision.ts:272). Clarify it's the durableMembership:true footprint, not the live-only path — so an implementer doesn't assume the row is already written. - Task 2 test cycle asserted only the ACL row (readAcl); a row-only impl would pass while still leaving dm_<id>/dlv_<id> absent → pumpDlv no-ops → still wake-blind. Add explicit dm/dlv durable-existence assertions (the checks that actually guard the incomplete-footprint bug the design identifies). Refs #3. Co-Authored-By: seal <noreply@sealedsecurity.com>
…rseable creds Two review findings on the reuse logic (cubic P1 + greptile P2 on #7): - Reuse now requires profile === "agent". The durable-ACL-orphan rationale is agent-specific (persona-refresh workflow + dm/dlv durables keyed to the id); observer/admin creds have neither, and silently preserving a privileged admin key across re-mints would extend its lifetime unexpectedly. Observer/admin now always rotate, as before. - A present-but-unparseable creds file (empty, truncated, not a user creds file) now fails loud with an actionable error naming --force, instead of letting identityFromCreds throw a raw parse exception. Silently rotating there would orphan the durable row the existing id may still own, so fresh-mint is not a safe fallback — the operator must opt in via --force or remove the stale file. Verified: agent re-mint keeps id; admin/observer re-mint rotate; corrupt creds at the out path errors naming --force (no raw crash, no silent rotate). Refs #3. Co-Authored-By: seal <noreply@sealedsecurity.com>
Layer 1 — packages/core/smoke/identity.smoke.ts (offline, 4 asserts): identityFromCreds
round-trips {id, seed} unchanged; agrees with idFromCreds (one-id-everywhere); rejects
spliced creds (seed vs foreign JWT subject) and a missing seed block.
Layer 2 — implementations/cli/smoke/mint-reuse.smoke.ts (hermetic, exercises the real
mint() against a tmp .cotal root, 8 checks): re-minting an agent reuses the SAME id;
--force rotates; a persona ACL change refreshes the baked sub.allow WITHOUT rotating the
id; first mint of a new name mints fresh (absent-creds path, no crash); observer + admin
re-mint rotate (reuse is agent-only); an unparseable existing creds file fails loud naming
--force (no silent rotate, no raw crash).
Red-green verified: reverting the reuse block to unconditional newIdentity() fails checks
1, 3, and 6 (two mints → two ids; the exact #3 cubic-P1 durable-orphan bug); the fix turns
them green. Registered both as smoke:identity + smoke:mint-reuse and wired into smoke:ci.
Refs #3.
Co-Authored-By: seal <noreply@sealedsecurity.com>
cubic P2 on #7: the corrupt-creds check only asserted the error message, so a future mint() refactor that wrote fresh creds *before* surfacing the parse error would still pass while silently rotating the id — the exact regression the test guards. Add a filesystem-state assertion: after the failed mint, the corrupt file must be byte-unchanged (still empty), proving no silent fresh-mint. Refs #3. Co-Authored-By: seal <noreply@sealedsecurity.com>
2 participants
Design record (design only, no implementation) for the durable-delivery ACL provisioning gap: the
cotal_acl_<space>read-ACL row is the keystone the delivery daemon re-authorizes durable @mention-wake deliveries against (absent ⇒ DEFER, never deliver), but neither of the launch paths agents actually use writes it —cotal mintis offline creds-only, andcotal spawnhard-codesdurableMembership: false. Live evidence: 0 rows across 7 connected agents; an out-of-bandcommitAclbackfill self-healed 0→8 durable memberships with no reconnect, proving the row is the whole gate.The record captures the 3-way fork (mint-online / bring-up-owned provisioning / mint
--commit-aclflag) and recommends (ii): one idempotent TS routine provisioning ACL rows for every persona-dir agent, auto-run at the tail ofcotal upand exposed as a re-runnablecotal provision-acl, keeping mint offline-pure — plus the completeness half: spawn switches to provision-when-daemon-live (delivery-lease probe) and its stale live-only rationale is rewritten. Plan carries global constraints (privileged writes only, row == mintedallowSubscribe, atomiccommitAclCAS, TS-only, red→green against a livenats-server) and per-task interfaces; five open questions are batched for the freeze gate.Co-Authored-By: seal noreply@sealedsecurity.com