[CXH-2211] feat: add clm_workflow_queue resource type - #67
Merged
Conversation
Implements 4a from the CXH-1975 QE validation notes: syncs DocuSign
CLM's WorkflowQueue object — the API's own term for what the CLM admin
console reportedly calls "Task Groups", the surface the customer (The
Trade Desk) explicitly asked for. That naming equivalence is an
unconfirmed assumption, not a documented fact; the resource type is
named after the API term (clm_workflow_queue), not the UI term, until
someone with access to a real CLM admin console can confirm it.
The API has no list-all endpoint for workflow queues and no reverse
lookup from a queue to its members — only a per-member read (GET
.../members/{id}/workflowqueues). So List() discovers the distinct set
by scanning every clm_member once and deduping, and — as a side effect
of that same scan — builds a queueID -> []memberID index in the SDK's
session cache (newly enabled via connectorrunner.WithSessionStoreEnabled
in main.go, previously unused in this connector) so Grants() can read a
queue's membership back out directly instead of re-scanning every
member per queue, which would turn one O(members) traversal into
O(queues * members) — a real cost given the open rate-limit issue on
this connector for the same customer (CXP-704).
Read-only: the API documents work-item assign/unassign, not
queue-membership grant/revoke, so there's no Grant/Revoke here, matching
clm_permission_set's precedent for a CLM object with no write endpoint.
Like every other CLM endpoint in this connector, the response shapes
are documented-but-unexercised — no live CLM tenant was available to
confirm them.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
Connector PR Review: [CXH-2211] feat: add clm_workflow_queue resource typeBlocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0 Review SummaryThe new commit adds Security IssuesNone found. Correctness IssuesNone found. SuggestionsNone. |
- Extract List()'s member-scan discovery into discoverClmWorkflowQueueMembership, a pure API-domain helper (no v2.Resource, no session cache) that List() just calls and converts. Matches every sibling CLM builder's simpler "discover, then build resources" shape instead of mixing five concerns in one function. - Merge queuesByID/membersByQueueID into one clmWorkflowQueueMembershipEntry map — the two always shared the same key set, so two maps and two trailing loops collapse into one of each with no behavior change. - Factor GetMemberGroups' and GetMemberWorkflowQueues' near-identical page-to-completion loops into a shared clmPageToCompletion[T] generic helper, so the bound/non-advancing-token guard only needs fixing once if it's ever wrong. - Add the missing baton-docusign: prefix on the per-member fetch error in the new discovery helper. Not changed, on purpose: Grants()'s Warn-level log on a session-cache miss (one precedent already exists in this package — singing_groups.go — and the code's own comment explains why Warn fits this specific "shouldn't happen" case; judged not worth changing). Also not changed: Grants() still returns a queue's full membership unpaginated — flagged in review as a real but pre-existing pattern (clm_folders.go's Grants() has the identical, and worse, shape already on main) rather than something unique to this PR; left for a separate, broader fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
FeliLucero1
changed the title
- Tolerate isOptInFeatureUnavailableError per-member in the workflow-queue member scan (a deleted member or a scope gap on one member's endpoint no longer aborts the whole clm_workflow_queue sync). - Skip clm_workflow_queue gracefully (not a hard sync-wide error) when the session cache write fails, matching every other CLM builder's degrade pattern — WithSessionStoreEnabled only opts in to a store existing; the parent process can still fall back to NoOpSessionStore at runtime. - Fail Grants() loudly on a cache miss instead of emitting zero grants, which is indistinguishable from a queue's membership being genuinely emptied out. - Guard against an empty Href collapsing every queue with no Href into one bogus merged resource. - Dedupe repeated RateLimitDescription annotations down to the latest one. - Fix the package doc's endpoint inventory and a doc comment that landed on the wrong declaration after the clmPageToCompletion refactor. - Add pagination coverage for GetMemberWorkflowQueues. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Narrow the per-member workflow-queue-fetch tolerance to codes.NotFound only (an isolated deleted-member case); Unauthenticated/PermissionDenied/ FailedPrecondition now bail the whole resource type via errClmWorkflowQueuesUnavailable instead of being silently swallowed per-member, which could otherwise produce a partial membership set that C1 reads as revoked access. - Apply logarithmic sampling (1, 10, 100, then every 1000) to the two per-member/per-queue Warn logs so a large account doesn't flood the log. - Preserve accumulated RateLimitDescription annotations on List()'s graceful-skip paths instead of discarding them. - Track the last RateLimitDescription by index instead of value in dedupeRateLimitAnnotations, dropping the only direct google.golang.org/protobuf import in the repo (go.mod still marked it // indirect).
Bug: discoverClmWorkflowQueueMembership escalated ANY per-member isOptInFeatureUnavailableError to "CLM unavailable, return zero resources" regardless of scan position. A PermissionDenied/Unauthenticated on member N (token expiring or scope revoked mid-scan) after earlier members had already contributed real queues would silently discard every already-discovered queue as if the whole feature were unavailable — the same false-deletion risk ListMembers' own memberPageToken == "" narrowing exists to avoid. Now only escalates to errClmWorkflowQueuesUnavailable while membership is still empty; once queues have been found, CLM is clearly available, so a later failure fails loud instead. Adds Server.ForceMemberWorkflowQueuesStatus to clmtest to test all three per-member outcomes: NotFound skip-and-continue, PermissionDenied on the first member (graceful skip), PermissionDenied after some discovery (hard failure).
Replace the session-store accumulator with per-member child-resource discovery and member-side grant emission, drop WithSessionStoreEnabled, and update tests/docs for the simpler design. Co-authored-by: Cursor <cursoragent@cursor.com>
Use dynamic ResourceType() annotations (SkipEntitlementsAndGrants when clm_workflow_queue is filtered out) matching the user/permission_profile pattern, drop unstable ParentResourceId on shared queues, update capabilities metadata and docs for the clm_member dependency. Co-authored-by: Cursor <cursoragent@cursor.com>
…urce The caller already uses parentResourceID for logging and API calls; ParentResourceId is no longer stamped on the resource. Co-authored-by: Cursor <cursoragent@cursor.com>
Add clm_workflow_queue to TestNew_IncludeClmDerivation and assert includeWorkflowQueues for each sync filter case. Co-authored-by: Cursor <cursoragent@cursor.com>
The SDK always calls List once without a parent per registered type; return empty instead of error. gofmt connector_test.go struct alignment. Co-authored-by: Cursor <cursoragent@cursor.com>
Warn when clm_workflow_queue is opted in without clm_member, drop dead opts nil guards, refresh GetMemberWorkflowQueues doc comment, and document accepted List/Grants phase skew in clmMemberBuilder.Grants(). Co-authored-by: Cursor <cursoragent@cursor.com>
Sergio's review asked for runtime visibility at debug level, not Warn. Co-authored-by: Cursor <cursoragent@cursor.com>
| includeWorkflowQueues := opts.WillSyncResourceType(clmWorkflowQueueResourceType.Id) | ||
|
|
||
| if includeWorkflowQueues && !opts.WillSyncResourceType(clmMemberResourceType.Id) { | ||
| l.Debug("baton-docusign: clm_workflow_queue is enabled but clm_member is not — workflow queues are discovered per member, so this sync will produce zero queues and zero grants") |
Contributor
There was a problem hiding this comment.
🟡 Suggestion (medium confidence): this breadcrumb is emitted at Debug, which is below the connector's default log level, so in practice the "opted into clm_workflow_queue but not clm_member" misconfiguration still produces a silently empty sync with nothing in the logs — the condition the reviewer thread asked this line to surface. Per the repo's log-level criteria, bad-config init conditions belong at Warn (Debug is for nil/zero/missing-but-expected values, which this isn't: it's a customer sync-filter mistake that yields zero queues and zero grants). Suggest l.Warn(...) here.
Wrap List() errors at the connector layer, PathEscape CLM path segments in prepareClmPagedRequest/buildClmClientURL, and document which SkipEntitlements variant baton_capabilities.json reflects. Co-authored-by: Cursor <cursoragent@cursor.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
8 participants
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.
Description
Useful links: