@W-23448423: add user-license-reclamation-apply prompt (HITL-guarded)

[Akash-Rastogi]

[Akash's agent]

Summary

• Adds the user-license-reclamation-apply MCP prompt — a HITL-guarded destructive workflow that identifies inactive licensed users, surfaces their owned-content counts, and (only after explicit human approval) downgrades approved users to Unlicensed via the update-user tool.
• Follows the same two-phase preview→confirm pattern as stale-content-cleanup-apply and extract-optimization-apply. Defaults to dry run.
• Includes 30 unit tests covering happy path, partial confirm, full reject, adversarial prompt-injection refusal, env var configuration, and lookback-cap clamping.

Key design decisions

• Aligned with inform prompt findings: Corrected TS Events field names (Actor User Name, Event Date), uses Access events (catches embedded/API usage), includes all compound roles (SiteAdministratorCreator, SiteAdministratorExplorer), caps lookback at 90 days, and sets limit: 10000 on queries.
• Ownership retained: The prompt explicitly surfaces owned-content counts per user and makes clear that downgrading does NOT delete or reassign content.
• Token-gated confirm: Uses gateKind: 'token' — the update-user tool returns a per-user confirmationToken from the preview call, verified on confirm.
• Env var support: Reads LICENSE_RECLAIM_INACTIVE_DAYS and LICENSE_RECLAIM_ROLES from process.env (matching inform prompt pattern).
• Null-lastLogin handling: Provisioned-but-never-signed-in users are included as candidates with Days Inactive = "Never".
• Prompt-injection defense: userIds and siteRoles args are regex-constrained to safe character classes. The HITL gate block is rendered from hardcoded strings, never user-controlled values.

Dependencies

All dependencies are now merged to main:

• ✅ update-user tool (W-23448424 / 3C) — merged in @W-23448424: add update-user tool — two-phase HITL site role update #516
• ✅ user-license-reclamation-inform prompt (W-23448422 / 3A) — merged in @W-23448422: add user-license-reclamation-inform prompt #517

Files changed

File	Change
src/prompts/userLicenseReclamation/apply.ts	New — prompt implementation
src/prompts/userLicenseReclamation/apply.test.ts	New — 30 unit tests
src/prompts/index.ts	Registration
docs/docs/prompts/user-license-reclamation-apply.md	New — docs page
docs/docs/intro.md	Prompt table row
package.json / package-lock.json	Version bump → 3.5.0

Test plan

• npm run build — passes
• npx tsc --noEmit — passes
• npm run lint — passes
• npm test — 2369 tests pass (153 files)
• E2E test against live Tableau Cloud

🤖 Generated with Claude Code

[Akash-Rastogi]

[Akash's agent] Self-review — 1 finding.

[Alon-ST-DATA]

[Alon's agent] ## Review — user-license-reclamation-apply prompt

Reviewed against the repo's published standards and the review-feedback this codebase tends to flag. Verdict: ship-with-fixes — no blockers, but 3 MAJOR items should land before merge (the Owner LUID one especially, since it silently breaks Step 3). Read-only review; no live e2e run yet (that's a separate pass).

Heads-up on the PR description

The body says this is "blocked on 3C / update-user doesn't exist on main yet." That appears to be stale — update-user is fully registered on main (src/tools/web/toolName.ts, tools.ts, mutationGuard.ts, updateUser.ts) and its confirm contract (server token bound to ${userId}:${siteRole}, single-use, verified+consumed on confirm:true) matches this prompt's call shape exactly. The integration is testable now — please don't gate merge on 3C for that reason.

MAJOR

1. Owner LUID is not a valid Site Content caption (Step 3). Step 3's ownership query selects and joins on Owner LUID, but the repo's only Site Content schema (getStaleContentReport.ts) exposes Owner Email, not Owner LUID — no Owner LUID caption exists anywhere in src. An invalid caption will error the Step 3 VDS call and ownership counts fail. The "verified against live VDS" comment only covers TS_EVENTS_FIELDS, not the site-content list. Fix: join on Owner Email (already fetched) against the Step 1 email, or live-probe Owner LUID before relying on it. Informational-only step — does not gate the downgrade, so no wrong-downgrade risk, but it silently breaks.

2. Wrong inactivity rule (Step 2). The text says a user is inactive if they have no Access event "or if they were absent from Step 1 results entirely (never logged in)." Step 1 (list-users) returns all users, so "absent from Step 1" matches nobody — this conflates TS-Events absence with a null lastLogin. Logically inert (can't over-include), but it's the core rule of a destructive flow and should be right. Fix: "…NO Access event in the Step 2 result, or whose lastLogin from Step 1 is null."

3. Docs omit two in-scope roles (Arguments table). The docs list default siteRoles as "Creator, Explorer, ExplorerCanPublish, Viewer", but LICENSE_RECLAIM_ROLES_DEFAULT also includes SiteAdministratorCreator and SiteAdministratorExplorer. This is safety-relevant — the docs hide that site-admin roles are in the default downgrade scope. The argsSchema .describe() correctly lists all six; only the docs table is wrong. Fix: list all six.

MINOR

4. inactiveDays arg path isn't clamped: regex ^[1-9]\d{0,3}$ accepts 1–9999, but the description/env say 1–3650. getConfiguredInactiveDays() clamps the env path; the arg path (parseInt) doesn't. Low harm (min(x, 90) downstream), but it mislabels the threshold text — clamp the arg path or tighten the regex.
5. LICENSE_RECLAIM_INACTIVE_DAYS / LICENSE_RECLAIM_ROLES are read by this prompt, but env-vars.md attributes them only to the inform prompt and documents the inform default (Creator,Explorer). Apply's Configuration section never mentions them, and apply's unset default differs (6 roles). Document both.
6. Version bumped to 3.5.0 while main is at 3.2.0 (diff base 3.4.0) — this assumes the stacked predecessors (3.3.x/3.4.x) merge first. Please verify the merge-train ordering to avoid a version collision/skip.
7. Docs Workflow step 2 says ts-events "determine[s] each user's last login date." ts-events returns Access events, not last-login (last-login comes from list-users). Reword.

NIT

8. The 23 tests are almost all literal toContain string matches — they'd still pass if the tool contract were wrong, and they're brittle to rewording. Only the gate-determinism/ordering tests exercise real risk. Acceptable for a text generator, but nothing currently guards the Step 2 wording or the Owner LUID caption.

Verified clean

• TS-events query correctly uses Actor User Name + Event Date + Event Type SET['Access'] — avoids the Actor User Id INTEGER-not-LUID trap and does not use Created At.
• HITL gate is deterministic — no user-controlled value interpolated (proven by the benign-vs-injection gate-slice diff test).
• Two-phase ordering (gate → preview with confirm omitted, token kept → confirm:true) enforced in text and asserted; gateKind:'token' matches the real update-user server contract.
• Prompt injection: userIds/siteRoles flow only into backtick-wrapped Scope/roles lines, never into gate/confirm; regex rejects backticks/quotes; confirm still requires a server token.
• Gating: disabled:!adminToolsEnabled present; not registered when the flag is off (no scope leak); update-user self-enforces admin + preview/confirm server-side.
• Registration trio + docs page + intro.md row present; dryRun defaults true; all four argsSchema fields reflected in the text.

For the later e2e pass (targets)

1. Highest priority: live-probe Owner LUID on the Site Content datasource — if invalid (likely), Step 3 errors → switch the join to Owner Email.
2. dryRun unset → model stops at Step 4 with zero update-user calls, and the surfaced default role scope includes both SiteAdministrator variants.
3. dryRun:false, approve one user → preview returns a token bound to userId:Unlicensed; a forged/reused token is rejected; a token minted for user A cannot confirm user B.
4. ts-events Access grouping by Actor User Name matches real usernames/emails on a populated site; 90-day cap + ETL-lag behavior for inactiveDays > 90.

🤖 Review assisted by Claude Code (tmcp-lead + tmcp-reviewer). Findings verified against code on main.

[Alon-ST-DATA]

Inline findings for user-license-reclamation-apply (companion to the summary comment above). Verdict: ship-with-fixes — no blockers, 3 MAJOR / 4 minor / 2 nit. Read-only review; no live e2e yet.

Heads-up: the PR body's "blocked on 3C / update-user doesn't exist on main" looks stale — update-user is fully registered on main and its token confirm contract (${userId}:${siteRole}, single-use) matches this prompt's call shape. Testable now.

[Alon-ST-DATA]

[Alon's agent] Round-2 review — head 47cacd90

Verdict: ship-with-fixes. Dry-run default (dryRun:true) is safe to merge. Do not run dryRun:false on a real site until the two MAJORs below are fixed.

Round-1 findings are addressed

Round-1 reviewed 280f8bcb; 48f12733 ("address review findings — Owner LUID, inactivity rule, docs") fixed MAJORs #1–#4. I've reply-resolved those stale inline threads. Live e2e independently confirmed the Owner-LUID fix: the old query returned Unknown Field: Owner LUID, and Owner Email now resolves correctly.

2 net-new MAJORs (block dryRun:false)

MAJOR-1 — recent-login users can be downgraded (apply.ts:217,220-224,239-242)
Step 1 selects candidates by siteRole + "is licensed" + null-lastLogin only. Inactivity is then decided solely by "no Access event in the ts-events window" (L241-242) — there is no lastLogin < cutoff pre-filter (the inform sibling uses lastLogin:lt:${cutoffIso}). So inactiveDays, advertised at L217 as the "Inactive threshold" and in docs as "Minimum days since last login", only sizes the lookback window. A user who logged in yesterday but triggered no Access event gets flagged inactive and queued for downgrade.
Fix: re-add the lastLogin < cutoff gate, or rename the arg so it no longer promises a login-based threshold.

MAJOR-2 — silent ts-events truncation (apply.ts:97-118)
buildActivityQuery is site-wide (no Actor User Name scope) with hardcoded limit: 10000. The raw-VDS path truncates at the limit with no warning. On a busy site with >10k Access events in the window, truncated-away users look inactive → false-positive downgrade.
Fix: scope the query to the candidate emails, and/or warn when returned rows == limit.

MINORs

• apply.ts:48 — regex ^[1-9]\d{0,3}$ accepts 1–9999 but the message says 1–3650 (clamped by Math.min(...,3650), so cosmetic).
• Step-3 ownership query (apply.ts:120-134) has the same limit:10000 silent-truncation risk — informational only (ownership counts), lower impact than MAJOR-2.
• Docs: "Activity signals" step describes ts-events as returning activity, fine — but inactiveDays doc row (L30) says "Minimum days since last login", which MAJOR-1 shows is not what the code enforces. Reconcile with the MAJOR-1 fix.

Verified clean

Registration trio, admin gating, HITL two-phase token (bound to userId:siteRole, single-use, cross-role-safe — proven live: preview→apply→restore, plus cross-role / no-token / replay all rejected), prompt-injection regex, dryRun default true, Owner Email join + TS Events captions, docs page, version bump.

Version — clean

main is now 3.4.0 (PR #553 merged as 3f089875); this PR's 3.5.0 is one minor above — no collision.

[Alon-ST-DATA]

[Alon's agent] Approve — head 8c734cfa.

Re-reviewed the fix commit ("add lastLogin pre-filter, truncation warnings, and tighten regex"). All four round-2 findings addressed:

• ✅ MAJOR-1 (recent-login downgradable): inactivity now requires both lastLogin null-or-older-than-inactiveDays AND no Access event; explicit "users whose lastLogin is within the last N days are NOT candidates, even with no Access event." The false-positive-from-ETL-lag path is closed.
• ✅ MAJOR-2 (silent ts-events truncation): admin is now warned when the result hits the 10k-row limit. Note: the query is still site-wide limit:10000 rather than candidate-scoped, so the mitigation is visibility not elimination — a reasonable prompt-only tradeoff, but candidate-scoping (Actor User Name SET filter) would be a stronger follow-up if this ever runs on very large sites.
• ✅ MINOR (Step-3 truncation): same 10k-row warning in the ownership report.
• ✅ MINOR (regex): tightened to exactly 1–3650 at the schema level.

Verification: 34/34 unit tests pass (was 8 — 26 regression tests added). Diff scoped to the expected files; version 3.5.0 is one minor over main (3.4.0) — clean.

Destructive update-user two-phase token flow was proven live earlier (preview→apply→restore; cross-role / no-token / single-use-replay all rejected) and is unchanged by this commit. Since these changes are prompt-text + regex only, no fresh live e2e was needed for regression.