@W-22951833 [TableauMCP][SPAM API] GetJobs as mcp tool#395
Conversation
|
Thanks for the contribution! It looks like @sf-nanjie-chenglie is an internal user so signing the CLA is not required. However, we need to confirm this. |
Akash-Rastogi
left a comment
There was a problem hiding this comment.
- Branched from stale base — version: "2.6.3" → "2.7.0" but main is already at 2.9.3+. Can have significant merge conflicts.
- Missing list-users scope in scopes.ts — their base doesn't have tableau:mcp:users:read in the McpScope type (since they branched before it was added). Merge conflict will surface this.
- No tests/e2e/server.test.ts update — list-jobs needs to be added to adminOnlyTools in the e2e server test or CI will fail (same issue we hit).
- No OAuth embedded test update — tableau:mcp:jobs:read needs to be in the scopes_supported arrays in tests/oauth/embedded-authz/oauth.test.ts.
- Filter validation trusts input partially — operator as FilterOperator | 'has' cast without full validation of the has literal before checking the allow list. Could pass through unexpected strings.
| callback: async (restApi) => { | ||
| const adminResult = await assertAdmin(restApi, extra); | ||
| if (adminResult.isErr()) { | ||
| throw new Error(adminResult.error); |
There was a problem hiding this comment.
throw new Error (its the older pattern). Should use AdminOnlyError.toErr()
|
@ncgit9c — 🤖 MattGPT (Matt Filbert's agent) Adds the One small inline note on |
|
|
||
| const paramsSchema = { | ||
| filter: z.string().optional(), | ||
| pageSize: z.number().int().positive().optional(), |
There was a problem hiding this comment.
🤖 MattGPT (Matt Filbert's agent): pageSize accepts any positive integer, but Tableau's Query Jobs API caps at 1000 (per the docs on line ~40 and the paginationParameters comment). Adding .max(1000) would reject invalid requests client-side. Same pattern exists in other list-* tools, so not unique to this PR — flagging since it's easy to fix here.
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
Adding mcp tool for existing tableau query jobs rest api.
Motivation and Context
Type of Change
How Has This Been Tested?
Tested using claude as client, tableau mcp as server, PAT on tableau cloud site, querying jobs data.
Related Issues
#394
Checklist
npm run version. For example,use
npm run version:patchfor a patch version bump.environment variable or changing its default value.
Contributor Agreement
By submitting this pull request, I confirm that:
its Contribution Checklist.