Skip to content

NEW @W-21910054@ Message sanitization - #500

Merged
nikhil-mittal-165 merged 3 commits into
devfrom
message-sanitization
Aug 18, 2026
Merged

NEW @W-21910054@ Message sanitization#500
nikhil-mittal-165 merged 3 commits into
devfrom
message-sanitization

Conversation

@nikhil-mittal-165

Copy link
Copy Markdown
Contributor

No description provided.

@git2gus

git2gus Bot commented Aug 17, 2026

Copy link
Copy Markdown

Git2Gus App is installed but the .git2gus/config.json doesn't have right values. You should add the required configuration.

@aruntyagiTutu aruntyagiTutu left a comment

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.

The message-sanitization changes themselves look right — dropping the leaked SFAP API returned ... prefix from formatHttpError, rewording the auth-failure message to be more actionable, retitling the INVALID_SESSION/SCAN_TIMEOUT copy to be consistent with the wording already shipped in sfdx-code-analyzer-vscode#368, and the new formatSeconds() helper for human-readable timeout messages are all reasonable and covered by updated tests.

However, packages/code-analyzer-apexguru-engine/src/config.ts also changes the default api_timeout_ms from 300000 to 3000 while leaving the // 5 minutes comment unchanged:

-    api_timeout_ms: 300000,         // 5 minutes
+    api_timeout_ms: 3000,         // 5 minutes

This looks like an unintentional typo (dropped two zeros) rather than a deliberate change — the PR description is empty and gives no rationale, the title/scope is "Message sanitization," and nothing else in the diff references timeout tuning. As written, this drops the default ApexGuru scan timeout from 5 minutes to 3 seconds, which would cause real scans to time out almost immediately for anyone relying on the default. It isn't caught by ApexGuruEngine.test.ts because that test hardcodes 300000 explicitly rather than exercising DEFAULT_APEXGURU_ENGINE_CONFIG.

Could you confirm whether this was intentional? If not, please restore 300000. If it was intentional, it should be called out explicitly in the PR description and probably split out from the sanitization change.

@aruntyagiTutu aruntyagiTutu left a comment

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.

Thanks for the fix — confirmed the timeout default is now 600000 (10 minutes), with the comment, config description, and test assertion ("600 seconds") all updated consistently. That resolves my earlier concern about the accidental 3-second default. LGTM.

@nikhil-mittal-165
nikhil-mittal-165 merged commit 4a1c535 into dev Aug 18, 2026
9 checks passed
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