Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 9f49073 The changes in this PR will be included in the next version bump. This PR includes changesets to release 24 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
📝 WalkthroughWalkthroughThis PR adds a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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 docstrings
🧪 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds per-role-assignment event history to the Omnigraph by wiring a new permissions_user_events join table into the indexer write-path and exposing it as PermissionsUser.events, mirroring the existing Permission.events pattern.
Changes:
- Introduces
permissions_user_eventsjoin table +ensurePermissionsUserEvent()helper and writes it on everyEACRolesChanged. - Extends Omnigraph to expose
PermissionsUser.events(where: EventsWhereInput, ...)via the sharedresolveFindEventsresolver. - Adds integration tests for scoping and filtering; updates Vitest integration workspace globbing and regenerates Omnigraph schema artifacts.
Reviewed changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.integration.config.ts | Excludes .git / .claude / node_modules from integration-project globbing to avoid loading stale worktree configs. |
| packages/enssdk/src/omnigraph/generated/schema.graphql | Regenerated schema exposing PermissionsUser.events + connection types. |
| packages/enssdk/src/omnigraph/generated/introspection.ts | Regenerated introspection matching the schema change. |
| packages/ensdb-sdk/src/ensindexer-abstract/ensv2.schema.ts | Adds permissions_user_events join table keyed by (permissionsUserId, eventId). |
| apps/ensindexer/src/plugins/ensv2/handlers/ensv2/EnhancedAccessControl.ts | On EACRolesChanged, writes both permissions_events and permissions_user_events join rows (including the zero-roles delete branch). |
| apps/ensindexer/src/lib/ensv2/event-db-helpers.ts | Adds ensurePermissionsUserEvent() insert helper with conflict-do-nothing semantics. |
| apps/ensapi/src/omnigraph-api/schema/permissions.ts | Adds PermissionsUser.events connection, scoped through permissionsUserEvent join table, with EventsWhereInput parity. |
| apps/ensapi/src/omnigraph-api/schema/permissions.integration.test.ts | Adds integration coverage for scoping and selector_in filtering for PermissionsUser.events. |
| apps/ensapi/src/omnigraph-api/lib/find-events/find-events-resolver.ts | Extends EventJoinTable union to include permissionsUserEvent. |
| AGENTS.md | Documents Ponder expectations around rebuilds and PK-only access constraints. |
| .changeset/permissions-user-events.md | Changeset for the additive Omnigraph/API + schema/indexer behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/ensapi/src/omnigraph-api/schema/permissions.integration.test.ts`:
- Around line 534-593: Add an integration test in
permissions.integration.test.ts that exercises the zero-roles deletion path:
create a new "grant" then a "revoke" (so PermissionsUser.roles becomes 0)
against V2_ETH_REGISTRY and then run the PermissionsUserEvents query to assert
the EACRolesChanged event for that revoke exists (use
EAC_ROLES_CHANGED_SELECTOR) and that the join/write side-effects for the event
were performed even though the PermissionsUser was removed (validate by checking
the event topics include the resource and padded user address and that the
revoke event is present while the PermissionsUser no longer has roles). Ensure
the new test references PermissionsUserEvents, V2_ETH_REGISTRY,
EAC_ROLES_CHANGED_SELECTOR, PermissionsUser and the roles == 0 branch so it
directly covers the grant→revoke flow.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 531b89a6-1b25-4734-8313-5df6fbb5c836
⛔ Files ignored due to path filters (2)
packages/enssdk/src/omnigraph/generated/introspection.tsis excluded by!**/generated/**packages/enssdk/src/omnigraph/generated/schema.graphqlis excluded by!**/generated/**
📒 Files selected for processing (9)
.changeset/permissions-user-events.mdAGENTS.mdapps/ensapi/src/omnigraph-api/lib/find-events/find-events-resolver.tsapps/ensapi/src/omnigraph-api/schema/permissions.integration.test.tsapps/ensapi/src/omnigraph-api/schema/permissions.tsapps/ensindexer/src/lib/ensv2/event-db-helpers.tsapps/ensindexer/src/plugins/ensv2/handlers/ensv2/EnhancedAccessControl.tspackages/ensdb-sdk/src/ensindexer-abstract/ensv2.schema.tsvitest.integration.config.ts
Contributor
Greptile SummaryThis PR adds Confidence Score: 5/5Safe to merge — additive write path and new GraphQL field with no breaking changes. All changes are additive and follow established patterns exactly. The handler correctly writes the join-table row on both branches of the zero-roles conditional. The previously flagged topic/resource type mismatch in the test is resolved via BigInt() conversion. No P0 or P1 findings. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant Chain as Blockchain
participant Handler as EACRolesChanged Handler
participant DB as ensDb (Ponder)
participant API as Omnigraph API
Chain->>Handler: EACRolesChanged(resource, account, oldBitmap, newBitmap)
Handler->>DB: ensureAccount(user)
Handler->>DB: ensurePermissionsResource(contract, resource)
alt newRoleBitmap == 0
Handler->>DB: delete permissionsUser (id = permissionsUserId)
else newRoleBitmap != 0
Handler->>DB: upsert permissionsUser (id = permissionsUserId)
end
Handler->>DB: ensureEvent → eventId
Handler->>DB: ensurePermissionsEvent(contract, eventId)
Handler->>DB: ensurePermissionsUserEvent(permissionsUserId, eventId)
Note over DB: permissions_user_events row persists even after permissionsUser deletion
API->>DB: PermissionsUser.events query
DB-->>API: JOIN permissionsUserEvent ON eventId WHERE permissionsUserId = parent.id
API-->>API: apply EventsWhereInput filters
Reviews (3): Last reviewed commit: "feat: PermissionsUser.events" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
- AGENTS.md: document ponder schema-rebuild + PK-cache constraints - vitest.integration.config.ts: exclude .git/.claude/node_modules from projects glob to avoid loading worktree configs
Exposes per-(contract, resource, user) event history on PermissionsUser, collapsing the prior client-side pattern of filtering Permission.events by account. Closes #1963.
shrugs
force-pushed
the
feat/permissions-user-events
branch
from
d1377cc to
9f49073
Compare
Member
Author
|
@greptile review |
Member
Author
|
merging as highly confident in the implementation and to prioritize omnigraph feature velocity for this week |
shrugs
added a commit
that referenced
this pull request
Apr 29, 2026
- events: add sender column row, update from description to flag never HCA-aware, add sender to indexes list - permissions_user_events: document join table (was missing from initial ENSDb docs in #2007) - domains.ownerId: polymorphic ENSv1 effective-owner / ENSv2 on-chain owner (HCA-aware) wording - registrations.registrantId / unregistrantId: ENSv2 HCA-aware note - permissions_users.user: HCA-aware grantee description Per #2014 (comment) review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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.
Reviewer Focus (Read This First)
Most of the surface area is mechanical mirrors of the existing
Permission.eventspattern. spend time on:apps/ensindexer/src/plugins/ensv2/handlers/ensv2/EnhancedAccessControl.ts— the newensurePermissionsUserEventcall sits alongside the existingensurePermissionsEvent. confirm it should fire on everyEACRolesChanged(including the zero-roles branch where thepermissionsUserrow is deleted but the event log is preserved).apps/ensapi/src/omnigraph-api/schema/permissions.ts— the newPermissionsUser.eventsfield. it intentionally keeps the fullEventsWhereInputarg for parity withPermission.events, even though the join is already maximally narrow.permissions.integration.test.ts— i replaced an initial wrong "disjoint sum" assertion with a topic-based(resource, user)scoping check after seeing it fail.Problem & Motivation
closes #1963.
portal's
/$name/roles/and/resolver/$address/roles/views want per-role-assignment history: which events granted/revoked/modified this specific role entry for this user. today the closest omnigraph query isPermission.events(where: { selector })filtered client-side by account + resource. this collapses that into a server-side join.What Changed (Concrete)
permissions_user_eventsjoin table inpackages/ensdb-sdk/src/ensindexer-abstract/ensv2.schema.ts(keyed on(permissionsUserId, eventId))ensurePermissionsUserEvent(context, permissionsUserId, eventId)helper inapps/ensindexer/src/lib/ensv2/event-db-helpers.tsEACRolesChangedhandler writes to bothpermissionsEventandpermissionsUserEventjoin tablesEventJoinTableunion infind-events-resolver.tsPermissionsUser.events(where: EventsWhereInput, ...)graphql field, mirror ofPermission.events(resource, user)validation, andselector_infilteringenssdk/src/omnigraph/generated/{schema.graphql,introspection.ts}vitest.integration.config.ts: exclude.git/.claude/node_modulesfrom the projects glob (was loading stale worktree configs)Design & Planning
straightforward parallel of
Permission.eventsand itspermissionsEventjoin tableSelf-Review
sum(per-user-events) == permission.events.countfor root users. wrong —Permission.eventscovers all resources, whilepermissions.root.users[].eventsonly covers root-resource users. replaced with a topic-based check that each event'stopics[1]equalsuser.resourceandtopics[2]equals paddeduser.user.address.Cross-Codebase Alignment
permissionsEvent,permissions_events,makePermissionsUserId,ensurePermissionsEvent,Permission.events,EventJoinTablePermission.eventsfield/resolver (left as-is — both fields coexist),Account.permissions,Domain.permissions,Resolver.permissions(existing connections, untouched)Downstream & Consumer Impact
PermissionsUser.eventsgraphql field. additive — no breaking changes.enssdk/src/omnigraph/generated/schema.graphqlregenerated.events(matchesPermission.events), join table ispermissions_user_events(matchespermissions_events).consumers per the issue:
apps/portal//$name/roles/apps/portal//resolver/$address/roles/Testing Evidence
pnpm test:integration apps/ensapi/src/omnigraph-api/schema/permissions.integration.test.ts→ 27/27 pass.pnpm -F ensdb-sdk typecheck,pnpm -F ensindexer typecheck,pnpm -F ensapi typecheck,pnpm lint,pnpm generate:gqlschemaall clean.PermissionsUser.events(the existingPermission.events paginationblock exercises the sharedresolveFindEventscode path).EACRolesChangedregardless of the roles=0 delete branch.Scope Reductions
PermissionsUser.events paginationtest usingtestEventPaginationif reviewers want it. would need a top-level by-id query forPermissionsUseror awkward traversal.resolveFindEventsis shared and already covered byPermission.events pagination. adding a clone there is low-value churn.Risk Analysis
Pre-Review Checklist (Blocking)