Merge pull request #477 from salesforcecli/chore/fix-failing-nuts

fix: report in-progress status when agent test run --wait times out @W-23915028@

Author: nico-pappagianis

src/commands/agent/test/resume.ts

@@ -141,10 +141,18 @@ export default class AgentTestResume extends SfCommand<AgentTestRunResult> {
       );
     }
 
-    if (completed) await agentTestCache.removeCacheEntry(runId);
-
     this.mso.stop();
 
+    // A client-side poll timeout does not mean the run finished — it is still running
+    // server-side. Leave the cache entry in place so the run can be resumed again (poll() has
+    // already printed the resume hint) and report the in-progress status rather than masking the
+    // timeout as COMPLETED with no results.
+    if (!completed || !response) {
+      return { status: 'IN_PROGRESS', runId };
+    }
+
+    await agentTestCache.removeCacheEntry(runId);
+
     await handleTestResults({
       id: runId,
       format: resultFormat ?? flags['result-format'],
@@ -157,16 +165,11 @@ export default class AgentTestResume extends SfCommand<AgentTestRunResult> {
     // Set exit code to 1 only for execution errors (tests couldn't run properly)
     // Test assertion failures are business logic and should not affect exit code
     // Only applicable to legacy responses (Agentforce Studio doesn't have test case status)
-    if (
-      response &&
-      'subjectName' in response &&
-      response.testCases.some((tc) => 'status' in tc && tc.status === 'ERROR')
-    ) {
+    if ('subjectName' in response && response.testCases.some((tc) => 'status' in tc && tc.status === 'ERROR')) {
       process.exitCode = 1;
     }
 
-    // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
-    return { ...response!, runId, status: 'COMPLETED' } as AgentTestRunResult;
+    return { ...response, runId, status: 'COMPLETED' } as AgentTestRunResult;
   }
 
   protected catch(error: Error | SfError | CLIError): Promise<never> {

src/commands/agent/test/run.ts

@@ -178,10 +178,18 @@ export default class AgentTestRun extends SfCommand<AgentTestRunResult> {
         );
       }
 
-      if (completed) await agentTestCache.removeCacheEntry(response.runId);
-
       this.mso.stop();
 
+      // A client-side poll timeout does not mean the run finished — it is still running
+      // server-side. Leave the cache entry in place so `agent test resume` works (poll() has
+      // already printed the resume hint) and report the in-progress status rather than masking
+      // the timeout as COMPLETED with no results.
+      if (!completed || !detailsResponse) {
+        return { status: 'IN_PROGRESS', runId: response.runId };
+      }
+
+      await agentTestCache.removeCacheEntry(response.runId);
+
       await handleTestResults({
         id: response.runId,
         format: flags['result-format'],
@@ -195,7 +203,6 @@ export default class AgentTestRun extends SfCommand<AgentTestRunResult> {
       // Test assertion failures are business logic and should not affect exit code
       // Only applicable to legacy responses (Agentforce Studio doesn't have test case status)
       if (
-        detailsResponse &&
         'subjectName' in detailsResponse &&
         detailsResponse.testCases.some((tc) => 'status' in tc && tc.status === 'ERROR')
       ) {

src/testStages.ts

@@ -113,7 +113,10 @@ export class TestStages {
         this.stop('async');
         this.ux.log(`Client timed out after ${wait.minutes} minutes.`);
         this.ux.log(`Run ${colorize('dim', `sf agent test resume --job-id ${id}`)} to resuming watching this test.`);
-        return { completed: true };
+        // The client stopped watching, but the run is still going server-side — it did NOT
+        // complete. Report completed:false so the caller preserves the cache entry (for
+        // `agent test resume`) and does not mask the timeout as a COMPLETED result.
+        return { completed: false };
       } else {
         this.error();
         throw e;

test/nuts/z2.agent.publish.nut.ts

@@ -87,6 +87,25 @@ describe('agent publish authoring-bundle NUTs', function () {
     connection = org.getConnection();
   });
 
+  // Per-attempt timing so a slow publish is visible in CI. This fires on EVERY attempt
+  // (including the ones mocha's this.retries() would otherwise hide by reporting only the
+  // final pass), so a stalled first attempt shows up here as a large elapsed with a failed
+  // state. Paired with the bounded publish-poll timeout, a genuine stall also surfaces which
+  // poll hung via its error (agentRetrievalError vs authoringBundleDeploymentError).
+  let attemptStart = 0;
+  beforeEach(() => {
+    attemptStart = Date.now();
+  });
+  afterEach(function () {
+    const elapsedSec = Math.round((Date.now() - attemptStart) / 1000);
+    // eslint-disable-next-line no-console
+    console.log(
+      `[z2 publish timing] "${this.currentTest?.title ?? 'unknown'}" attempt took ${elapsedSec}s (${
+        this.currentTest?.state ?? 'unknown'
+      })`
+    );
+  });
+
   it('should publish a new agent (first version)', async function () {
     // Increase timeout to 30 minutes since deployment can take a long time
     this.timeout(30 * 60 * 1000); // 30 minutes

test/nuts/z4.agent.test.AFS.nut.ts

@@ -123,9 +123,11 @@ describe('agent test (agentforce-studio)', function () {
     writeFileSync(join(metaDir, `${afsTestName}.aiTestingDefinition-meta.xml`), metaXml, 'utf8');
     console.log(`Wrote AiTestingDefinition metadata to ${metaDir}`);
 
-    // Deploy the definition
+    // Deploy the definition. Scope to just the file we authored — deploying the whole
+    // aiTestingDefinitions dir would also sweep in static fixtures (e.g. ReturnsCheckoutSuite,
+    // which targets a non-deployed multi-agent subject) and fail server-side validation.
     const cs = await ComponentSetBuilder.build({
-      sourcepath: [metaDir],
+      sourcepath: [join(metaDir, `${afsTestName}.aiTestingDefinition-meta.xml`)],
     });
     const deploy = await cs.deploy({ usernameOrConnection: getUsername() });
     const deployResult = await deploy.pollStatus({ frequency: Duration.seconds(10), timeout: Duration.minutes(10) });
@@ -139,8 +141,10 @@ describe('agent test (agentforce-studio)', function () {
     console.log(`Deployed AiTestingDefinition '${afsTestName}'`);
   });
 
-  // Set by the run test, consumed by the results tests (Mocha runs describes sequentially)
-  let completedRunId: string;
+  // Set by the run test, consumed by the results/resume tests (Mocha runs describes sequentially).
+  // The AFS eval can outlast the client --wait window, so the run may still be IN_PROGRESS here.
+  let sharedRunId: string;
+  let primaryRunCompleted = false;
 
   describe('agent test list', () => {
     it('should include the AFS test definition in list', async () => {
@@ -162,20 +166,26 @@ describe('agent test (agentforce-studio)', function () {
         { ensureExitCode: 0 }
       ).jsonOutput;
 
-      expect(output?.result.status).to.equal('COMPLETED');
-      expect(output?.result.runId.startsWith('3A2')).to.be.true;
       const result = output?.result as AgentTestRunResult & { testCases?: unknown[] };
-      expect(result?.testCases).to.be.an('array');
+      // The AFS eval can take longer than the --wait window. A client-side timeout is a valid
+      // outcome that now reports IN_PROGRESS (rather than masking as COMPLETED), so accept either
+      // and assert only the run shape. testCases is only guaranteed once the run has COMPLETED.
+      expect(result?.status).to.be.oneOf(['COMPLETED', 'IN_PROGRESS']);
+      expect(result?.runId.startsWith('3A2')).to.be.true;
       expect(result).to.not.have.property('subjectName');
+      if (result?.status === 'COMPLETED') {
+        expect(result?.testCases).to.be.an('array');
+        primaryRunCompleted = true;
+      }
 
-      completedRunId = output!.result.runId;
+      sharedRunId = output!.result.runId;
     });
   });
 
   describe('agent test results', () => {
     it('should fetch AFS results by job ID (json)', async () => {
       const output = execCmd<AgentTestResultsResult>(
-        `agent test results --job-id ${completedRunId} --target-org ${getUsername()} --json`,
+        `agent test results --job-id ${sharedRunId} --target-org ${getUsername()} --json`,
         { ensureExitCode: 0 }
       ).jsonOutput;
 
@@ -187,15 +197,15 @@ describe('agent test (agentforce-studio)', function () {
 
     it('should support human result format', () => {
       const output = execCmd(
-        `agent test results --job-id ${completedRunId} --result-format human --target-org ${getUsername()}`,
+        `agent test results --job-id ${sharedRunId} --result-format human --target-org ${getUsername()}`,
         { ensureExitCode: 0 }
       );
       expect(output.shellOutput.stdout).to.be.a('string').with.length.greaterThan(0);
     });
 
     it('should support junit result format', () => {
       const output = execCmd(
-        `agent test results --job-id ${completedRunId} --result-format junit --target-org ${getUsername()}`,
+        `agent test results --job-id ${sharedRunId} --result-format junit --target-org ${getUsername()}`,
         { ensureExitCode: 0 }
       );
       expect(output.shellOutput.stdout).to.include('<?xml');
@@ -204,44 +214,59 @@ describe('agent test (agentforce-studio)', function () {
 
     it('should support tap result format', () => {
       const output = execCmd(
-        `agent test results --job-id ${completedRunId} --result-format tap --target-org ${getUsername()}`,
+        `agent test results --job-id ${sharedRunId} --result-format tap --target-org ${getUsername()}`,
         { ensureExitCode: 0 }
       );
       expect(output.shellOutput.stdout).to.include('TAP version 13');
     });
   });
 
   describe('agent test resume', () => {
-    it('should start async then resume by job ID, and support --use-most-recent', async () => {
-      // Clear any stale entries before the run
-      const cacheBefore = await AgentTestCache.create();
-      cacheBefore.clear();
-      await cacheBefore.write();
+    it('should resume the in-flight run (or a fresh async run) and honor the cache', async function () {
+      this.timeout(30 * 60 * 1000);
 
-      // One async start covers both resume paths
-      const runResult = execCmd<AgentTestRunResult>(
-        `agent test run --api-name ${afsTestName} --target-org ${getUsername()} --json`,
-        { ensureExitCode: 0 }
-      ).jsonOutput;
+      if (primaryRunCompleted) {
+        // The primary --wait run already finished, so its cache entry was removed and no run is
+        // in flight. Start a fresh async run to exercise NEW + cache-write, then resume by job-id.
+        const cacheBefore = await AgentTestCache.create();
+        cacheBefore.clear();
+        await cacheBefore.write();
 
-      expect(runResult?.result.runId.startsWith('3A2')).to.be.true;
-      expect(runResult?.result.status).to.equal('NEW');
+        const runResult = execCmd<AgentTestRunResult>(
+          `agent test run --api-name ${afsTestName} --target-org ${getUsername()} --json`,
+          { ensureExitCode: 0 }
+        ).jsonOutput;
 
-      // Re-read from disk — the run command wrote the cache entry in a subprocess
-      const cache = await AgentTestCache.create();
-      expect(cache.resolveFromCache().runnerType).to.equal('agentforce-studio');
+        expect(runResult?.result.runId.startsWith('3A2')).to.be.true;
+        expect(runResult?.result.status).to.equal('NEW');
 
-      const output = execCmd<AgentTestRunResult>(
-        `agent test resume --job-id ${runResult?.result.runId} --target-org ${getUsername()} --json`,
-        { ensureExitCode: 0 }
-      ).jsonOutput;
+        // Re-read from disk — the run command wrote the cache entry in a subprocess
+        const cache = await AgentTestCache.create();
+        expect(cache.resolveFromCache().runnerType).to.equal('agentforce-studio');
+
+        const output = execCmd<AgentTestRunResult>(
+          `agent test resume --job-id ${runResult?.result.runId} --target-org ${getUsername()} --json`,
+          { ensureExitCode: 0 }
+        ).jsonOutput;
+
+        // Resume may itself time out (the eval can outlast the poll window); both are valid.
+        expect(output?.result.status).to.be.oneOf(['COMPLETED', 'IN_PROGRESS']);
+        expect(output?.result.runId.startsWith('3A2')).to.be.true;
+      } else {
+        // The primary --wait run timed out client-side and is still running server-side. Our fix
+        // preserves the cache entry on a timeout, so resume --use-most-recent picks it up. Starting
+        // a NEW run here would collide — AFS allows only one run per definition at a time.
+        const cache = await AgentTestCache.create();
+        expect(cache.resolveFromCache().runnerType).to.equal('agentforce-studio');
 
-      expect(output?.result.status).to.equal('COMPLETED');
-      expect(output?.result.runId.startsWith('3A2')).to.be.true;
+        const output = execCmd<AgentTestRunResult>(
+          `agent test resume --use-most-recent --target-org ${getUsername()} --json`,
+          { ensureExitCode: 0 }
+        ).jsonOutput;
 
-      // Re-read from disk — resume removes the entry in a subprocess
-      const cacheAfter = await AgentTestCache.create();
-      expect(() => cacheAfter.resolveFromCache()).to.throw('Could not find a runId to resume');
+        expect(output?.result.status).to.be.oneOf(['COMPLETED', 'IN_PROGRESS']);
+        expect(output?.result.runId).to.equal(sharedRunId);
+      }
     });
   });