diff --git a/packages/code-analyzer-apexguru-engine/package.json b/packages/code-analyzer-apexguru-engine/package.json index 73d66378..9dcaa50c 100644 --- a/packages/code-analyzer-apexguru-engine/package.json +++ b/packages/code-analyzer-apexguru-engine/package.json @@ -1,7 +1,7 @@ { "name": "@salesforce/code-analyzer-apexguru-engine", "description": "ApexGuru Engine Package for the Salesforce Code Analyzer", - "version": "0.42.0", + "version": "0.43.0-SNAPSHOT", "author": "The Salesforce Code Analyzer Team", "license": "BSD-3-Clause", "homepage": "https://developer.salesforce.com/docs/platform/salesforce-code-analyzer/overview", diff --git a/packages/code-analyzer-apexguru-engine/src/apexguru-rules.ts b/packages/code-analyzer-apexguru-engine/src/apexguru-rules.ts index c24dab68..b0da6033 100644 --- a/packages/code-analyzer-apexguru-engine/src/apexguru-rules.ts +++ b/packages/code-analyzer-apexguru-engine/src/apexguru-rules.ts @@ -1,8 +1,6 @@ -import { RuleDescription, SeverityLevel } from '@salesforce/code-analyzer-engine-api'; - -export const DEV_PREVIEW_TAG_APEXGURU: string = 'DevPreviewApexGuru'; +import { COMMON_TAGS, RuleDescription, SeverityLevel } from '@salesforce/code-analyzer-engine-api'; /** * Known ApexGuru rules with descriptions and metadata. @@ -18,7 +16,7 @@ export const APEXGURU_RULES: RuleDescription[] = [ { name: 'SoqlInALoop', severityLevel: SeverityLevel.High, - tags: [DEV_PREVIEW_TAG_APEXGURU], + tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.PERFORMANCE], description: 'SOQL query inside a loop causes performance issues and can hit governor limits', resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_soql_in_loop.htm&type=5'] }, @@ -26,7 +24,7 @@ export const APEXGURU_RULES: RuleDescription[] = [ { name: 'DmlInALoop', severityLevel: SeverityLevel.High, - tags: [DEV_PREVIEW_TAG_APEXGURU], + tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.PERFORMANCE], description: 'DML statement inside a loop causes performance issues and can hit governor limits', resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_dml_in_loop.htm&type=5'] }, @@ -38,7 +36,7 @@ export const APEXGURU_RULES: RuleDescription[] = [ { name: 'SoqlInALoopOneHop', severityLevel: SeverityLevel.High, - tags: [DEV_PREVIEW_TAG_APEXGURU], + tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.PERFORMANCE], description: 'SOQL query reached one method-hop away inside a loop causes performance issues and can hit governor limits', resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_soql_in_loop_one_hop.htm&type=5'] }, @@ -46,7 +44,7 @@ export const APEXGURU_RULES: RuleDescription[] = [ { name: 'ExpensiveMethods', severityLevel: SeverityLevel.High, - tags: [DEV_PREVIEW_TAG_APEXGURU], + tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.PERFORMANCE], description: 'Method accounts for a large share of observed Apex CPU time and is a hotspot for performance work', resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_expensive_methods.htm&type=5'] }, @@ -58,7 +56,7 @@ export const APEXGURU_RULES: RuleDescription[] = [ { name: 'SoqlWithoutAWhereClauseOrLimitStatement', severityLevel: SeverityLevel.Moderate, - tags: [DEV_PREVIEW_TAG_APEXGURU], + tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.PERFORMANCE], description: 'SOQL query without WHERE clause or LIMIT statement can cause performance issues and heap size exceptions', resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_soql_without_where_clause_or_limit_statement.htm&type=5'] }, @@ -66,7 +64,7 @@ export const APEXGURU_RULES: RuleDescription[] = [ { name: 'SoqlWithWildcardFilter', severityLevel: SeverityLevel.Moderate, - tags: [DEV_PREVIEW_TAG_APEXGURU], + tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.PERFORMANCE], description: 'SOQL query using LIKE with leading wildcard is inefficient and cannot use indexes', resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_soql_with_wildcard_filter.htm&type=5'] }, @@ -74,7 +72,7 @@ export const APEXGURU_RULES: RuleDescription[] = [ { name: 'SchemaGetGlobalDescribeNotEfficient', severityLevel: SeverityLevel.Moderate, - tags: [DEV_PREVIEW_TAG_APEXGURU], + tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.PERFORMANCE], description: 'Using Schema.getGlobalDescribe() causes unnecessary overhead and decreases performance', resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_schema_getglobaldescribe_not_efficient.htm&type=5'] }, @@ -86,7 +84,7 @@ export const APEXGURU_RULES: RuleDescription[] = [ { name: 'Soql Aggregation', severityLevel: SeverityLevel.Moderate, - tags: [DEV_PREVIEW_TAG_APEXGURU], + tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.PERFORMANCE], description: 'Manual aggregation in Apex instead of using SOQL aggregate functions causes performance issues', resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_aggregating_in_apex.htm&type=5'] }, @@ -94,7 +92,7 @@ export const APEXGURU_RULES: RuleDescription[] = [ { name: 'SoqlWithApexFilter', severityLevel: SeverityLevel.Moderate, - tags: [DEV_PREVIEW_TAG_APEXGURU], + tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.PERFORMANCE], description: 'Filtering records in Apex instead of using SOQL WHERE clause causes performance issues', resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_soql_with_apex_filter.htm&type=5'] }, @@ -102,7 +100,7 @@ export const APEXGURU_RULES: RuleDescription[] = [ { name: 'CopyingListOrSetElementsUsingAForLoop', severityLevel: SeverityLevel.Moderate, - tags: [DEV_PREVIEW_TAG_APEXGURU], + tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.PERFORMANCE], description: 'Copying list or set elements using a for loop is inefficient - use addAll() instead', resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_copying_elements_with_for_loop.htm&type=5'] }, @@ -110,7 +108,7 @@ export const APEXGURU_RULES: RuleDescription[] = [ { name: 'Redundant Soql', severityLevel: SeverityLevel.Moderate, - tags: [DEV_PREVIEW_TAG_APEXGURU], + tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.PERFORMANCE], description: 'Multiple identical SOQL queries cause unnecessary database round trips', resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_redundant_soql.htm&type=5'] }, @@ -118,7 +116,7 @@ export const APEXGURU_RULES: RuleDescription[] = [ { name: 'SoqlWithNegativeExpressions', severityLevel: SeverityLevel.Moderate, - tags: [DEV_PREVIEW_TAG_APEXGURU], + tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.PERFORMANCE], description: 'SOQL queries using negative expressions (NOT IN, !=) don\'t use indexes and cause full table scans', resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_soql_with_negative_expressions.htm&type=5'] }, @@ -126,7 +124,7 @@ export const APEXGURU_RULES: RuleDescription[] = [ { name: 'SObjectMapInAForLoop', severityLevel: SeverityLevel.Moderate, - tags: [DEV_PREVIEW_TAG_APEXGURU], + tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.PERFORMANCE], description: 'Building Map using .put() in a for loop is inefficient - use map constructor or putAll()', resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_sobject_map_in_for_loop.htm&type=5'] }, @@ -134,7 +132,7 @@ export const APEXGURU_RULES: RuleDescription[] = [ { name: 'SoqlWithoutPlatformCache', severityLevel: SeverityLevel.Moderate, - tags: [DEV_PREVIEW_TAG_APEXGURU], + tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.PERFORMANCE], description: 'Frequently executed SOQL query whose results could be served from Platform Cache to reduce database load', resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_soql_without_platform_cache.htm&type=5'] }, @@ -146,7 +144,7 @@ export const APEXGURU_RULES: RuleDescription[] = [ { name: 'LimitsGetHeapsizeMethods', severityLevel: SeverityLevel.Low, - tags: [DEV_PREVIEW_TAG_APEXGURU], + tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.PERFORMANCE], description: 'Frequent Limits.getHeapSize() calls add runtime overhead', resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_limits_getheapsize_methods.htm&type=5'] }, @@ -154,7 +152,7 @@ export const APEXGURU_RULES: RuleDescription[] = [ { name: 'ExpensiveStringComparison', severityLevel: SeverityLevel.Low, - tags: [DEV_PREVIEW_TAG_APEXGURU], + tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.PERFORMANCE], description: 'Inefficient string comparison wastes CPU time', resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_expensive_string_comparison.htm&type=5'] }, @@ -162,7 +160,7 @@ export const APEXGURU_RULES: RuleDescription[] = [ { name: 'ExpensiveDebugStatements', severityLevel: SeverityLevel.Low, - tags: [DEV_PREVIEW_TAG_APEXGURU], + tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.PERFORMANCE], description: 'Expensive System.debug() statements add runtime overhead', resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_expensive_debug_statements.htm&type=5'] }, @@ -174,7 +172,7 @@ export const APEXGURU_RULES: RuleDescription[] = [ { name: 'UsingTheTestMethodKeyword', severityLevel: SeverityLevel.Low, - tags: [DEV_PREVIEW_TAG_APEXGURU], + tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.PERFORMANCE], description: 'The testMethod keyword is deprecated - use @isTest annotation instead', resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_test_case_antipattern_using_testmethod.htm&type=5'] }, @@ -186,7 +184,7 @@ export const APEXGURU_RULES: RuleDescription[] = [ { name: 'SortingInApex', severityLevel: SeverityLevel.Low, - tags: [DEV_PREVIEW_TAG_APEXGURU], + tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.PERFORMANCE], description: 'Sorting records in Apex wastes CPU time and can exceed governor limits - use ORDER BY in SOQL', resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_sorting_in_apex.htm&type=5'] }, @@ -194,7 +192,7 @@ export const APEXGURU_RULES: RuleDescription[] = [ { name: 'BusyLoopDelay', severityLevel: SeverityLevel.Low, - tags: [DEV_PREVIEW_TAG_APEXGURU], + tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.PERFORMANCE], description: 'Using empty loops to delay execution wastes CPU time - use System.enqueueJob with delay parameter', resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_busy_loop_delay.htm&type=5'] }, @@ -202,7 +200,7 @@ export const APEXGURU_RULES: RuleDescription[] = [ { name: 'SoqlWithUnusedFields', severityLevel: SeverityLevel.Low, - tags: [DEV_PREVIEW_TAG_APEXGURU], + tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.PERFORMANCE], description: 'SOQL query selecting unused fields increases resource consumption unnecessarily', resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_soql_with_unused_fields.htm&type=5'] }, @@ -210,7 +208,7 @@ export const APEXGURU_RULES: RuleDescription[] = [ { name: 'WritingFillerStatements', severityLevel: SeverityLevel.Low, - tags: [DEV_PREVIEW_TAG_APEXGURU], + tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.PERFORMANCE], description: 'Filler statements written to inflate code coverage instead of testing real behavior', resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_test_case_antipattern_filler_statements.htm&type=5'] }, @@ -222,7 +220,7 @@ export const APEXGURU_RULES: RuleDescription[] = [ { name: 'apexguru-other', severityLevel: SeverityLevel.Moderate, - tags: [DEV_PREVIEW_TAG_APEXGURU], + tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.PERFORMANCE], description: 'Other ApexGuru rules - covers new rules added by Salesforce that are not yet explicitly declared', resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru.htm'] } diff --git a/packages/code-analyzer-apexguru-engine/src/engine.ts b/packages/code-analyzer-apexguru-engine/src/engine.ts index 0d5518b7..085737bc 100644 --- a/packages/code-analyzer-apexguru-engine/src/engine.ts +++ b/packages/code-analyzer-apexguru-engine/src/engine.ts @@ -94,8 +94,15 @@ export class ApexGuruEngine extends EngineEventEmitter implements Engine { } catch (error) { const detail = error instanceof Error ? error.message : String(error); this.apexGuruService.cleanup(); + if (this.isInvalidSessionError(error)) { + return this.skipWithError('INVALID_SESSION', + 'ApexGuru skipped the scan because your org session is invalid or expired. (401 Unauthorized: invalid session).\n' + + 'Re-authenticate: sf org login web\n' + + 'Then run the scan again.', + ''); + } return this.skipWithError('NO_ORG_CONNECTION', - `Failed to authenticate: ${detail}`, + detail, "Please authenticate with 'sf org login web' or pass --target-org"); } @@ -195,10 +202,16 @@ export class ApexGuruEngine extends EngineEventEmitter implements Engine { } catch (error) { // Catch API failures (5xx, timeout, connection refused) and unexpected errors const detail = error instanceof Error ? error.message : String(error); + if (this.isScanTimeoutError(error)) { + return this.skipWithError('SCAN_TIMEOUT', + `Code Analyzer skipped ApexGuru scan because the workspace scan timed out after ${this.config.api_timeout_ms} ms.`, + ''); + } if (this.isApiUnavailableError(error)) { return this.skipWithError('API_UNAVAILABLE', - `ApexGuru service is unavailable: ${detail}`, - 'The ApexGuru service is temporarily unavailable. Please try again later.'); + 'Code Analyzer skipped ApexGuru scan because the service is unavailable right now. ' + + 'Try again later. If the issue persists, contact Salesforce Support.', + ''); } return this.skipWithError('UNEXPECTED_ERROR', `An unexpected error occurred: ${detail}`, @@ -216,7 +229,7 @@ export class ApexGuruEngine extends EngineEventEmitter implements Engine { * this may be invoked from within a try-finally that already handles cleanup. */ private skipWithError(code: string, message: string, remediation: string): EngineRunResults { - this.emitLogEvent(LogLevel.Warn, `ApexGuru skipped: ${message}`); + this.emitLogEvent(LogLevel.Warn, message); this.emitRunRulesProgressEvent(100); return { violations: [], @@ -227,6 +240,25 @@ export class ApexGuruEngine extends EngineEventEmitter implements Engine { }; } + /** + * Determines whether an error is an invalid/expired org session (401 Unauthorized). + * Surfaces from the Org JWT minting step during initialize() when the org access + * token is no longer valid. + */ + private isInvalidSessionError(error: unknown): boolean { + if (!(error instanceof Error)) return false; + const msg = error.message.toLowerCase(); + return msg.includes('401') || msg.includes('invalid session') || msg.includes('session id'); + } + + /** + * Determines whether an error is the workspace scan timeout raised by ApexGuruService + * when the scan exceeds the configured api_timeout_ms. + */ + private isScanTimeoutError(error: unknown): boolean { + return error instanceof Error && error.message.includes('Workspace scan timed out'); + } + /** * Determines whether an error is an API unavailability issue (network/timeout/5xx). */ diff --git a/packages/code-analyzer-apexguru-engine/src/services/ApexGuruAuthService.ts b/packages/code-analyzer-apexguru-engine/src/services/ApexGuruAuthService.ts index 443256db..f695e64c 100644 --- a/packages/code-analyzer-apexguru-engine/src/services/ApexGuruAuthService.ts +++ b/packages/code-analyzer-apexguru-engine/src/services/ApexGuruAuthService.ts @@ -60,10 +60,11 @@ export class ApexGuruAuthService { const errorMessage = err instanceof Error ? err.message : String(err); this.emitLogEvent(LogLevel.Fine, `Failed to authenticate: No default org found: ${errorMessage}`); throw new Error( - 'No default org found. Please either:\n' + + 'Code Analyzer skipped ApexGuru scan because no default org is set. ' + + 'To continue, do one of the following:\n' + ' 1. Set a default org: sf config set target-org \n' + - ' 2. Pass --target-org flag: sf code-analyzer run --target-org ...\n' + - ' 3. Authenticate to an org: sf org login web' + ' 2. Run the scan with a target org: sf code-analyzer run --target-org ...\n' + + ' 3. Authenticate to your org: sf org login web' ); } } diff --git a/packages/code-analyzer-apexguru-engine/src/services/ApexGuruService.ts b/packages/code-analyzer-apexguru-engine/src/services/ApexGuruService.ts index 52d318c9..cdda1abf 100644 --- a/packages/code-analyzer-apexguru-engine/src/services/ApexGuruService.ts +++ b/packages/code-analyzer-apexguru-engine/src/services/ApexGuruService.ts @@ -8,7 +8,8 @@ import { ApexGuruPollResponse, ApexGuruResponseStatus, ApexGuruScanMetadata, - ApexGuruViolation + ApexGuruViolation, + ApexGuruOrgResolveResponse } from '../types'; import * as fs from 'node:fs'; import * as path from 'node:path'; @@ -159,16 +160,64 @@ export class ApexGuruService { }); } + /** + * Resolve the caller's org to its production org id via the org-resolve + * Connect API (POST /services/data/v{version}/apexguru/org/resolve). The + * returned productionOrgId is forwarded as an optional `productionOrgId` + * multipart form field on the SFAP scan submit call. Uses the org access + * token (not the Org JWT), + * since this is a standard Salesforce org REST endpoint. + */ + private async resolveProductionOrgId(): Promise { + const accessToken = this.authService.getAccessToken(); + const instanceUrl = this.authService.getInstanceUrl(); + const apiVersion = this.authService.getApiVersion(); + const url = `${instanceUrl}/services/data/v${apiVersion}/apexguru/org/resolve`; + + try { + const response = await fetch(url, { + method: 'POST', + headers: { + 'Accept': 'application/json', + 'Authorization': `Bearer ${accessToken}` + } + }); + + if (!response.ok) { + const errorText = await response.text(); + throw new Error(formatHttpError(response.status, errorText)); + } + + const resolveResponse = await response.json() as ApexGuruOrgResolveResponse; + + if (!resolveResponse.productionOrgId) { + throw new Error('Org resolve response missing productionOrgId field'); + } + return resolveResponse.productionOrgId; + } catch (error) { + // The productionOrgId is optional on the scan submit call, so a failure here is non-fatal. + // Log a warning and return an empty string so the scan can proceed without it. + const errorMessage = error instanceof Error ? error.message : String(error); + this.emitLogEvent(LogLevel.Warn, `Failed to resolve production org id: ${errorMessage}`); + return ''; + } + } + /** * Submit workspace zip to SFAP API */ private async submitScan(zipBuffer: Buffer): Promise { const orgJwt = await this.authService.mintOrgJwt(); + const productionOrgId = await this.resolveProductionOrgId(); const url = `${this.sfapBaseUrl}/apex-guru/scan`; const form = new FormData(); form.append('file', zipBuffer, { filename: 'project.zip', contentType: 'application/zip' }); form.append('analysisModeHint', 'full'); + // productionOrgId is optional — only include it when the org-resolve call returned one. + if (productionOrgId) { + form.append('productionOrgId', productionOrgId); + } try { const response = await fetch(url, { diff --git a/packages/code-analyzer-apexguru-engine/src/types/index.ts b/packages/code-analyzer-apexguru-engine/src/types/index.ts index 37164205..1afd8b40 100644 --- a/packages/code-analyzer-apexguru-engine/src/types/index.ts +++ b/packages/code-analyzer-apexguru-engine/src/types/index.ts @@ -89,6 +89,20 @@ export type ApexGuruSuggestion = { message: string; // Code suggestion }; +/** + * Response from the ApexGuru org-resolve endpoint + * (POST /services/data/v{version}/apexguru/org/resolve). + * Resolves the caller's org to its production org, whose id is + * forwarded as the `productionOrgId` multipart form field on the SFAP scan submit call. + */ +export type ApexGuruOrgResolveResponse = { + fullCopySandboxOrgIds: string[]; + inputOrgId: string; + message: string | null; + productionOrgId: string; + status: string; +}; + /** * Response from the Org JWT minting endpoint (POST /ide/auth) * Used to authenticate against the SFAP ApexGuru API diff --git a/packages/code-analyzer-apexguru-engine/test/ApexGuruAuthService.test.ts b/packages/code-analyzer-apexguru-engine/test/ApexGuruAuthService.test.ts index eda9b01e..2fee6f79 100644 --- a/packages/code-analyzer-apexguru-engine/test/ApexGuruAuthService.test.ts +++ b/packages/code-analyzer-apexguru-engine/test/ApexGuruAuthService.test.ts @@ -66,7 +66,7 @@ describe('ApexGuruAuthService', () => { await expect(authService.initialize({})) .rejects - .toThrow('No default org found'); + .toThrow('Code Analyzer skipped ApexGuru scan because no default org is set'); }); }); diff --git a/packages/code-analyzer-apexguru-engine/test/ApexGuruEngine.test.ts b/packages/code-analyzer-apexguru-engine/test/ApexGuruEngine.test.ts index 9c18817e..66d52b71 100644 --- a/packages/code-analyzer-apexguru-engine/test/ApexGuruEngine.test.ts +++ b/packages/code-analyzer-apexguru-engine/test/ApexGuruEngine.test.ts @@ -207,7 +207,7 @@ describe('ApexGuruEngine', () => { }); expect(logSpy).toHaveBeenCalledWith( LogLevel.Warn, - expect.stringContaining('Failed to authenticate') + expect.stringContaining('No default org found') ); expect(mockApexGuruService.cleanup).toHaveBeenCalled(); expect(progressSpy).toHaveBeenCalledWith(100); @@ -215,6 +215,34 @@ describe('ApexGuruEngine', () => { expect(mockApexGuruService.scanWorkspace).not.toHaveBeenCalled(); }); + it('should gracefully skip with INVALID_SESSION when the org session is expired (401)', async () => { + mockApexGuruService.initialize.mockRejectedValue(new Error( + 'Org JWT minting failed: Failed to mint Org JWT: 401 Unauthorized. Response: {"message":"Invalid session ID sent, unable to establish login session"}' + )); + mockWorkspace.getTargetedFiles.mockResolvedValue(['/test/workspace/Test.cls']); + const logSpy = jest.spyOn(engine as any, 'emitLogEvent'); + + const results = await engine.runRules(['SoqlInALoop'], mockRunOptions); + + expect(results.violations).toEqual([]); + expect(results.insights).toEqual({ + status: 'skipped', + error: { + code: 'INVALID_SESSION', + message: 'ApexGuru skipped the scan because your org session is invalid or expired. (401 Unauthorized: invalid session).\n' + + 'Re-authenticate: sf org login web\n' + + 'Then run the scan again.', + remediation: '' + } + }); + expect(logSpy).toHaveBeenCalledWith( + LogLevel.Warn, + expect.stringContaining('your org session is invalid or expired') + ); + expect(mockApexGuruService.cleanup).toHaveBeenCalled(); + expect(mockApexGuruService.scanWorkspace).not.toHaveBeenCalled(); + }); + it('should gracefully skip with API_UNAVAILABLE when API is unreachable', async () => { mockWorkspace.getTargetedFiles.mockResolvedValue(['/test/workspace/Test.cls']); mockApexGuruService.scanWorkspace.mockRejectedValue(new Error('connect ECONNREFUSED 127.0.0.1:443')); @@ -227,13 +255,14 @@ describe('ApexGuruEngine', () => { status: 'skipped', error: { code: 'API_UNAVAILABLE', - message: expect.stringContaining('ECONNREFUSED'), - remediation: expect.stringContaining('temporarily unavailable') + message: 'Code Analyzer skipped ApexGuru scan because the service is unavailable right now. ' + + 'Try again later. If the issue persists, contact Salesforce Support.', + remediation: '' } }); expect(logSpy).toHaveBeenCalledWith( LogLevel.Warn, - expect.stringContaining('unavailable') + expect.stringContaining('the service is unavailable right now') ); expect(mockApexGuruService.cleanup).toHaveBeenCalled(); }); @@ -250,13 +279,34 @@ describe('ApexGuruEngine', () => { status: 'skipped', error: { code: 'API_UNAVAILABLE', - message: expect.stringContaining('timeout'), - remediation: expect.stringContaining('try again later') + message: 'Code Analyzer skipped ApexGuru scan because the service is unavailable right now. ' + + 'Try again later. If the issue persists, contact Salesforce Support.', + remediation: '' } }); expect(logSpy).toHaveBeenCalledWith(LogLevel.Warn, expect.any(String)); }); + it('should gracefully skip with SCAN_TIMEOUT when the workspace scan times out', async () => { + mockWorkspace.getTargetedFiles.mockResolvedValue(['/test/workspace/Test.cls']); + mockApexGuruService.scanWorkspace.mockRejectedValue(new Error('Workspace scan timed out after 300000ms')); + const logSpy = jest.spyOn(engine as any, 'emitLogEvent'); + + const results = await engine.runRules(['SoqlInALoop'], mockRunOptions); + + expect(results.violations).toEqual([]); + expect(results.insights).toEqual({ + status: 'skipped', + error: { + code: 'SCAN_TIMEOUT', + message: 'Code Analyzer skipped ApexGuru scan because the workspace scan timed out after 300000 ms.', + remediation: '' + } + }); + expect(logSpy).toHaveBeenCalledWith(LogLevel.Warn, expect.stringContaining('workspace scan timed out after 300000 ms')); + expect(mockApexGuruService.cleanup).toHaveBeenCalled(); + }); + it('should gracefully skip with UNEXPECTED_ERROR on non-network errors', async () => { mockWorkspace.getTargetedFiles.mockResolvedValue(['/test/workspace/Test.cls']); mockApexGuruService.scanWorkspace.mockRejectedValue(new Error('Unexpected internal failure')); @@ -603,8 +653,8 @@ describe('ApexGuruEngine', () => { }); }); - describe('DevPreview behavior', () => { - it('rules list should show DevPreview tag on all rules and no Recommended tag', async () => { + describe('Rule tagging behavior', () => { + it('rules list should no longer carry the DevPreview tag', async () => { const rules = await engine.describeRules({ logFolder: '/tmp/logs', workingFolder: '/tmp/working' @@ -612,8 +662,7 @@ describe('ApexGuruEngine', () => { expect(rules.length).toBeGreaterThan(0); for (const rule of rules) { - expect(rule.tags).toContain('DevPreviewApexGuru'); - expect(rule.tags).not.toContain('Recommended'); + expect(rule.tags).not.toContain('DevPreviewApexGuru'); } }); @@ -627,14 +676,24 @@ describe('ApexGuruEngine', () => { expect(engine.getName()).toBe('apexguru'); }); - it('should describe rules selectable by DevPreview tag', async () => { + it('should describe rules selectable by the Recommended tag', async () => { + const rules = await engine.describeRules({ + logFolder: '/tmp/logs', + workingFolder: '/tmp/working' + }); + + const recommendedRules = rules.filter(r => r.tags.includes(COMMON_TAGS.RECOMMENDED)); + expect(recommendedRules).toHaveLength(rules.length); + }); + + it('should describe rules selectable by the Performance tag', async () => { const rules = await engine.describeRules({ logFolder: '/tmp/logs', workingFolder: '/tmp/working' }); - const devPreviewRules = rules.filter(r => r.tags.includes('DevPreviewApexGuru')); - expect(devPreviewRules).toHaveLength(rules.length); + const performanceRules = rules.filter(r => r.tags.includes(COMMON_TAGS.CATEGORIES.PERFORMANCE)); + expect(performanceRules).toHaveLength(rules.length); }); it('should describe individual rules by name for explicit selection', async () => { diff --git a/packages/code-analyzer-apexguru-engine/test/ApexGuruService.test.ts b/packages/code-analyzer-apexguru-engine/test/ApexGuruService.test.ts index aa383825..33e86d46 100644 --- a/packages/code-analyzer-apexguru-engine/test/ApexGuruService.test.ts +++ b/packages/code-analyzer-apexguru-engine/test/ApexGuruService.test.ts @@ -1,3 +1,4 @@ +import { LogLevel } from '@salesforce/code-analyzer-engine-api'; import { ApexGuruService } from '../src/services/ApexGuruService'; import { ApexGuruAuthService } from '../src/services/ApexGuruAuthService'; import { ApexGuruResponseStatus } from '../src/types'; @@ -8,10 +9,28 @@ jest.mock('archiver'); jest.mock('node:fs'); const TEST_SFAP_BASE_URL = 'https://api.salesforce.com/platform/scale/v1-beta.1'; +const TEST_INSTANCE_URL = 'https://test.salesforce.com'; +const TEST_API_VERSION = '64.0'; +const TEST_PRODUCTION_ORG_ID = '00D5g000005FGSeEAO'; const mockFetch = jest.fn(); globalThis.fetch = mockFetch as unknown as typeof globalThis.fetch; +// Standard success response for the org-resolve Connect API, which is called +// before every scan submit to obtain the production org id. +function mockResolveResponse(): void { + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => ({ + fullCopySandboxOrgIds: [], + inputOrgId: '00DC4000003sEyjMAE', + message: null, + productionOrgId: TEST_PRODUCTION_ORG_ID, + status: 'SUCCESS' + }) + } as any); +} + describe('ApexGuruService', () => { let apexGuruService: ApexGuruService; let mockEmitLogEvent: jest.Mock; @@ -109,6 +128,9 @@ describe('ApexGuruService', () => { report_generated_ms: 1234567890 }; + // Mock resolve response + mockResolveResponse(); + // Mock submit response mockFetch.mockResolvedValueOnce({ ok: true, @@ -143,7 +165,7 @@ describe('ApexGuruService', () => { expect(result.violations).toEqual(mockViolations); expect(result.scanMetadata).toEqual(mockScanMetadata); expect(result.analysisMode).toBe('full'); - expect(mockFetch).toHaveBeenCalledTimes(2); // submit + poll + expect(mockFetch).toHaveBeenCalledTimes(3); // resolve + submit + poll }); it('should poll multiple times until success', async () => { @@ -158,6 +180,9 @@ describe('ApexGuruService', () => { } ]; + // Mock resolve response + mockResolveResponse(); + // Mock submit response mockFetch.mockResolvedValueOnce({ ok: true, @@ -208,10 +233,13 @@ describe('ApexGuruService', () => { const result = await apexGuruService.scanWorkspace(mockWorkspaceRoot, mockPathsToZip); expect(result.violations).toEqual(mockViolations); - expect(mockFetch).toHaveBeenCalledTimes(3); // submit + 2 polls + expect(mockFetch).toHaveBeenCalledTimes(4); // resolve + submit + 2 polls }); it('should throw error when scan fails', async () => { + // Mock resolve response + mockResolveResponse(); + // Mock submit response mockFetch.mockResolvedValueOnce({ ok: true, @@ -246,6 +274,9 @@ describe('ApexGuruService', () => { }); it('should throw error when submit fails', async () => { + // Mock resolve response + mockResolveResponse(); + mockFetch.mockResolvedValueOnce({ ok: false, status: 401, @@ -256,7 +287,64 @@ describe('ApexGuruService', () => { .rejects.toThrow('Failed to submit scan'); }); + it('should continue scan without productionOrgId when org resolve fails', async () => { + // Mock failed resolve response - this is non-fatal, scan should proceed without productionOrgId + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 403, + text: async () => 'Forbidden' + } as any); + + // Mock successful submit + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => ({ + scanId: 'scan-no-org', + status: ApexGuruResponseStatus.QUEUED, + analysisMode: 'full', + createdMs: Date.now() + }) + } as any); + + // Mock successful poll + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => ({ + scanId: 'scan-no-org', + status: ApexGuruResponseStatus.SUCCEEDED, + analysisMode: 'full', + createdMs: Date.now(), + updatedMs: Date.now(), + processingStartMs: Date.now(), + processingEndMs: Date.now(), + scanMetadata: null, + report: null, + reportS3Key: null, + message: null + }) + } as any); + + const result = await apexGuruService.scanWorkspace(mockWorkspaceRoot, mockPathsToZip); + + // Scan should succeed despite the org-resolve failure + expect(result.analysisMode).toBe('full'); + expect(mockFetch).toHaveBeenCalledTimes(3); // resolve (failed) + submit + poll + + // A warning should be logged about the failed org resolve + expect(mockEmitLogEvent).toHaveBeenCalledWith( + LogLevel.Warn, + expect.stringContaining('Failed to resolve production org id') + ); + + // productionOrgId should be omitted from the submit form body + const submitBody = (mockFetch.mock.calls[1][1] as any).body as Buffer; + expect(submitBody.toString()).not.toContain('name="productionOrgId"'); + }); + it('should throw error when poll returns HTTP error', async () => { + // Mock resolve response + mockResolveResponse(); + // Mock successful submit mockFetch.mockResolvedValueOnce({ ok: true, @@ -280,6 +368,9 @@ describe('ApexGuruService', () => { }); it('should handle empty violation list', async () => { + // Mock resolve response + mockResolveResponse(); + // Mock submit response mockFetch.mockResolvedValueOnce({ ok: true, @@ -322,6 +413,9 @@ describe('ApexGuruService', () => { }); it('should handle null report gracefully', async () => { + // Mock resolve response + mockResolveResponse(); + // Mock submit response mockFetch.mockResolvedValueOnce({ ok: true, @@ -360,6 +454,9 @@ describe('ApexGuruService', () => { it('should timeout if scan takes too long', async () => { jest.useFakeTimers(); + // Mock resolve response + mockResolveResponse(); + // Mock submit response mockFetch.mockResolvedValueOnce({ ok: true, @@ -405,6 +502,9 @@ describe('ApexGuruService', () => { const mockProgressCallback = jest.fn(); apexGuruService.setProgressCallback(mockProgressCallback); + // Mock resolve response + mockResolveResponse(); + // Mock submit response mockFetch.mockResolvedValueOnce({ ok: true, @@ -442,6 +542,9 @@ describe('ApexGuruService', () => { }); it('should use correct SFAP API endpoints', async () => { + // Mock resolve response + mockResolveResponse(); + mockFetch.mockResolvedValueOnce({ ok: true, json: async () => ({ @@ -471,9 +574,21 @@ describe('ApexGuruService', () => { await apexGuruService.scanWorkspace(mockWorkspaceRoot, mockPathsToZip); - // Check submit endpoint + // Check org-resolve endpoint expect(mockFetch).toHaveBeenNthCalledWith( 1, + `${TEST_INSTANCE_URL}/services/data/v${TEST_API_VERSION}/apexguru/org/resolve`, + expect.objectContaining({ + method: 'POST', + headers: expect.objectContaining({ + 'Authorization': 'Bearer test-token' + }) + }) + ); + + // Check submit endpoint + expect(mockFetch).toHaveBeenNthCalledWith( + 2, `${TEST_SFAP_BASE_URL}/apex-guru/scan`, expect.objectContaining({ method: 'POST', @@ -484,9 +599,14 @@ describe('ApexGuruService', () => { }) ); + // productionOrgId is sent as a multipart form field in the body, not a header + const submitBody = (mockFetch.mock.calls[1][1] as any).body as Buffer; + expect(submitBody.toString()).toContain('name="productionOrgId"'); + expect(submitBody.toString()).toContain(TEST_PRODUCTION_ORG_ID); + // Check poll endpoint expect(mockFetch).toHaveBeenNthCalledWith( - 2, + 3, `${TEST_SFAP_BASE_URL}/apex-guru/scan/scan-endpoint-check`, expect.objectContaining({ method: 'GET', diff --git a/packages/code-analyzer-apexguru-engine/test/apexguru-rules.test.ts b/packages/code-analyzer-apexguru-engine/test/apexguru-rules.test.ts index 3ae6b909..d9082b15 100644 --- a/packages/code-analyzer-apexguru-engine/test/apexguru-rules.test.ts +++ b/packages/code-analyzer-apexguru-engine/test/apexguru-rules.test.ts @@ -1,69 +1,24 @@ -import { APEXGURU_RULES, DEV_PREVIEW_TAG_APEXGURU, isKnownRule, FALLBACK_RULE_NAME } from '../src/apexguru-rules'; +import { COMMON_TAGS } from '@salesforce/code-analyzer-engine-api'; +import { APEXGURU_RULES, isKnownRule, FALLBACK_RULE_NAME } from '../src/apexguru-rules'; describe('apexguru-rules', () => { - describe('DEV_PREVIEW_TAG_APEXGURU constant', () => { - it('should equal DevPreviewApexGuru', () => { - expect(DEV_PREVIEW_TAG_APEXGURU).toBe('DevPreviewApexGuru'); - }); - }); - - describe('7 rules that previously had RECOMMENDED tag', () => { - const rulesWithFormerRecommended = [ - 'SoqlInALoop', - 'DmlInALoop', - 'SoqlWithoutAWhereClauseOrLimitStatement', - 'SoqlWithWildcardFilter', - 'SchemaGetGlobalDescribeNotEfficient', - 'UsingTheTestMethodKeyword', - 'apexguru-other' - ]; - - it.each(rulesWithFormerRecommended)('%s should have exactly [DevPreviewApexGuru] as its tags', (ruleName) => { - const rule = APEXGURU_RULES.find(r => r.name === ruleName); - expect(rule).toBeDefined(); - expect(rule!.tags).toEqual(['DevPreviewApexGuru']); - }); - }); - - describe('16 rules without former RECOMMENDED tag', () => { - const rulesWithoutFormerRecommended = [ - 'SoqlInALoopOneHop', - 'ExpensiveMethods', - 'Soql Aggregation', - 'SoqlWithApexFilter', - 'CopyingListOrSetElementsUsingAForLoop', - 'Redundant Soql', - 'SoqlWithNegativeExpressions', - 'SObjectMapInAForLoop', - 'SoqlWithoutPlatformCache', - 'LimitsGetHeapsizeMethods', - 'ExpensiveStringComparison', - 'ExpensiveDebugStatements', - 'SortingInApex', - 'BusyLoopDelay', - 'SoqlWithUnusedFields', - 'WritingFillerStatements' - ]; - - it.each(rulesWithoutFormerRecommended)('%s should have exactly [DevPreviewApexGuru] as its tags', (ruleName) => { - const rule = APEXGURU_RULES.find(r => r.name === ruleName); - expect(rule).toBeDefined(); - expect(rule!.tags).toEqual(['DevPreviewApexGuru']); - }); - }); - describe('aggregate invariants across all 23 rules', () => { it('should have exactly 23 rules', () => { expect(APEXGURU_RULES).toHaveLength(23); }); - it('every rule should have exactly [DevPreviewApexGuru] as its tags', () => { + it('every rule should carry exactly the Recommended and Performance tags', () => { for (const rule of APEXGURU_RULES) { - expect(rule.tags).toEqual(['DevPreviewApexGuru']); + expect(rule.tags).toEqual([COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.PERFORMANCE]); } }); + it('no rule should carry the removed DevPreviewApexGuru tag', () => { + for (const rule of APEXGURU_RULES) { + expect(rule.tags).not.toContain('DevPreviewApexGuru'); + } + }); }); describe('isKnownRule', () => {