Skip to content

@W-21254846: Implement Pulse registration conditions and unmet-condition instructions - #881

Merged
stephendeoca merged 2 commits into
tableau:add-conditional-registrationfrom
dplatt84:add-pulse-registration-conditions
Sep 8, 2026
Merged

@W-21254846: Implement Pulse registration conditions and unmet-condition instructions#881
stephendeoca merged 2 commits into
tableau:add-conditional-registrationfrom
dplatt84:add-pulse-registration-conditions

Conversation

@dplatt84

@dplatt84 dplatt84 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Description

Builds on #871, which added the registrationConditions tool property and the omission logic for
RequiresPulse / RequiresPulsePremium. That PR left three things open: how to actually check the
conditions, declaring them on the Pulse tools, and an initialization message explaining why Pulse
was unavailable. This adds all three, plus tests for the Pulse enablement scenarios.

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

RequiresPulse — there is no endpoint that reports Pulse availability directly, so
checkPulseEnabled probes the cheapest Pulse read available (a one-item page of metric
definitions) and classifies the outcome:

Outcome Result
Ok enabled — including an empty page, since a brand-new Pulse site has no definitions yet
PulseDisabledError disabled (site setting off), not retried
PulseNotAvailableError disabled (Tableau Server), not retried
anything else retried, then fails closed

The two Pulse-specific errors are definitive answers rather than failures, so retrying them would
just delay registration for every caller on a Pulse-less site. The retry predicate mirrors
getCurrentUserSiteRole but also understands McpToolError, which is what the Pulse methods
return instead of a raw AxiosError.

RequiresPulsePremium — reads GET /api/-/pulse/entitlements and requires
ENTITLEMENT_TYPE_PULSE_PREMIUM_INSIGHTS to be enabled. That is the entitlement whose absence
produces PulseInsightsDisabledError ("requires Tableau+"). It short-circuits on the memoized
base-Pulse probe first, since premium is a superset of base Pulse — so a tool declaring both
conditions costs one probe per capability, not two.

The entitlements endpoint was not in the SDK yet, so this also adds the Zodios endpoint, the
response schema, getPulseEntitlements, and the tableau:entitlements:read scope.

2. Tool declarations

All eight Pulse-backed tools declare RequiresPulse. That includes generate-insight-cards, which
isn't named like a Pulse tool but routes through generatePulseMetricValueInsightBundle. The three
AI-insight tools (generate-pulse-insight-brief, generate-pulse-metric-value-insight-bundle,
generate-insight-cards) additionally declare RequiresPulsePremium.

3. Initialization message (src/server.web.ts)

Replaces the TODO with one explanation per distinct unmet condition, appended to the initialize
instructions. Registration runs before the transport connects, so the instructions are the only
channel that reaches the user — without this a Pulse-less site just gets a shorter tool list and no
reason for it. The copy is a total Record<RegistrationCondition, string>, so adding a condition
without user-facing copy is a compile error rather than a silent omission.

Motivation and Context

Continues the goal from #871: register only the tools that are actually usable for a given site, and
tell the user when something was withheld and why.

Notes for reviewers

Two deliberate decisions worth a look:

  • entitlement_type is a bare string, not a Zod enum. The service owns that enum, and a
    validation error here happens at registration time — it would fail closed and hide tools from
    entitled customers. Forward compatibility is worth more than strictness in this specific spot.
  • The premium probe requests tableau:entitlements:read. Per the existing notes in scopes.ts,
    a Connected App rejects a JWT mint that asks for an un-granted scope, so any deployment turning
    on enforce-registration-conditions must grant that scope or the premium tools will fail closed.
    Everything stays behind that default-off flag, so there is no change in behavior until it is
    enabled — but it should be called out in the rollout notes.

One open question: ENTITLEMENT_TYPE_PULSE_PREMIUM_GAI is a sibling entitlement that gates
generative-AI features specifically. Mapping the single coarse RequiresPulsePremium condition to
PULSE_PREMIUM_INSIGHTS covers the insight-bundle tools cleanly, but the conversational insight
brief is arguably GAI. Splitting the condition is a reasonable follow-up if that distinction matters
in practice.

Type of Change

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

How Has This Been Tested?

Unit tests. registrationConditions.test.ts covers the enablement scenarios end to end: Pulse
enabled with and without definitions, disabled by site setting, unavailable on Tableau Server,
retry-then-succeed, exhausted retries, 4xx not retried, 5xx retried, memoization across tools, and
for premium — entitlement enabled, disabled, absent from the response, a different premium
entitlement not being mistaken for it, and the short-circuit when base Pulse is off.
server.web.test.ts covers registration and the initialize instructions in both directions.

Full unit suite: 2895 passing. tsc --noEmit and eslint clean over the changed files.

Related Issues

Stacked on #871. Work item: @W-21254846

Checklist

  • I have updated the version in the package.json file by using npm run version. Not bumped
    deliberately — the base branch is still at 4.8.0, and bumping in a stacked PR would just
    conflict. Worth a single bump when the stack lands.
  • 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

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.
Comment thread src/tools/web/registrationConditions.ts Outdated
Comment thread src/tools/web/registrationConditions.ts Outdated
Stephen's notes on tableau#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.
@dplatt84

dplatt84 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

Live verification

Exercised the registration path against a built server with enforce-registration-conditions: true (flag left default-off in the PR; flipped only for local verification).

Environment Expected signal Result
Tableau Server — genapps.demo.tableau.com (Pulse not available) PulseNotAvailableError → omit Pulse tools All Pulse tools omitted; initialize NOTE that Pulse is not available; stderr warning names RequiresPulse + the 7 gated tools
Tableau Cloud — 10ay.online.tableau.com / site danp10ay (Pulse disabled) PulseDisabledError → omit Pulse tools Same omission, NOTE, and warning
Control — genapps with the flag off Tools register regardless of Pulse 7 Pulse tools present again (21 tools total)

Notes:

  • generate-insight-cards stays unregistered in all three runs because it is still gated by INSIGHTS_TOOLS_ENABLED, independent of registration conditions.
  • Feature flag restored to false after the runs so the PR default stays unchanged.

@dplatt84

dplatt84 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

Live verification (premium split)

Follow-up to the earlier Pulse-unavailable runs. Same setup: built server with enforce-registration-conditions: true (and INSIGHTS_TOOLS_ENABLED=true so generate-insight-cards is in the premium set). Flag restored to default-off afterward.

Environment Expected Result
Pulse on, Tableau+ missing — dev-dataplane7 / corexq3devdp7noentitlements Basic Pulse tools present; premium tools hidden Pass. All 5 basic list tools present; all 3 premium tools omitted (generate-pulse-metric-value-insight-bundle, generate-pulse-insight-brief, generate-insight-cards). Initialize NOTE mentions Tableau+; warning names RequiresPulsePremium only.
Pulse on, Tableau+ present — dev-dataplane7 / pulseproductvalidation Basic + premium all present Pass. All 5 basic + all 3 premium tools registered. No registration-condition warning; no Tableau+ omit NOTE.

Together with the earlier Server / Pulse-disabled Cloud runs, that covers the four registration outcomes we care about for this change.

@dplatt84

dplatt84 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

Live verification summary

End-to-end checks of the registration-condition path against a built build/index.js over stdio (same shape as our e2e MCP client), not unit mocks. Credentials were supplied out-of-band for each site and are not repeated here.

Method

  1. Checked out add-pulse-registration-conditions (this PR).
  2. Temporarily set enforce-registration-conditions: true in features.json, ran npm run build (so build/features.json picked up the flag). The PR default remains false; the flag was restored to false and rebuilt after every session.
  3. For each site, spawned the built server with TRANSPORT=stdio, AUTH=pat, PRODUCT_TELEMETRY_ENABLED=false, ENABLE_MCP_SITE_SETTINGS=false, plus that site’s SERVER / SITE_NAME / PAT.
  4. Connected an MCP client, called listTools(), read initialize instructions, and scraped stderr JSON for the registration-condition warning.
  5. For the premium-split runs, also set INSIGHTS_TOOLS_ENABLED=true so generate-insight-cards is in the premium set (it is independently gated and would otherwise stay off).

Tool buckets used when scoring results

  • Basic Pulse (RequiresPulse only):
    list-all-pulse-metric-definitions, list-pulse-metric-definitions-from-definition-ids, list-pulse-metrics-from-metric-definition-id, list-pulse-metrics-from-metric-ids, list-pulse-metric-subscriptions
  • Premium Pulse (RequiresPulse + RequiresPulsePremium):
    generate-pulse-metric-value-insight-bundle, generate-pulse-insight-brief, generate-insight-cards

Results

# Site Pulse / entitlement state Expected Observed
1 https://genapps.demo.tableau.com (default site) Tableau Server — Pulse not available (PulseNotAvailableError) Omit all Pulse tools; initialize NOTE that Pulse is not available Pass. 0 Pulse tools registered (14 tools total). Initialize NOTE present. Stderr: 7 tool(s) were omitted … RequiresPulse: … (the 7 tools that declare the condition and are otherwise enabled; generate-insight-cards was already off via INSIGHTS_TOOLS_ENABLED in this run).
2 https://10ay.online.tableau.com / danp10ay Tableau Cloud — Pulse disabled (PulseDisabledError) Same full omit as (1) Pass. Same pattern: all Pulse tools omitted, Pulse NOTE in instructions, RequiresPulse warning for the 7 gated tools.
3 genapps again, flag off (enforce-registration-conditions: false) Same Server site as (1); conditions not enforced Basic + premium Pulse tools register despite Pulse being unavailable Pass (control). 7 Pulse tools present again (21 tools total): the 5 basic list tools + insight bundle + insight brief. Confirms the gate is opt-in and not permanently hiding tools.
4 https://dev-dataplane7.tableau.sfdc-3vx9f4.svc.sfdcfc.net / corexq3devdp7noentitlements Pulse enabled, Tableau+ / ENTITLEMENT_TYPE_PULSE_PREMIUM_INSIGHTS missing Basic present; premium hidden; Tableau+ NOTE Pass. All 5 basic tools present; all 3 premium tools absent (19 tools total). Initialize NOTE: AI-powered Pulse insight tools omitted because Tableau+ entitlement is missing. Stderr: 3 tool(s) were omitted … RequiresPulsePremium: generate-pulse-metric-value-insight-bundle, generate-pulse-insight-brief, generate-insight-cards. No RequiresPulse omission (base Pulse probe succeeded).
5 same host / pulseproductvalidation Pulse enabled, Tableau+ entitlements present Basic + premium all present; no omit warning Pass. All 5 basic + all 3 premium tools registered (22 tools total). No registration-condition warning. No Tableau+ omit NOTE.

What this covers

Outcome Covered by
Pulse unavailable (Server) → hide all Pulse tools #1
Pulse disabled (Cloud site setting) → hide all Pulse tools #2
Feature flag off → no gating #3
Pulse on, premium entitlement off → keep basic, hide premium #4
Pulse on, premium entitlement on → keep everything #5

Also confirms the initialize-instruction copy paths for RequiresPulse and RequiresPulsePremium (including the review follow-up that genericized the Pulse-unavailable wording).

Not claimed here

  • No live run against a site that only fails ENTITLEMENT_TYPE_PULSE_PREMIUM_GAI while insights entitlement stays on (open question called out in the PR description).
  • Unit coverage for retries / fail-closed / memoization remains in registrationConditions.test.ts / server.web.test.ts; this comment is only the live registration matrix.

@stephendeoca
stephendeoca merged commit 5f5e496 into tableau:add-conditional-registration Sep 8, 2026
4 checks passed
yyk16293 pushed a commit that referenced this pull request Sep 10, 2026
* add requireAdmin to tools

* Fetch userRole during tool reg

* change requireAdmin to requiredRoles

* Add version bump

* Add min role hierarchy

* role value changes

* add support user and remove cache

* add role requirements feature flag and bump version

* make SupportUser Admin

* add retry and tool omission message

* add retries

* add registration context and lazy fetching

* add conditional registration

* update fetch user role logic

* chore: bump version to 4.8.2

* chore: bump version to 4.8.3

* @W-21254846: Implement Pulse registration conditions and unmet-condition 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.

* chore: bump version to 4.8.4

* Update comments and remove unneccesary scope

* chore: bump version to 4.8.5

* Remove lazy fetching

* chore: bump version to 4.8.6

* update comment

* chore: bump version to 4.10.0

* update comments

* Fix lint

* update comments

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: dplatt84 <dplatt@salesforce.com>
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.

2 participants