[DRAFT] Phase 5: add type-scoped grants and entitlements#1029
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -1278,11 +1286,17 @@ func (s *syncer) SyncTargetedResource(ctx context.Context, action *Action) error | |||
| } | |||
|
|
|||
| if !shouldSkipEnts { | |||
| s.state.PushAction(ctx, Action{ | |||
| Op: SyncEntitlementsOp, | |||
| ResourceTypeID: resourceTypeID, | |||
| ResourceID: resourceID, | |||
| }) | |||
| typeScopedEnts, err := s.resourceTypeHasTypeScopedEntitlements(ctx, resourceTypeID) | |||
| if err != nil { | |||
| return err | |||
| } | |||
| if !typeScopedEnts { | |||
| s.state.PushAction(ctx, Action{ | |||
| Op: SyncEntitlementsOp, | |||
| ResourceTypeID: resourceTypeID, | |||
| ResourceID: resourceID, | |||
| }) | |||
| } | |||
| } | |||
There was a problem hiding this comment.
🟡 Suggestion: For a targeted sync, SyncTargetedResource skips the per-resource grant/entitlement actions when the type is type-scoped, but the targeted-sync seeding in parallel_syncer.go (InitOp path, ~L199-217) only pushes SyncTargetedResourceOp + SyncResourceTypesOp and never enqueues a top-level SyncGrantsOp/SyncEntitlementsOp. Net effect: a targeted sync of a type-scoped resource type collects zero grants/entitlements. If that's intended for a later phase, consider a comment noting it; otherwise the targeted path needs to enqueue the type-scoped action. (medium confidence)
| p.entitlementsCountOnly[resourceType] = true | ||
| } | ||
|
|
||
| func (p *ProgressLog) EntitlementsProgress(resourceType string) int { |
There was a problem hiding this comment.
🟡 Suggestion: EntitlementsProgress (and GrantsProgress at L342) are new exported methods but have no callers in this PR. Exported surface on a shared SDK is a long-term commitment — if these are only needed by Phase 6 replay/equivalence tests, consider adding them alongside their first use rather than landing dead exported API now. (low confidence)
General PR Review: [DRAFT] Phase 5: add type-scoped grants and entitlementsBlocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0 Review SummaryScanned the full PR diff for security and correctness. This change adds opt-in type-scoped grant/entitlement collection (new optional Security IssuesNone found. Correctness IssuesNone found. SuggestionsNone. |
Add type-scoped collection for grants and entitlements, including checkpointed EnqueuePageTokens fan-out, connectorbuilder routing and registration validation, targeted-sync guards, progress accounting, and I7/I8 invariant coverage.
| @@ -313,6 +333,18 @@ func (p *ProgressLog) LogGrantsProgress(ctx context.Context, resourceType string | |||
| } | |||
| } | |||
|
|
|||
| func (p *ProgressLog) SetGrantsCountOnly(resourceType string) { | |||
| p.mu.Lock() | |||
| defer p.mu.Unlock() | |||
| p.grantsCountOnly[resourceType] = true | |||
| } | |||
|
|
|||
| func (p *ProgressLog) GrantsProgress(resourceType string) int { | |||
| p.mu.RLock() | |||
| defer p.mu.RUnlock() | |||
There was a problem hiding this comment.
🟡 Suggestion: EntitlementsProgress and GrantsProgress are newly exported but have no callers anywhere in the repo (only the Set*CountOnly and Add*Progress helpers are used by the type-scoped path). Per the SDK scope-control guidance, unused exported helpers become long-term compatibility commitments — consider dropping them until a caller needs them. (confidence: high)
| if !errors.Is(context.Cause(runCtx), context.DeadlineExceeded) { | ||
| return warnings, batchErr | ||
| } | ||
| checkpointErr := s.Checkpoint(ctx, true) | ||
| return warnings, errors.Join(checkpointErr, ErrSyncNotComplete) |
There was a problem hiding this comment.
🟡 Suggestion (low confidence): when runCtx is deadline-exceeded, a genuine action error carried in batchErr is silently discarded in favor of ErrSyncNotComplete. This only bites in the race where the run duration expires at the same moment a worker hits a real (non-cancellation) error, and resume-then-retry limits the blast radius, but the original error is neither logged nor surfaced. Consider logging batchErr here so a connector bug isn't masked by an expiry that happened to land concurrently.
1 participant
Summary
Implements Phase 5 of the source-cache replay stack: type-scoped grant and entitlement collection.
Resource types can now opt out of per-resource fan-out using
TypeScopedGrantsorTypeScopedEntitlements. The syncer instead issues one type-scoped planner action, and connectors can enqueue checkpointed sibling cursors withEnqueuePageTokens.Changes
Scope
This PR contains collection and routing behavior only. Source-cache replay, continuation lookup, and warm/cold equivalence tests remain in Phase 6.
Testing
go test ./pkg/sync ./pkg/sync/progresslog ./pkg/connectorbuilder