Skip to content

@W-21254846: Add mechanism for conditional registration - #871

Merged
yyk16293 merged 38 commits into
mainfrom
add-conditional-registration
Sep 10, 2026
Merged

@W-21254846: Add mechanism for conditional registration#871
yyk16293 merged 38 commits into
mainfrom
add-conditional-registration

Conversation

@stephendeoca

@stephendeoca stephendeoca commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

IMPORTANT: Please do not create a Pull Request without creating an issue first.

Any change needs to be discussed before proceeding. Failure to do so may result in the rejection of
the pull request.

Pull Request Template

Description

Introduces a new tool property called registrationConditions which can be used to indicate if a certain criteria has been met before registering the tool. When a tool specifies a registration condition, a method for checking the condition must be implemented. The registration context object is also populated so that conditions do not need to be rechecked with every tool. This logic is gated by the enforce-registration-conditions feature flag.

Introduces 2 conditions, RequiresPulse and RequiresPulsePremium.

Motivation and Context

As we increase our surface area of tools, we want to make sure only the relevant tools are registered for a user. For example, if pulse is not available for a site then the pulse tools should not be registered for a user.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Other (please describe):

Version Bump

Version is bumped automatically after merge. Select the bump this PR needs (defaults to Patch if none selected):

  • Patch
  • Minor
  • Major

How Has This Been Tested?

Related Issues

N/A

Checklist

  • I have made any necessary changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have documented any breaking changes in the PR description. For example, renaming a config
    environment variable or changing its default value.

Contributor Agreement

By submitting this pull request, I confirm that:

@stephendeoca stephendeoca self-assigned this Sep 1, 2026
@stephendeoca
stephendeoca changed the base branch from main to gate-admin-tools-by-role September 1, 2026 22:40
Comment thread src/tools/web/registrationConditions.ts
Comment thread src/tools/web/registrationConditions.ts
Comment thread src/server.web.ts Outdated
github-actions Bot and others added 8 commits September 6, 2026 22:31
…ion instructions (#881)

* implement Pulse registration conditions and unmet-condition instructions

Fills in the three pieces left open by the conditional-registration
mechanism: the capability checks, the tool declarations, and the
client-facing explanation.

Capability checks (src/tools/web/registrationConditions.ts):

- RequiresPulse has no dedicated availability endpoint, so checkPulseEnabled
  probes the cheapest Pulse read (a one-item page of metric definitions) and
  classifies the outcome. PulseDisabledError (site setting off) and
  PulseNotAvailableError (Tableau Server) are definitive answers and are not
  retried; anything indeterminate is retried and then fails closed. A site
  with no definitions still counts as enabled.
- RequiresPulsePremium reads GET /api/-/pulse/entitlements and requires
  ENTITLEMENT_TYPE_PULSE_PREMIUM_INSIGHTS to be enabled -- the entitlement
  whose absence produces PulseInsightsDisabledError ("requires Tableau+").
  It short-circuits on the memoized base Pulse probe first, so a tool
  declaring both conditions costs one probe per capability.

The entitlements endpoint is new to the SDK: added the Zodios endpoint,
the response schema (entitlement_type kept as a bare string so a new
service-side enum member cannot fail validation and hide entitled tools),
the getPulseEntitlements method, and the tableau:entitlements:read scope.

Tool declarations: all eight Pulse-backed tools declare RequiresPulse,
including generate-insight-cards, which routes through the Pulse insight
bundle. The three AI-insight tools additionally declare
RequiresPulsePremium.

Initialization message (src/server.web.ts): unmet conditions now append a
per-condition explanation to the initialize instructions, so a Pulse-less
site is told Pulse is off and how to enable it instead of silently
receiving a shorter tool list. Typed as a total Record so adding a
condition without user-facing copy fails to compile.

Note for operators: the premium probe requests tableau:entitlements:read,
and a Connected App rejects a JWT mint asking for an un-granted scope, so
a deployment turning on enforce-registration-conditions must grant that
scope or the premium tools fail closed.

* address review: genericize Pulse omit copy

Stephen's notes on #881:
- RequiresPulse: don't assume site-admin-disabled; Pulse can also be
  unavailable for user-level preferences or Tableau Server. Soften to
  "not available" and keep the Cloud setup link as optional guidance.
- MissingConditionCheck: reconnect cannot help (missing code path). Drop
  the disconnect/reconnect advice and say the check is not implemented.
@stephendeoca
stephendeoca added this pull request to stack #899 September 9, 2026 21:51
@stephendeoca
stephendeoca removed this pull request from stack #899 September 9, 2026 23:53
Base automatically changed from gate-admin-tools-by-role to main September 10, 2026 00:01
@stephendeoca
stephendeoca marked this pull request as ready for review September 10, 2026 00:07
Comment thread src/tools/web/registrationConditions.ts
Comment thread src/tools/web/registrationConditions.ts Outdated
@yyk16293
yyk16293 merged commit 2bc5f10 into main Sep 10, 2026
10 checks passed
@yyk16293
yyk16293 deleted the add-conditional-registration branch September 10, 2026 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants