Skip to content

@W-22998764: Add update-cloud-extract-refresh-task tool#404

Open
Akash-Rastogi wants to merge 4 commits into
mainfrom
akashrastogi/update-cloud-extract-refresh-task
Open

@W-22998764: Add update-cloud-extract-refresh-task tool#404
Akash-Rastogi wants to merge 4 commits into
mainfrom
akashrastogi/update-cloud-extract-refresh-task

Conversation

@Akash-Rastogi

Copy link
Copy Markdown
Contributor

Summary

Adds the Tableau Cloud extract refresh schedule update tool (1B-2 in the 264 admin-MCP breakdown), sibling to delete-extract-refresh-task (#392). Lets admins change the frequency, time window, and recurrence intervals of an existing scheduled extract refresh without recreating the task.

API integration notes

The Tableau "Update Cloud Extract Refresh Task" endpoint has a few traps worth flagging for the next reviewer:

  • POST (not PUT) on /sites/{id}/tasks/extractRefreshes/{id} — same path as delete, different verb.
  • Request body uses extractRefresh and schedule as siblings at top level — NOT nested.
  • Response shape mirrors that (siblings, not wrapped); the SDK method merges them so callers get a single ExtractRefreshTask matching list-extract-refresh-tasks.
  • For Hourly schedules, start.minutes must equal end.minutes and end > start.
  • Tableau ignores end for Daily/Weekly/Monthly but rejects 409004 Invalid subscription schedule if missing on Hourly. The Zod schema makes end mandatory; for non-Hourly callers pass end == start.
  • API documentation lives in rest_api_ref_extract_and_encryption.htm (despite being about schedules).

The tool surfaces Tableau's response error code/detail (e.g. Tableau 400 [409004]: Bad Request: Invalid subscription schedule) so callers can recover from server-side validation without parsing raw axios errors.

Changes

New

  • src/tools/web/extractRefreshTasks/updateCloudExtractRefreshTask.ts — admin-gated, single-phase, destructive + idempotent.
  • src/tools/web/extractRefreshTasks/updateCloudExtractRefreshTask.test.ts — 7 unit tests (registration, annotations, admin gate, happy path, API error pass-through, Hourly schedule, schema enforcement).
  • tests/e2e/extractRefreshTasks/updateCloudExtractRefreshTask.test.ts — registration gate + schema validation + opt-in destructive leg gated on UPDATE_CLOUD_EXTRACT_REFRESH_TASK_E2E_ID.
  • docs/docs/tools/tasks/update-cloud-extract-refresh-task.md — per-tool docs page with request/response shape, schedule constraints, examples.

Modified

  • src/sdks/tableau/types/extractRefreshTask.tsupdateCloudExtractRefreshScheduleSchema + request/response sibling schemas.
  • src/sdks/tableau/apis/tasksApi.ts — POST endpoint with body shape.
  • src/sdks/tableau/methods/tasksMethods.ts — method merges sibling response; surfaces error code/detail.
  • src/server/oauth/scopes.tstableau:mcp:tasks:write + tableau:tasks:write, toolScopeMap entry, admin-gate disabled list.
  • src/server/oauth/scopes.test.ts — toggle tests for the new write scopes.
  • src/restApiInstance.tsJwtScopes union extended.
  • src/tools/web/toolName.ts, tools.ts — webToolNames + tasks group + factory registration.
  • tests/oauth/embedded-authz/oauth.test.tstableau:mcp:tasks:write added to all 3 scopes_supported arrays.
  • tests/e2e/server.test.ts — added to adminOnlyTools (default + combined variants).
  • docs/docs/intro.md, docs/docs/configuration/mcp-config/env-vars.md — tool row + reference link + ADMIN_TOOLS_ENABLED list.
  • package.json, package-lock.json — version `2.13.0` -> `2.14.0`.

Test plan

  • `npx tsc --noEmit` clean
  • `npm run lint` clean
  • `npm test -- --run` — 1753 passing (+10 new on this branch)
  • `npm run build` succeeds
  • Manual MCP-Inspector smoke test against a Tableau Cloud admin site — verified error surfacing on invalid Hourly schedules (409004) and confirmed body-shape resolution (initial 405/404 -> 400 progression documented in commit notes)
  • E2E destructive leg on a Tableau Cloud admin site: `UPDATE_CLOUD_EXTRACT_REFRESH_TASK_E2E_ID= npm run test:e2e`

🤖 Generated with Claude Code

@Akash-Rastogi Akash-Rastogi marked this pull request as ready for review June 18, 2026 00:02
@Akash-Rastogi Akash-Rastogi force-pushed the akashrastogi/update-cloud-extract-refresh-task branch from dd318f6 to 890bf5f Compare June 18, 2026 00:14
* Response body shape for `Update cloud extract refresh task`. `extractRefresh` and `schedule`
* are sibling top-level elements; the schedule is NOT nested inside extractRefresh on this endpoint.
*/
export const updateCloudExtractRefreshTaskResponseSchema = z.object({

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overly strict response schema can report a successful update as a failure
updateCloudExtractRefreshTaskResponseSchema requires extractRefresh: extractRefreshTaskSchema (with non-optional id) and a schedule sibling. Zodios validates responses. If the live Cloud update response omits id or the sibling schedule
(its shape differs from list — siblings, not nested), the parse throws, the catch maps it to {type:'unknown'}, and the tool reports failure on an update that actually succeeded. The mutating e2e leg is gated behind
UPDATE_CLOUD_EXTRACT_REFRESH_TASK_E2E_ID and skipped, so this path is never exercised against the real API — only happy-path mocks. Highest-confidence correctness risk.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a6f6c91e. updateCloudExtractRefreshTaskResponseSchema now uses extractRefreshTaskSchema.partial().optional() for extractRefresh and extractRefreshScheduleSchema.optional() for the sibling schedule. The method falls back to the requested taskId/schedule if either field is missing/partial, so a permissive Cloud response no longer turns a successful update into an Err.

.refine(
(s) => {
if (s.frequency !== 'Hourly' || s.frequencyDetails.end === undefined) return true;
return s.frequencyDetails.end > s.frequencyDetails.start;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hourly end > start uses lexical string comparison, wrongly rejecting valid windows
start/end are bare z.string() (no format enforcement). '10:00:00' > '9:00:00' is false lexically ('1' < '9'), so a valid 9am→10am Hourly window is rejected with "end must be strictly after start". An LLM driving this tool may well emit
non-zero-padded times. Conversely start:'10:00:00', end:'9:00:00' is wrongly accepted.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a6f6c91e. Replaced the lexical compare with a numeric one via timeToSeconds(). New tests cover both directions: start='09:00:00', end='10:00:00' is now accepted (was wrongly rejected lexically), and start='10:00:00', end='09:00:00' is now rejected (was wrongly accepted lexically).

.object({
frequency: z.enum(['Hourly', 'Daily', 'Weekly', 'Monthly']),
frequencyDetails: z.object({
start: z.string().describe('Start time in HH:mm:ss (24-hour) format, e.g. "06:00:00".'),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also includs src/sdks/tableau/types/extractRefreshTask.ts:110:
no HH:mm:ss regex; minute-match refine and the 5-minute boundary are unenforced/fragile
start.slice(3) === end.slice(3) assumes a fixed HH:mm:ss layout. With mixed/short formats ('06:00' → '00' vs '07:00:00' → '00:00') it mis-compares — false accepts and false rejects. Separately, the 5-minute boundary (00,05,…,55) is
documented as a hard constraint in the tool description and docs, but there is no schema check for it; only Tableau rejects it (409004). The docs' "Schema-level rejection before any API call" framing overstates what's enforced. A single
.regex(/^([01]\d|2[0-3]):([0-5]\d):([0-5]\d)$/) on start/end (plus a %5 check) would fix #2, #3, and make the slice logic safe.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a6f6c91e. Added TIME_REGEX = /^([01]\d|2[0-3]):([0-5]\d):([0-5]\d)$/ to both start and end (consolidated as timeStringSchema), plus an isFiveMinuteBoundary refine on each (minutes %5 === 0 AND seconds === 0). The slice(3) minute-match is now safe because the regex guarantees the layout. The tool description and docs page are reframed from "Tableau rejects with 409004" to "schema rejects before any Tableau call" since the rules are now actually enforced client-side.

.describe(
'End time in HH:mm:ss (24-hour) format. Required for Hourly schedules; ignored for Daily/Weekly/Monthly.',
),
intervals: z

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Weekly requires weekDay / Monthly requires monthDay" documented but not enforced
intervals is .optional() and there is no per-frequency refine. {frequency:'Weekly', frequencyDetails:{start:'06:00:00'}} (no intervals) passes the schema and is sent to Tableau, contradicting the tool description and docs which state these
are required. Either add a refine or soften the docs.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a6f6c91e. Added two per-frequency refines: Weekly requires at least one intervals.interval[] entry with weekDay; Monthly requires monthDay. Tests added for both — {frequency:'Weekly', frequencyDetails:{start:'06:00:00'}} (no intervals) is now rejected with a schema-level error pointing at frequencyDetails.intervals.

* are optional; sending only `schedule` is sufficient to change a task's schedule.
*/
export const updateCloudExtractRefreshTaskRequestSchema = z.object({
extractRefresh: z

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

request schema's extractRefresh field is dead/unreachable (YAGNI)
updateCloudExtractRefreshTaskRequestSchema.extractRefresh.type is never populated: the tool params have no such field and tasksMethods sends only { schedule } (the SDK test at tasksMethods.test.ts asserts exactly { schedule }). It
advertises a FullRefresh/IncrementalRefresh capability the tool doesn't offer. Drop it or wire it through.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a6f6c91e. Dropped extractRefresh from updateCloudExtractRefreshTaskRequestSchema — it now wraps just { schedule }. The tool didn't expose FullRefresh/IncrementalRefresh, the SDK method only sent { schedule }, and the unit test asserts exactly { schedule }. Removing it stops advertising a capability the tool doesn't back. If we want to wire it through later, we add the field to paramsSchema, the method, and the body simultaneously.

);
return new Ok({ ...response.extractRefresh, schedule: response.schedule });
} catch (error) {
if (isAxiosError(error) && error.response?.data?.error) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

till line 127

axios→structured-error mapping duplicates viewsMethods.handleQueryImageError
Same isAxiosError && error.response?.data?.error guard, same destructure, same getExceptionMessage fallback (the comment even says "Mirrors viewsMethods"). No shared helper. A shared mapAxiosTableauError(error) would let both converge and
stop the next mutating tool from copy-pasting it again. Also: error.response.data.error fields are read without a zod parse — a non-conforming error body yields a partly-undefined struct.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a6f6c91e. Extracted to src/sdks/tableau/tableauApiError.ts as parseTableauApiError(error). The error body is now parsed through a Zod schema (tableauApiErrorBodySchema), so a non-conforming body returns null and the caller falls back to getExceptionMessage — no more partly-undefined struct. tasksMethods now uses it. viewsMethods.handleQueryImageError can be migrated separately (it has additional feature-disabled branching tied to its 403157 code, so I left it alone in this PR to keep the change scoped).

}

const updated = result.value;
const frequency = updated.schedule?.frequency ?? args.schedule.frequency;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lines 100-105
success message reflects the response echo with no fallback for the time window
frequency falls back to args.schedule.frequency, but start/end come only from updated.schedule?.frequencyDetails. If the response omits frequencyDetails, the message reads "New schedule: Weekly." with no window, even though the caller set
start:'06:00:00'. Fall back to args.schedule.frequencyDetails for the window too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a6f6c91e. Both start and end now fall back to args.schedule.frequencyDetails. New test 'should use args.schedule for the time window when the response omits frequencyDetails' mocks a partial Ok({...task}) (no frequencyDetails on the response schedule) and asserts the message reads New schedule: Weekly (start 06:00:00). from the args.

if (result.isErr()) {
if (result.error.type === 'tableau-api') {
const { status, code, summary, detail } = result.error;
const codeStr = code ? ` [${code}]` : '';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lines 88-92

Nit:
error string emits a stray separator when summary is absent
summaryStr = summary ?? '' then Tableau ${status}${codeStr}: ${summaryStr}${detailStr} produces Tableau 400 [409004]: : Invalid… (double colon) when only code+detail are present. Untested combination.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a6f6c91e. Replaced the unconditional : ${summaryStr}${detailStr} with [summary, detail].filter(Boolean).join(': ') and prepend : only when that joined string is non-empty. New test 'should not produce a double-colon when Tableau returns code without summary' covers the code+detail-only combination and asserts the result reads Tableau 400 [409004]: Invalid subscription schedule. (single colon).


This tool is restricted to Tableau site administrators and requires the \`ADMIN_TOOLS_ENABLED\` feature flag to be enabled.

**Tableau Cloud only.** This tool calls the Cloud variant of the update endpoint and is not appropriate for Tableau Server.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit:
"Cloud only" has no runtime guard
Documented Cloud-only in three places but a Server site just gets a raw 404. Low priority; simplest fix is to map status 404 in the tableau-api branch to a "this tool is Cloud-only" hint.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a6f6c91e. A 404 in the tableau-api branch now surfaces a Cloud-only hint:

Tableau 404: extract refresh task '' not found. This tool is Tableau Cloud only — verify you're connected to a Cloud site (not Server) and that the taskId came from list-extract-refresh-tasks.

New test 'should map a 404 to a Cloud-only hint instead of the raw status' covers it. Kept it as a runtime hint rather than a hard preflight check since detecting Server-vs-Cloud at the SDK layer is a separate cross-cutting concern.

Akash-Rastogi added a commit that referenced this pull request Jun 18, 2026
Bundles fixes for the 9 review comments on PR #404:

C1 — Permissive update response schema
  updateCloudExtractRefreshTaskResponseSchema previously required a non-
  optional id and a sibling schedule. The Cloud endpoint's exact payload
  varies by site and the destructive e2e leg is gated behind
  UPDATE_CLOUD_EXTRACT_REFRESH_TASK_E2E_ID, so the strict schema risked
  zodios rejecting a successful update and the tool reporting it as a
  failure. Both fields now optional/partial; tasksMethods falls back to
  the requested taskId/schedule so a partial response still surfaces a
  populated ExtractRefreshTask.

C2 — Hourly end>start now compares numerically
  Replaced lexical string comparison with timeToSeconds(). Previously
  '10:00:00' > '9:00:00' was false (because '1' < '9'), so a valid
  9am→10am window would have been wrongly rejected; conversely
  start='10:00:00' end='9:00:00' would have been wrongly accepted.

C3 — Time-format regex + 5-minute boundary refines
  start/end now require zero-padded HH:mm:ss via a regex; minutes must
  be on a 5-minute boundary (00, 05, 10, ..., 55) with seconds=00. Fixes
  the slice-based minute-match (which assumed a fixed string length and
  silently mis-compared mixed formats) and brings the documented "5-min
  boundary" rule under actual schema enforcement instead of waiting on
  a 409004 round-trip.

C4 — Weekly/Monthly intervals now enforced
  Added per-frequency refines: Weekly requires at least one
  intervals.interval[] entry with a weekDay; Monthly requires monthDay.
  Previously documented but unenforced — Tableau caught it via 409004.

C5 — Drop dead extractRefresh.type from request schema
  updateCloudExtractRefreshTaskRequestSchema no longer carries an
  extractRefresh field — the tool exposes no way to set
  FullRefresh/IncrementalRefresh, the SDK method sends only { schedule },
  and the unit test asserts exactly { schedule }. Dropping it removes a
  capability advertisement the tool doesn't back.

R1 — Shared parseTableauApiError helper
  Extracted the axios->{status,code,summary,detail} parsing into
  src/sdks/tableau/tableauApiError.ts so the next mutating tool method
  doesn't copy-paste it again. Body is now parsed via a Zod schema, so
  a non-conforming error body produces null (caller falls back to
  getExceptionMessage) instead of a partly-undefined struct.

R2 — Success message falls back to args for the time window
  If the response omits frequencyDetails, the success message used to
  read "New schedule: Weekly." with no window even when start was set.
  Both start and end now fall back to args.schedule.frequencyDetails.

N1 — No double-colon in Tableau error string
  When Tableau returns code+detail without summary, the formatter no
  longer produces "Tableau 400 [409004]: : Invalid…" — summary/detail
  are joined with a single ": " and the leading colon is conditional.

N2 — 404 mapped to a Cloud-only hint
  A 404 from the Cloud endpoint is now surfaced as "extract refresh task
  '<id>' not found. This tool is Tableau Cloud only — verify you're
  connected to a Cloud site (not Server) and that the taskId came from
  list-extract-refresh-tasks." Most common 404 cause for this tool.

Tests: replaced 4 schema tests with 11 covering zero-padding, 5-minute
boundaries, non-zero seconds, the lexical-vs-numeric comparison gotcha
(both directions), Weekly-without-weekDay, Monthly-without-monthDay.
Added 3 tool-layer tests for the new error formatting (404 hint, no
double-colon, response-omits-frequencyDetails fallback). 1806 unit
tests pass.

Docs: schema-constraints section reframed from "Tableau rejects with
409004" to "schema rejects before any Tableau call" since the rules are
now actually enforced client-side. The 409004 fallback note remains for
site-specific rules outside our refines.
new Err({
type: 'tableau-api',
status: 400,
code: '409004',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MattGPT — automated-assist review on @mattcfilbert's behalf]

@Akash-Rastogi two small doc/test-fidelity nits — runtime is fine, but worth fixing since this is the documented example for a destructiveHint write tool:

1. 409004 is HTTP 409, not 400. Tableau error codes lead with their status (404002→404, all 409xxx→409).

  • The fixtures here (and at ~L313) plus the tool description (409004 Bad Request) pair it with 400.
  • Pass-through handler is correct — a real 409 renders fine; it's only the fixtures + description that drift.
  • Suggest: fixtures → status: 409, assertion → Tableau 409 [409004], description → 409004 Conflict.

2. POST, not PUT (tests/e2e/.../updateCloudExtractRefreshTask.test.ts:9): the header comment says "the actual PUT is blocked…" but the endpoint is method: 'post' in tasksApi.ts (overloading the delete path). One-word fix — and it's the exact subtlety that comment exists to capture.

Otherwise the auth surface looks right: assertAdmin runs before the write, tool's disabled unless admin-tools are on, and the scope wiring (users:read backing the admin lookup) checks out. 👍

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both fixed in 34d49c00. Thanks for the catch (and the auto-assist tone — appreciated).

1. 409004 → 409/Conflict. Updated:

  • tool description: 409004 Bad Request409004 Conflict
  • two test fixtures: status: 400 → 409, summary: 'Bad Request' → 'Conflict', assertion Tableau 400Tableau 409
  • docs page: matching example-output update

Pass-through to error.response.status was always correct so a real 409 would have rendered fine — this was purely fixture/description drift.

2. PUT → POST in tests/e2e/.../updateCloudExtractRefreshTask.test.ts:9 header comment. Done. The exact subtlety the comment was trying to capture, ironically.

Akash-Rastogi added a commit that referenced this pull request Jun 18, 2026
Two doc/test-fidelity nits flagged by mattcfilbert's automated-assist
review on PR #404:

1. 409004 is HTTP 409 (Conflict), not 400 (Bad Request). Tableau error
   codes lead with their status — all 409xxx codes are 409. Updated:
   - tool description: "409004 Bad Request" -> "409004 Conflict"
   - test fixtures: status 400 -> 409, summary "Bad Request" -> "Conflict"
   - assertions: "Tableau 400" -> "Tableau 409"
   - docs page: matching example output update

2. POST, not PUT, in the e2e test header comment. The endpoint is
   `method: 'post'` in tasksApi.ts (overloading the delete path) — the
   header comment described it as PUT. One-word fix.

Pass-through formatting was always correct; this is purely fixture +
description fidelity. 1806 unit tests pass.
},
)
.refine(
(s) => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lines 167-188
Weekly refine L174, Monthly refine L185
The schema adds client-side refines for Weekly (weekDay) and Monthly (monthDay), but Hourly and Daily also require at least one weekDay interval on Cloud. Confirmed live — an Hourly update with intervals:[{hours:1}] and no weekDay
round-trips to Tableau and is rejected:
Tableau 400 [409004]: ...Hourly and Daily schedules must have at least one weekDay interval...
(0x5CE10192 : Provided schedule must have a weekday interval present)
This is the same round-trip-avoidance the Weekly/Monthly refines target. Possible suggest adding a refine covering Hourly and Daily:

.refine(
(s) => {
if (s.frequency !== 'Hourly' && s.frequency !== 'Daily') return true;
return s.frequencyDetails.intervals?.interval.some((i) => i.weekDay !== undefined) ?? false;
},
{
message:
'Hourly and Daily schedules require at least one frequencyDetails.intervals.interval entry with a weekDay',
path: ['frequencyDetails', 'intervals'],
},
)

(Worth confirming the Daily rule against the API on this site before finalizing the message.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a3e8126f. Added a refine that requires a weekDay interval for Hourly and Daily, mirroring the Weekly/Monthly refines: Hourly and Daily schedules require at least one frequencyDetails.intervals.interval entry with a weekDay. Trusted the Tableau error text ("Hourly and Daily schedules must have at least one weekDay interval") for the Daily case rather than narrowing to Hourly-only — happy to relax it to Hourly-only if Daily turns out to be site-specific.

New tests: should reject Hourly schedule without a weekDay interval and should reject Daily schedule without a weekDay interval. Updated 5 pre-existing Hourly/Daily fixtures (accept Hourly with start/end window, accept Daily without end, reject Hourly missing end, reject Hourly mismatched minutes, reject Hourly end-before-start, accept Hourly 09:00–10:00 window) to seed weekDay: 'Monday' so they exercise the rule they intend to test rather than newly tripping the weekDay refine. Tool description and the docs page (Hourly example + constraints) now mention the rule explicitly.

// 404 from Cloud commonly means the tool was called against a Tableau Server
// site or the taskId doesn't exist on this site — surface a Cloud-only hint
// instead of the bare "Not Found".
if (status === 404) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lines 94-99

Nit:
The 404 branch replaces the whole message and omits code (e.g. [404026]) that the generic branch at line 100-103 surfaces. The hint is more actionable, but the underlying Tableau code is no longer visible for 404s. If code-visibility is
desired across all error classes, append it:
if (status === 404) {
const codeStr = code ? [${code}] : '';
return new UnknownError(
Tableau 404${codeStr}: extract refresh task '${args.taskId}' not found. This tool is Tableau Cloud only — verify you're connected to a Cloud site (not Server) and that the taskId came from list-extract-refresh-tasks.,
404,
).toErr();
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a3e8126f. The 404 branch now appends [code] like the generic branch — for a 404001 the message reads Tableau 404 [404001]: extract refresh task '<id>' not found. This tool is Tableau Cloud only — verify…. Code visibility is now consistent across error classes, so a 404026 vs 404001 is still distinguishable. Existing 404 test renamed to should map a 404 to a Cloud-only hint and preserve the Tableau code and now asserts Tableau 404 [404001] literally.

if (parsed) {
return new Err({ type: 'tableau-api', ...parsed });
}
return new Err({ type: 'unknown', message: getExceptionMessage(error) });

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

128-132 + isFiveMinuteBoundary function
Nit:

Input like "6:00:00" fails the timeStringSchema regex AND the 5-minute-boundary refine (because isFiveMinuteBoundary's TIME_REGEX.match returns no groups on the unpadded string, so it reports false). Result is two messages for one root
cause. Harmless; if you want a single clean error, have isFiveMinuteBoundary return true when the format is already invalid (let the regex own the format error):

function isFiveMinuteBoundary(t: string): boolean {
const m = t.match(TIME_REGEX);
if (!m) return true; // format error already reported by timeStringSchema
const [, , minutes, seconds] = m;
return Number(minutes) % 5 === 0 && Number(seconds) === 0;
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a3e8126f (in extractRefreshTask.ts — the comment was anchored to tasksMethods.ts but isFiveMinuteBoundary lives next to the schema). isFiveMinuteBoundary now returns true when the format doesn't match, letting the regex on timeStringSchema own the format error. New test should report only the format error (not the 5-minute-boundary error) for an unpadded time parses '6:00:00' and asserts the issues at frequencyDetails.start have length 1 and contain HH:mm:ss — guards against the double-error regression directly.

Akash-Rastogi and others added 4 commits June 19, 2026 15:51
Adds the Tableau Cloud extract refresh schedule update tool (1B-2 in
the 264 admin-MCP breakdown), sibling to delete-extract-refresh-task.
Lets admins change frequency, time window, and recurrence intervals
of an existing scheduled extract refresh without recreating the task.

Tool: src/tools/web/extractRefreshTasks/updateCloudExtractRefreshTask.ts
- Admin-gated, single-phase, destructive + idempotent.
- Calls the Cloud variant POST /sites/{id}/tasks/extractRefreshes/{id}
  with `extractRefresh` and `schedule` as siblings in the body.
- Surfaces Tableau's response error code/summary/detail (e.g.
  409004 Invalid subscription schedule. Schedule minute must be on a
  5-minute boundary) so callers can recover from server-side
  validation rejections without parsing raw axios stacks.

SDK: src/sdks/tableau/{apis,methods,types}
- POST endpoint in tasksApi.ts.
- updateCloudExtractRefreshTask method returns
  Result<ExtractRefreshTask, UpdateCloudExtractRefreshTaskError>
  mirroring the viewsMethods.handleQueryImageError pattern.
  Discriminated error: { type: 'tableau-api', status, code?, summary?,
  detail? } | { type: 'unknown', message }.
- updateCloudExtractRefreshScheduleSchema closes frequency to
  Hourly|Daily|Weekly|Monthly. end is optional but enforced for
  Hourly via chained .refine() (presence + minute alignment +
  strictly-after-start).

Wiring: webToolNames + tasks group, factory in tools.ts, JwtScopes,
scopes.ts adds tableau:mcp:tasks:write and tableau:tasks:write,
admin-gate disabled list.

Tests: 11 unit tests for the tool (registration, annotations, admin
gate, happy path, Tableau-typed Err formatting, unknown Err fallback,
disabled-when-admin-off, schema-validation refines), 3 SDK
method-level tests (sibling-merge response shape, request body
pass-through, axios->tableau-api Err mapping), e2e schema-validation
with opt-in destructive leg gated on
UPDATE_CLOUD_EXTRACT_REFRESH_TASK_E2E_ID, scopes.test.ts toggle tests
for the new write scopes. 1797 unit tests pass.

Docs: per-tool page at docs/docs/tools/tasks/update-cloud-extract-
refresh-task.md (with schedule constraints — 5-minute boundary,
Hourly minute alignment, Weekly weekDay, Monthly monthDay, sample
requests for each frequency); intro.md row + reference link;
env-vars.md ADMIN_TOOLS_ENABLED list now includes all 11 admin-only
tools.

Tableau API gotchas captured for future reviewers:
- Update Cloud uses POST not PUT, same path as delete.
- Body uses extractRefresh + schedule as siblings, not nested.
- Response is also siblings; method merges them.
- start (and end when present) must be on a 5-minute boundary.
- 409004 is the catch-all error code for schedule validation.
- Docs live in rest_api_ref_extract_and_encryption.htm
  (despite being about schedules).

Version bump: 2.15.0 -> 2.16.0 (minor; new feature).
Bundles fixes for the 9 review comments on PR #404:

C1 — Permissive update response schema
  updateCloudExtractRefreshTaskResponseSchema previously required a non-
  optional id and a sibling schedule. The Cloud endpoint's exact payload
  varies by site and the destructive e2e leg is gated behind
  UPDATE_CLOUD_EXTRACT_REFRESH_TASK_E2E_ID, so the strict schema risked
  zodios rejecting a successful update and the tool reporting it as a
  failure. Both fields now optional/partial; tasksMethods falls back to
  the requested taskId/schedule so a partial response still surfaces a
  populated ExtractRefreshTask.

C2 — Hourly end>start now compares numerically
  Replaced lexical string comparison with timeToSeconds(). Previously
  '10:00:00' > '9:00:00' was false (because '1' < '9'), so a valid
  9am→10am window would have been wrongly rejected; conversely
  start='10:00:00' end='9:00:00' would have been wrongly accepted.

C3 — Time-format regex + 5-minute boundary refines
  start/end now require zero-padded HH:mm:ss via a regex; minutes must
  be on a 5-minute boundary (00, 05, 10, ..., 55) with seconds=00. Fixes
  the slice-based minute-match (which assumed a fixed string length and
  silently mis-compared mixed formats) and brings the documented "5-min
  boundary" rule under actual schema enforcement instead of waiting on
  a 409004 round-trip.

C4 — Weekly/Monthly intervals now enforced
  Added per-frequency refines: Weekly requires at least one
  intervals.interval[] entry with a weekDay; Monthly requires monthDay.
  Previously documented but unenforced — Tableau caught it via 409004.

C5 — Drop dead extractRefresh.type from request schema
  updateCloudExtractRefreshTaskRequestSchema no longer carries an
  extractRefresh field — the tool exposes no way to set
  FullRefresh/IncrementalRefresh, the SDK method sends only { schedule },
  and the unit test asserts exactly { schedule }. Dropping it removes a
  capability advertisement the tool doesn't back.

R1 — Shared parseTableauApiError helper
  Extracted the axios->{status,code,summary,detail} parsing into
  src/sdks/tableau/tableauApiError.ts so the next mutating tool method
  doesn't copy-paste it again. Body is now parsed via a Zod schema, so
  a non-conforming error body produces null (caller falls back to
  getExceptionMessage) instead of a partly-undefined struct.

R2 — Success message falls back to args for the time window
  If the response omits frequencyDetails, the success message used to
  read "New schedule: Weekly." with no window even when start was set.
  Both start and end now fall back to args.schedule.frequencyDetails.

N1 — No double-colon in Tableau error string
  When Tableau returns code+detail without summary, the formatter no
  longer produces "Tableau 400 [409004]: : Invalid…" — summary/detail
  are joined with a single ": " and the leading colon is conditional.

N2 — 404 mapped to a Cloud-only hint
  A 404 from the Cloud endpoint is now surfaced as "extract refresh task
  '<id>' not found. This tool is Tableau Cloud only — verify you're
  connected to a Cloud site (not Server) and that the taskId came from
  list-extract-refresh-tasks." Most common 404 cause for this tool.

Tests: replaced 4 schema tests with 11 covering zero-padding, 5-minute
boundaries, non-zero seconds, the lexical-vs-numeric comparison gotcha
(both directions), Weekly-without-weekDay, Monthly-without-monthDay.
Added 3 tool-layer tests for the new error formatting (404 hint, no
double-colon, response-omits-frequencyDetails fallback). 1806 unit
tests pass.

Docs: schema-constraints section reframed from "Tableau rejects with
409004" to "schema rejects before any Tableau call" since the rules are
now actually enforced client-side. The 409004 fallback note remains for
site-specific rules outside our refines.
Two doc/test-fidelity nits flagged by mattcfilbert's automated-assist
review on PR #404:

1. 409004 is HTTP 409 (Conflict), not 400 (Bad Request). Tableau error
   codes lead with their status — all 409xxx codes are 409. Updated:
   - tool description: "409004 Bad Request" -> "409004 Conflict"
   - test fixtures: status 400 -> 409, summary "Bad Request" -> "Conflict"
   - assertions: "Tableau 400" -> "Tableau 409"
   - docs page: matching example output update

2. POST, not PUT, in the e2e test header comment. The endpoint is
   `method: 'post'` in tasksApi.ts (overloading the delete path) — the
   header comment described it as PUT. One-word fix.

Pass-through formatting was always correct; this is purely fixture +
description fidelity. 1806 unit tests pass.
…e, 404 code, single-error on bad format)
@Akash-Rastogi Akash-Rastogi force-pushed the akashrastogi/update-cloud-extract-refresh-task branch from a3e8126 to 405bd62 Compare June 19, 2026 22:55
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