diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 04639c09..4420d9c5 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -21,6 +21,18 @@ jobs: strategy: matrix: os: [ubuntu-latest, windows-latest] + # Split the NUTs across jobs so each group runs on its own runner (its own + # egress IP). The `login rate exceeded` OAuth cap is per-IP, and running the + # whole suite serially on one IP exhausts the budget before run-eval. run-eval + # is isolated on its own job for maximum headroom; the z0-z5 pipeline stays in + # one job because its tests share one org in create->...->trace order. + command: + - 'yarn test:nuts:run-eval' + - 'yarn test:nuts:test' + - 'yarn test:nuts:generate' + - 'yarn test:nuts:z' + - 'yarn test:nuts:adl-mcp' fail-fast: false with: os: ${{ matrix.os }} + command: ${{ matrix.command }} diff --git a/README.md b/README.md index 69129dc5..53b78b6c 100644 --- a/README.md +++ b/README.md @@ -2498,3 +2498,5 @@ EXAMPLES _See code: [src/commands/agent/validate/authoring-bundle.ts](https://github.com/salesforcecli/plugin-agent/blob/2.2.3/src/commands/agent/validate/authoring-bundle.ts)_ + + diff --git a/package.json b/package.json index f63aa1a1..a6f3f40e 100644 --- a/package.json +++ b/package.json @@ -139,6 +139,11 @@ "prepare": "sf-install", "test": "wireit", "test:nuts": "nyc mocha \"test/nuts/**/*.nut.ts\" --slow 4500 --timeout 600000 --reporter-options maxDiffSize=15000 --exit", + "test:nuts:run-eval": "nyc mocha \"test/nuts/agent.test.run-eval.nut.ts\" --slow 4500 --timeout 600000 --reporter-options maxDiffSize=15000 --exit", + "test:nuts:test": "nyc mocha \"test/nuts/agent.test.nut.ts\" \"test/nuts/agent.test.create.nut.ts\" --slow 4500 --timeout 600000 --reporter-options maxDiffSize=15000 --exit", + "test:nuts:generate": "nyc mocha \"test/nuts/agent.activate.nut.ts\" \"test/nuts/agent.generate.*.nut.ts\" \"test/nuts/template.nut.ts\" --slow 4500 --timeout 600000 --reporter-options maxDiffSize=15000 --exit", + "test:nuts:z": "nyc mocha \"test/nuts/z*.nut.ts\" --slow 4500 --timeout 600000 --reporter-options maxDiffSize=15000 --exit", + "test:nuts:adl-mcp": "nyc mocha \"test/nuts/agent.adl.nut.ts\" \"test/nuts/agent.mcp.nut.ts\" --slow 4500 --timeout 600000 --reporter-options maxDiffSize=15000 --exit", "test:only": "wireit", "version": "oclif readme" }, diff --git a/test/nuts/agent.test.run-eval.nut.ts b/test/nuts/agent.test.run-eval.nut.ts index 8c73b42c..3533be5b 100644 --- a/test/nuts/agent.test.run-eval.nut.ts +++ b/test/nuts/agent.test.run-eval.nut.ts @@ -29,55 +29,65 @@ describe('agent test run-eval', function () { const yamlSpecPath = join(mockProjectDir, 'eval-test-spec.yaml'); const yamlWithContextPath = join(mockProjectDir, 'eval-with-context.yaml'); + // The JSON-payload run-eval is expensive (~18 min) and hammers the shared admin user's + // OAuth refresh rate limit when invoked repeatedly. Run it exactly once here and share the + // parsed result across every test that asserts against it, so a rate-limit failure surfaces + // once rather than per-test. + let jsonResult: RunEvalResult; + before(async function () { this.timeout(30 * 60 * 1000); // 30 minutes for setup await getTestSession(); + + const jsonCommand = `agent test run-eval --spec ${jsonPayloadPath} --api-name Local_Info_Agent --target-org ${getUsername()} --json`; + const jsonExec = execCmd(jsonCommand, { ensureExitCode: 0 }); + expect(jsonExec.jsonOutput?.result).to.be.ok; + jsonResult = jsonExec.jsonOutput!.result; }); describe('run-eval with JSON file', () => { it('should run evaluation with JSON payload file', async () => { - const command = `agent test run-eval --spec ${jsonPayloadPath} --api-name Local_Info_Agent --target-org ${getUsername()} --json`; - const result = execCmd(command, { ensureExitCode: 0 }); - - expect(result.jsonOutput?.result).to.be.ok; - expect(result.jsonOutput?.result.tests).to.be.an('array'); - expect(result.jsonOutput?.result.tests.length).to.be.greaterThan(0); - expect(result.jsonOutput?.result.summary).to.be.ok; - expect(result.jsonOutput?.result.summary.passed).to.be.a('number'); - expect(result.jsonOutput?.result.summary.failed).to.be.a('number'); - expect(result.jsonOutput?.result.summary.scored).to.be.a('number'); - expect(result.jsonOutput?.result.summary.errors).to.be.a('number'); + expect(jsonResult).to.be.ok; + expect(jsonResult.tests).to.be.an('array'); + expect(jsonResult.tests.length).to.be.greaterThan(0); + expect(jsonResult.summary).to.be.ok; + expect(jsonResult.summary.passed).to.be.a('number'); + expect(jsonResult.summary.failed).to.be.a('number'); + expect(jsonResult.summary.scored).to.be.a('number'); + expect(jsonResult.summary.errors).to.be.a('number'); }); it('should run evaluation with normalized payload', async () => { - const command = `agent test run-eval --spec ${jsonPayloadPath} --api-name Local_Info_Agent --target-org ${getUsername()} --json`; - const result = execCmd(command, { ensureExitCode: 0 }); - - expect(result.jsonOutput?.result.tests[0]).to.be.ok; - expect(result.jsonOutput?.result.tests[0].id).to.equal('test-topic-routing'); - expect(result.jsonOutput?.result.tests[0].status).to.be.oneOf(['passed', 'failed']); - expect(result.jsonOutput?.result.tests[0].evaluations).to.be.an('array'); + expect(jsonResult.tests[0]).to.be.ok; + expect(jsonResult.tests[0].id).to.equal('test-topic-routing'); + expect(jsonResult.tests[0].status).to.be.oneOf(['passed', 'failed']); + expect(jsonResult.tests[0].evaluations).to.be.an('array'); }); }); describe('run-eval with YAML file', () => { - it('should run evaluation with YAML test spec file', async () => { + // The YAML-spec run-eval is run once and shared across the two tests that assert against it, + // for the same OAuth-rate-limit reason as the JSON-payload run above. + let yamlResult: RunEvalResult; + + before(() => { const command = `agent test run-eval --spec ${yamlSpecPath} --target-org ${getUsername()} --json`; - const result = execCmd(command, { ensureExitCode: 0 }); + const output = execCmd(command, { ensureExitCode: 0 }); + expect(output.jsonOutput?.result).to.be.ok; + yamlResult = output.jsonOutput!.result; + }); - expect(result.jsonOutput?.result).to.be.ok; - expect(result.jsonOutput?.result.tests).to.be.an('array'); - expect(result.jsonOutput?.result.tests.length).to.be.greaterThan(0); - expect(result.jsonOutput?.result.summary).to.be.ok; + it('should run evaluation with YAML test spec file', async () => { + expect(yamlResult).to.be.ok; + expect(yamlResult.tests).to.be.an('array'); + expect(yamlResult.tests.length).to.be.greaterThan(0); + expect(yamlResult.summary).to.be.ok; }); it('should auto-infer agent name from YAML subjectName', async () => { - const command = `agent test run-eval --spec ${yamlSpecPath} --target-org ${getUsername()} --json`; - const result = execCmd(command, { ensureExitCode: 0 }); - // Should succeed without explicit --api-name flag - expect(result.jsonOutput?.result).to.be.ok; - expect(result.jsonOutput?.result.tests).to.be.an('array'); + expect(yamlResult).to.be.ok; + expect(yamlResult.tests).to.be.an('array'); }); it('should handle YAML spec with contextVariables', async () => { @@ -173,11 +183,8 @@ describe('agent test run-eval', function () { describe('run-eval output structure', () => { it('should include test summaries with correct structure', async () => { - const command = `agent test run-eval --spec ${jsonPayloadPath} --api-name Local_Info_Agent --target-org ${getUsername()} --json`; - const result = execCmd(command, { ensureExitCode: 0 }); - - expect(result.jsonOutput?.result.tests).to.be.an('array'); - const firstTest = result.jsonOutput?.result.tests[0]; + expect(jsonResult.tests).to.be.an('array'); + const firstTest = jsonResult.tests[0]; expect(firstTest).to.have.property('id'); expect(firstTest).to.have.property('status'); expect(firstTest).to.have.property('evaluations'); @@ -187,10 +194,7 @@ describe('agent test run-eval', function () { }); it('should include summary with all metrics', async () => { - const command = `agent test run-eval --spec ${jsonPayloadPath} --api-name Local_Info_Agent --target-org ${getUsername()} --json`; - const result = execCmd(command, { ensureExitCode: 0 }); - - const summary = result.jsonOutput?.result.summary; + const summary = jsonResult.summary; expect(summary).to.have.property('passed'); expect(summary).to.have.property('failed'); expect(summary).to.have.property('scored'); diff --git a/test/nuts/z2.agent.publish.nut.ts b/test/nuts/z2.agent.publish.nut.ts index ec516cc5..a79afb6f 100644 --- a/test/nuts/z2.agent.publish.nut.ts +++ b/test/nuts/z2.agent.publish.nut.ts @@ -26,51 +26,52 @@ import { getAgentUsername, getTestSession, getUsername } from './shared-setup.js type BotDefinitionWithVersions = { Id: string; BotVersions: { - records: Array<{ DeveloperName: string }>; + records: Array<{ VersionNumber: number }>; }; }; const verifyPublishedAgent = async ( botApiName: string, - expectedVersion: string, + minVersionCount: number, connection: Connection ): Promise => { - let botDefinition; - try { - botDefinition = await connection.singleRecordQuery( - `SELECT SELECT Id, (SELECT DeveloperName FROM BotVersions LIMIT 10) FROM BotDefinition WHERE DeveloperName = '${botApiName}' LIMIT 1` - ); - const botVersion = botDefinition.BotVersions.records[0].DeveloperName; - expect(botVersion).to.equal(expectedVersion); - } catch { - // bot not found - void Promise.reject(new Error('Bot not published')); - } -}; - -async function verifyGenAiPlannerBundleExistsOrNot( - projectDir: string, - expectedBundleName: string, - expectedVersion: string, - shouldExist: boolean -): Promise { - const genAiPlannerBundleDir = join( - projectDir, - 'force-app', - 'main', - 'default', - 'genAiPlannerBundles', - `${expectedBundleName}_${expectedVersion}` + // singleRecordQuery throws if the agent was never published (0 records), which correctly fails + // the test. The old `SELECT SELECT ... DeveloperName` query was malformed, checked the wrong + // field (BotVersion.DeveloperName is an unrelated string like "Test_v1", not the version), and + // swallowed every failure via `void Promise.reject(...)`, so this check never actually ran. + // Each publish of the bundle adds a BotVersion, so after the Nth publish there should be at + // least N versions. We assert the COUNT (>=) rather than a specific VersionNumber because + // VersionNumber indexing isn't guaranteed to start at 1, and a --force-overwrite retry can add + // an extra version — both of which an exact check would get wrong. + const botDefinition = await connection.singleRecordQuery( + `SELECT Id, (SELECT VersionNumber FROM BotVersions ORDER BY VersionNumber) FROM BotDefinition WHERE DeveloperName = '${botApiName}' LIMIT 1` ); + expect( + botDefinition.BotVersions.records.length, + `agent "${botApiName}" should have at least ${minVersionCount} published version(s)` + ).to.be.at.least(minVersionCount); +}; - // Verify the genAiPlannerBundles directory exists - expect(existsSync(genAiPlannerBundleDir)).to.equal(shouldExist); - - if (shouldExist) { - // Verify the directory contains files - const files = readdirSync(genAiPlannerBundleDir); - expect(files.length).to.be.greaterThan(0); - } +/** + * After a successful publish the agent metadata is retrieved into the project. On modern orgs + * (API 68+) that is an `AiAgentDefinition` (dir `aiAgents`) plus an `AiAgentDefinitionVersion` + * (dir `aiAgentDefinitionVersions`); older orgs fall back to the legacy `Bot` (dir `bots`). The + * publish no longer produces the `genAiPlannerBundles/_vN` directory the old assertion + * looked for (removed with the move off the `_vN` suffix convention, forcedotcom/agents#310). We + * check for any artifact whose name starts with the agent api name, so the assertion is robust to + * the retrieve format and the exact file suffix. + */ +function verifyAgentMetadataRetrieved(projectDir: string, botApiName: string, shouldExist: boolean): void { + const defaultDir = join(projectDir, 'force-app', 'main', 'default'); + const hasEntry = (subDir: string): boolean => { + const dir = join(defaultDir, subDir); + return existsSync(dir) && readdirSync(dir).some((entry) => entry.startsWith(botApiName)); + }; + const retrieved = hasEntry('aiAgents') || hasEntry('aiAgentDefinitionVersions') || hasEntry('bots'); + expect( + retrieved, + `expected agent "${botApiName}" metadata ${shouldExist ? 'to' : 'not to'} be retrieved into the project` + ).to.equal(shouldExist); } describe('agent publish authoring-bundle NUTs', function () { @@ -119,7 +120,11 @@ describe('agent publish authoring-bundle NUTs', function () { execCmd(specCommand, { ensureExitCode: 0 }); // Step 2: Generate the authoring bundle from the spec - const generateCommand = `agent generate authoring-bundle --spec ${specPath} --name "${bundleApiName}" --api-name ${bundleApiName} --target-org ${getUsername()} --json`; + // --force-overwrite so a retry (this.retries(2)) re-generates over the bundle dir left on + // disk by a failed first attempt, instead of dying immediately with JsonAabExistsError. Without + // it, the retries are poisoned: they never reach the publish step, so the real first-attempt + // publish failure never surfaces in CI. + const generateCommand = `agent generate authoring-bundle --spec ${specPath} --name "${bundleApiName}" --api-name ${bundleApiName} --target-org ${getUsername()} --force-overwrite --json`; const generateResult = execCmd(generateCommand, { ensureExitCode: 0, }).jsonOutput?.result; @@ -146,8 +151,8 @@ describe('agent publish authoring-bundle NUTs', function () { expect(publishResult?.success).to.be.true; expect(publishResult?.botDeveloperName).to.be.a('string'); expect(publishResult?.errors).to.be.undefined; - await verifyPublishedAgent(bundleApiName, 'v1', connection); - await verifyGenAiPlannerBundleExistsOrNot(session.project.dir, bundleApiName, 'v1', true); + await verifyPublishedAgent(bundleApiName, 1, connection); + verifyAgentMetadataRetrieved(session.project.dir, bundleApiName, true); }); it('should publish a new version of an existing agent', async function () { @@ -165,8 +170,8 @@ describe('agent publish authoring-bundle NUTs', function () { expect(result?.success).to.be.true; expect(result?.botDeveloperName).to.be.a('string'); expect(result?.errors).to.be.undefined; - await verifyPublishedAgent(bundleApiName, 'v2', connection); - await verifyGenAiPlannerBundleExistsOrNot(session.project.dir, bundleApiName, 'v2', true); + await verifyPublishedAgent(bundleApiName, 2, connection); + verifyAgentMetadataRetrieved(session.project.dir, bundleApiName, true); }); it('should publish agent with skip-retrieve flag', async function () { @@ -183,10 +188,11 @@ describe('agent publish authoring-bundle NUTs', function () { ); const output = result.shellOutput.stdout; + // "Retrieve Metadata - Skipped" in stdout is the authoritative signal that --skip-retrieve + // suppressed the retrieve. A disk check can't prove v3 specifically wasn't retrieved, because + // the agent-level AiAgentDefinition from the earlier versions is already on disk. expect(output).to.include('Retrieve Metadata - Skipped'); - await verifyPublishedAgent(bundleApiName, 'v3', connection); - // skip-retrieve should not create a new version of the genAiPlannerBundle - await verifyGenAiPlannerBundleExistsOrNot(session.project.dir, bundleApiName, 'v3', false); + await verifyPublishedAgent(bundleApiName, 3, connection); }); it('should fail for invalid bundle api-name', async () => { diff --git a/test/nuts/z3.agent.preview.context-variables.nut.ts b/test/nuts/z3.agent.preview.context-variables.nut.ts index 014ff899..3abaef3f 100644 --- a/test/nuts/z3.agent.preview.context-variables.nut.ts +++ b/test/nuts/z3.agent.preview.context-variables.nut.ts @@ -37,8 +37,7 @@ import { getTestSession, getUsername } from './shared-setup.js'; * * Fixture: Willie_Resort_Manager declares test-only External vars NutProbeText (Text), * NutProbeBool (Boolean), NutProbeNum (Number), NutProbeObj (Json object), NutProbeList - * (List), plus the pre-existing Internal var VerifiedCustomerId (no visibility) used by the - * server-rejection case. + * (List). */ describe('agent preview --context-variables-json', function () { this.timeout(30 * 60 * 1000); // 30 minutes (shared setup deploys + waits on Einstein) @@ -148,15 +147,12 @@ describe('agent preview --context-variables-json', function () { expect(JSON.stringify(result.shellOutput)).to.include('Expected Name=Value'); }); - it('CV5: the server rejects an Internal variable', () => { - // VerifiedCustomerId has no visibility -> Internal -> not settable via preview. - const result = execCmd( - `agent preview start --authoring-bundle ${bundle} --simulate-actions --context-variables-json '[{"name":"VerifiedCustomerId","type":"Text","value":"x"}]' --target-org ${getUsername()} --json`, - { cwd: session.project.dir } - ); - expect(result.shellOutput.code, 'command should fail').to.not.equal(0); - expect(JSON.stringify(result.shellOutput)).to.include('Internal'); - }); + // Removed CV5 ("the server rejects an Internal variable"): it asserted a server-side + // rejection of the no-visibility Internal var VerifiedCustomerId, but that is purely server + // behavior — neither the plugin (flags.ts validates only name/type/value-type) nor the SDK + // (scriptAgent passes `variables` straight through) enforces visibility. The server stopped + // rejecting it (confirmed e2e: command exits 0 and returns a session), so the test pinned a + // server contract the plugin doesn't own. CV1–CV4/CV6 still cover the plugin's own handling. it('CV6: a value whose JSON type mismatches its declared type is rejected client-side', () => { const result = execCmd(