[codex] Archive NLnet proposal reference and harden Coop sync#19
Conversation
|
Warning Review limit reached
More reviews will be available in 7 minutes and 5 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?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 credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. 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, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (8)
📝 WalkthroughWalkthroughTwo independent functional areas are changed. The CRDT merge function ChangesCRDT Merge Hardening and Invite Handoff Retry
Grant Proposal Docs and Dependency Pin
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Oba-One
force-pushed
the
codex/code-root-cleanup-coop
branch
from
ae83262 to
6c17910
Compare
Oba-One
force-pushed
the
codex/code-root-cleanup-coop
branch
from
6c17910 to
b02941c
Compare
Oba-One
force-pushed
the
codex/code-root-cleanup-coop
branch
from
b02941c to
666c081
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/extension/src/background/handlers/__tests__/coop-handlers.test.ts (1)
799-858: 💤 Low valueConsider adding a test for exhausted retries.
The retry success path is well covered. For completeness, a test that mocks 5+ undefined responses to verify the fallback error message when all retries are exhausted would strengthen coverage of the failure path.
🤖 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 `@packages/extension/src/background/handlers/__tests__/coop-handlers.test.ts` around lines 799 - 858, Add a new test alongside the existing "retries external invite handoff while the offscreen sync runtime wakes" test that covers the failure path. Create a test that follows the same pattern with handleJoinCoop, but instead of mocking three sendMessage calls (with the third returning success), mock 5 or more consecutive undefined responses to simulate all retries being exhausted. Verify that the joinResult.ok is false and that the appropriate error message or fallback behavior is returned when all retry attempts are depleted.
🤖 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 `@packages/shared/src/modules/storage/__tests__/db-crud-content-sync.test.ts`:
- Around line 403-407: The test for mergeCoopStateUpdate with
invalidRemoteUpdate should assert that an error is thrown rather than expecting
a successful result. Refactor the test around the mergeCoopStateUpdate call to
use expect().rejects pattern to verify the promise rejection occurs, then keep
an assertion comparing the original state to prove it was not modified during
the failed merge operation. Remove the current success assertions on
result.profile.id and result.rituals, replacing them with error handling
verification.
In `@packages/shared/src/modules/storage/db-crud-content.ts`:
- Around line 243-257: The mergeCoopStateUpdate function is returning stale
state when an invalid remote update is detected (when parseResult.success is
false), which makes the invalid merge appear successful and violates
error-handling semantics. Remove the return path that returns
readCoopState(existingDoc) in the if (existing) block when parsing fails,
ensuring the error is always thrown via throw parseResult.error at the end
instead. The function should never return a successful result when
parseResult.success is false, even if existing data is available.
---
Nitpick comments:
In `@packages/extension/src/background/handlers/__tests__/coop-handlers.test.ts`:
- Around line 799-858: Add a new test alongside the existing "retries external
invite handoff while the offscreen sync runtime wakes" test that covers the
failure path. Create a test that follows the same pattern with handleJoinCoop,
but instead of mocking three sendMessage calls (with the third returning
success), mock 5 or more consecutive undefined responses to simulate all retries
being exhausted. Verify that the joinResult.ok is false and that the appropriate
error message or fallback behavior is returned when all retry attempts are
depleted.
🪄 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
Run ID: bca84281-b2b8-4442-b372-b36963b08c0a
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
docs/package.jsondocs/reference/index.mddocs/reference/nlnet-fediversity-coop-proposal-2026-05-31.mdpackages/extension/src/background/handler-registry.tspackages/extension/src/background/handlers/__tests__/coop-handlers.test.tspackages/extension/src/background/handlers/coop.tspackages/shared/src/modules/storage/__tests__/db-crud-content-sync.test.tspackages/shared/src/modules/storage/db-crud-content.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Unit Coverage
🧰 Additional context used
📓 Path-based instructions (5)
**/package.json
📄 CodeRabbit inference engine (CLAUDE.md)
**/package.json: Use internal workspace dependencies withworkspace:*(neverworkspace:^) in package.json
Pin exact versions for critical dependencies: viem, permissionless, react, dexie, yjs due to breaking changes between minors
Use ranges for dev dependencies: vitest^, typescript~,@types/*^in package.json
Files:
docs/package.json
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Prefer interface over type for defining object shapes in TypeScript
Use Dexie for structured data, Yjs for CRDT sync, y-webrtc for direct peer transport, and y-websocket for server-assisted document sync
Files:
packages/shared/src/modules/storage/db-crud-content.tspackages/extension/src/background/handlers/__tests__/coop-handlers.test.tspackages/extension/src/background/handler-registry.tspackages/shared/src/modules/storage/__tests__/db-crud-content-sync.test.tspackages/extension/src/background/handlers/coop.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Use barrel imports from@coop/sharedor@coop/shared/app, never import from deep source paths
All frontend environment variables must use theVITE_prefix for access viaimport.meta.env
Never swallow errors; always surface failures to the user
**/*.{ts,tsx,js,jsx}: Import from shared public surfaces only:@coop/sharedfor general consumers,@coop/shared/appfor the app shell
Never use deep import paths like@coop/shared/modules/auth/...; use public API surfaces only
All environment variables requireVITE_prefix for frontend access
Surface all errors to the user; never swallow errors
Set chain viaVITE_COOP_CHAINenvironment variable:sepolia(default) orarbitrum(production)
Set onchain integration mode viaVITE_COOP_ONCHAIN_MODE:mock(default) orlive
Set archive mode viaVITE_COOP_ARCHIVE_MODE:mock(default) orlive
Set session mode viaVITE_COOP_SESSION_MODE:mock,live, oroff(default)
Files:
packages/shared/src/modules/storage/db-crud-content.tspackages/extension/src/background/handlers/__tests__/coop-handlers.test.tspackages/extension/src/background/handler-registry.tspackages/shared/src/modules/storage/__tests__/db-crud-content-sync.test.tspackages/extension/src/background/handlers/coop.ts
packages/extension/src/**/*.{ts,tsx,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Reuse existing CSS classes from
packages/extension/src/global.cssand design tokens fromshared/src/styles/tokens.cssinstead of duplicating styles
Files:
packages/extension/src/background/handlers/__tests__/coop-handlers.test.tspackages/extension/src/background/handler-registry.tspackages/extension/src/background/handlers/coop.ts
packages/extension/src/background/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
packages/extension/src/background/**/*.ts: Usechrome.alarmsfor timers in background worker; never usesetInterval
Never usewindowin service worker context
Files:
packages/extension/src/background/handlers/__tests__/coop-handlers.test.tspackages/extension/src/background/handler-registry.tspackages/extension/src/background/handlers/coop.ts
🪛 LanguageTool
docs/reference/nlnet-fediversity-coop-proposal-2026-05-31.md
[grammar] ~26-~26: Ensure spelling is correct
Context: ...sskey identity + export are real today; onchain anchoring + Filecoin archival are **moc...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~74-~74: Ensure spelling is correct
Context: ...ludes moving the currently mock-default onchain/archival rails toward a production path...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~89-~89: Ensure spelling is correct
Context: ...ction-hardening currently-mock rails**: onchain anchoring and Filecoin archival are moc...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~94-~94: Consider an alternative for the overused word “exactly”.
Context: ... waste, education, and solar. These are exactly the contexts that most need self-owned,...
(EXACTLY_PRECISELY)
🪛 markdownlint-cli2 (0.22.1)
docs/reference/nlnet-fediversity-coop-proposal-2026-05-31.md
[warning] 46-46: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🔇 Additional comments (10)
docs/reference/index.md (1)
34-34: LGTM!docs/package.json (1)
23-24: LGTM!packages/extension/src/background/handlers/coop.ts (5)
63-72: LGTM!
74-80: LGTM!
82-91: LGTM!
93-129: LGTM!
131-144: LGTM!packages/extension/src/background/handlers/__tests__/coop-handlers.test.ts (1)
735-735: LGTM!Also applies to: 769-773
packages/shared/src/modules/storage/db-crud-content.ts (1)
259-269: LGTM!packages/extension/src/background/handler-registry.ts (1)
571-581: LGTM!
Oba-One
force-pushed
the
codex/code-root-cleanup-coop
branch
from
666c081 to
3eefb88
Compare
Oba-One
changed the title
Oba-One
force-pushed
the
codex/code-root-cleanup-coop
branch
from
3eefb88 to
0ac6bf4
Compare
Oba-One
force-pushed
the
codex/code-root-cleanup-coop
branch
from
0ac6bf4 to
35f37de
Compare
1 participant
Summary
Validation