docs: update license terminology and improve error messages in docume…#125
docs: update license terminology and improve error messages in docume…#125mrdailey99 merged 2 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Updates licensing terminology and clarifies user-facing messaging across the CLI and MCP documentation, plus adds an API key expiry warning to the sf provar auth status command.
Changes:
- Standardize “license” terminology (code comments/logs + docs) and update related phrasing.
- Make auth/login flow errors more provider-agnostic (remove Cognito-specific wording).
- Enhance
sf provar auth statusoutput to warn when an API key is near expiry/expired; expand Claude Desktop Windows config guidance (Store vs direct installer).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/services/auth/loginFlow.ts | Updates callback-port comment and token/callback error messages to be provider-agnostic. |
| src/mcp/licensing/licenseValidator.ts | Standardizes “license” wording in JSDoc and log message. |
| src/commands/provar/auth/status.ts | Adds expiry/expiration warning output when expires_at is present. |
| docs/mcp.md | Adds Windows Store config path and explanatory note for Claude Desktop on Windows. |
| docs/mcp-pilot-guide.md | Standardizes “license” wording; updates prerequisites table and related sections. |
| CLAUDE.md | Updates docs requirement row to use “license” terminology. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (stored.expires_at) { | ||
| this.log(` Expires: ${stored.expires_at}`); | ||
| const expiresMs = new Date(stored.expires_at).getTime(); | ||
| const daysLeft = Math.ceil((expiresMs - Date.now()) / (1000 * 60 * 60 * 24)); | ||
| if (daysLeft <= 14 && daysLeft > 0) { | ||
| this.log(''); | ||
| this.log(` Warning: API key expires in ${daysLeft} day${daysLeft === 1 ? '' : 's'}.`); | ||
| this.log(" Run 'sf provar auth rotate' now to avoid CI/CD disruption."); | ||
| } else if (daysLeft <= 0) { | ||
| this.log(''); | ||
| this.log(' Warning: API key has expired. Run: sf provar auth login'); | ||
| } |
There was a problem hiding this comment.
New expiry warning output (<=14 days and expired) isn’t covered by existing auth status tests. Adding NUT/unit coverage that seeds expires_at (and fixes Date.now() via a stub/fake timer) would prevent regressions in the warning thresholds and pluralization.
|
|
||
| // All three ports must be pre-registered in the Cognito App Client. | ||
| // Cognito requires redirect_uri to exactly match a registered callback URL — no wildcards. | ||
| // All three ports must be pre-registered in both the Cognito App Client and the SF ECA. |
There was a problem hiding this comment.
The new comment references “SF ECA” without expanding the acronym. For maintainability, consider spelling out the full name on first mention (e.g., “Salesforce <…> (ECA)”) so future readers know what needs to be pre-registered.
| // All three ports must be pre-registered in both the Cognito App Client and the SF ECA. | |
| // All three ports must be pre-registered in both the Cognito App Client and the Salesforce ECA (SF ECA). |
| | Requirement | Version | Notes | | ||
| | --------------------------- | ------- | ------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | Provar Automation IDE | ≥ 2.x | Must be installed with an **activated licence** on the same machine. The MCP server reads the licence from `~/Provar/.licenses/`. | | ||
| | Provar Automation IDE | ≥ 3.x | Must be installed with an **activated license** on the same machine. The MCP server reads the license from `~/Provar/.licenses/`. | |
There was a problem hiding this comment.
This guide now states Provar Automation IDE version "≥ 3.x". No other docs in the repo specify this higher minimum (e.g. docs/mcp.md only requires an activated IDE license), so this introduces an inconsistency for users. Either align the other docs to the same requirement or add a brief note explaining why IDE 2.x is no longer supported.
| ``` | ||
|
|
||
| > **Licence:** The server reads your Provar Automation IDE licence automatically from `~/Provar/.licenses/`. No extra configuration is required — just ensure Provar Automation IDE is installed and activated on this machine. | ||
| > **license:** The server reads your Provar Automation IDE license automatically from `~/Provar/.licenses/`. No extra configuration is required — just ensure Provar Automation IDE is installed and activated on this machine. |
There was a problem hiding this comment.
The callout label is now > **license:** (lowercase), but the rest of the document’s callouts use title case (e.g. > **Tip:**, > **Windows:**). Consider changing this to > **License:** for consistency and readability.
| > **license:** The server reads your Provar Automation IDE license automatically from `~/Provar/.licenses/`. No extra configuration is required — just ensure Provar Automation IDE is installed and activated on this machine. | |
| > **License:** The server reads your Provar Automation IDE license automatically from `~/Provar/.licenses/`. No extra configuration is required — just ensure Provar Automation IDE is installed and activated on this machine. |
| const daysLeft = Math.ceil((expiresMs - Date.now()) / (1000 * 60 * 60 * 24)); | ||
| if (daysLeft <= 14 && daysLeft > 0) { | ||
| this.log(''); | ||
| this.log(` Warning: API key expires in ${daysLeft} day${daysLeft === 1 ? '' : 's'}.`); | ||
| this.log(" Run 'sf provar auth rotate' now to avoid CI/CD disruption."); | ||
| } else if (daysLeft <= 0) { | ||
| this.log(''); | ||
| this.log(' Warning: API key has expired. Run: sf provar auth login'); |
There was a problem hiding this comment.
stored.expires_at is parsed with new Date(...).getTime() without validating the result. If the stored value is missing timezone info or otherwise not parseable, expiresMs becomes NaN and the warning logic silently becomes a no-op (and can make future refactors error-prone). Consider guarding with Number.isFinite(expiresMs) and either skipping the days-left calculation or printing a dedicated message when the expiry timestamp is invalid.
| const daysLeft = Math.ceil((expiresMs - Date.now()) / (1000 * 60 * 60 * 24)); | |
| if (daysLeft <= 14 && daysLeft > 0) { | |
| this.log(''); | |
| this.log(` Warning: API key expires in ${daysLeft} day${daysLeft === 1 ? '' : 's'}.`); | |
| this.log(" Run 'sf provar auth rotate' now to avoid CI/CD disruption."); | |
| } else if (daysLeft <= 0) { | |
| this.log(''); | |
| this.log(' Warning: API key has expired. Run: sf provar auth login'); | |
| if (Number.isFinite(expiresMs)) { | |
| const daysLeft = Math.ceil((expiresMs - Date.now()) / (1000 * 60 * 60 * 24)); | |
| if (daysLeft <= 14 && daysLeft > 0) { | |
| this.log(''); | |
| this.log(` Warning: API key expires in ${daysLeft} day${daysLeft === 1 ? '' : 's'}.`); | |
| this.log(" Run 'sf provar auth rotate' now to avoid CI/CD disruption."); | |
| } else if (daysLeft <= 0) { | |
| this.log(''); | |
| this.log(' Warning: API key has expired. Run: sf provar auth login'); | |
| } | |
| } else { | |
| this.log(''); | |
| this.log(' Warning: API key expiry timestamp is invalid.'); |
…ntation