Copilot feedback

Author: richardhjtan

packages/bot-runner/lib/command-runner.ts

@@ -88,7 +88,11 @@ export class CommandRunner {
         return;
       }
 
-      if (!eventContent?.input || typeof eventContent.input !== 'object') {
+      if (
+        !eventContent?.input ||
+        typeof eventContent.input !== 'object' ||
+        Array.isArray(eventContent.input)
+      ) {
         return;
       }
 

packages/bot-runner/lib/pr-listing/pr-listing-workflow-handler.ts

@@ -13,10 +13,7 @@ import {
   CreateListingPRHandler,
   type BotTriggerEventContent,
 } from './create-listing-pr-handler';
-import type {
-  BotCommandHandler,
-  EnqueueRunCommandFn,
-} from '../command-runner';
+import type { BotCommandHandler, EnqueueRunCommandFn } from '../command-runner';
 import type { GitHubClient } from '../github';
 
 export type LintSubmissionFilesFn = (
@@ -114,7 +111,13 @@ export class PrListingWorkflowHandler implements BotCommandHandler {
       return this.runWorkflow(ctx);
     }
     if (eventContent.type === PR_LISTING_RETRY) {
-      let ctx = await this.buildRetryContext(runAs, eventContent);
+      let ctx;
+      try {
+        ctx = await this.buildRetryContext(runAs, eventContent);
+      } catch (err) {
+        await this.recordRetryContextFailure(runAs, eventContent, err);
+        throw err;
+      }
       return this.runWorkflow(ctx);
     }
     throw new Error(
@@ -244,9 +247,7 @@ export class PrListingWorkflowHandler implements BotCommandHandler {
         : await this.runFreshPrCardFlow(ctx);
 
       await runStep('github-pr', () => this.pushToGitHub(ctx, prCardData));
-      await runStep('github-pr', () =>
-        this.linkPrCardOnWorkflow(ctx, prCardData.prCardUrl),
-      );
+      await runStep('github-pr', () => this.clearWorkflowError(ctx));
 
       return prCardData.prCardResult;
     } catch (err) {
@@ -264,6 +265,9 @@ export class PrListingWorkflowHandler implements BotCommandHandler {
     let { prCardResult, prCardUrl } = await runStep('create-pr-card', () =>
       this.createPrCard(ctx, textFiles, totalCount),
     );
+    await runStep('create-pr-card', () =>
+      this.linkPrCardOnWorkflow(ctx, prCardUrl),
+    );
     return { prCardResult, prCardUrl, binaryFiles };
   }
 
@@ -425,13 +429,19 @@ export class PrListingWorkflowHandler implements BotCommandHandler {
   ): Promise<void> {
     if (!ctx.workflowCardUrl || !prCardUrl) return;
     await this.patchWorkflowCard(ctx, {
-      attributes: { prCreationError: null, failedStep: null },
       relationships: {
         prCard: { links: { self: prCardUrl } },
       },
     });
   }
 
+  private async clearWorkflowError(ctx: WorkflowContext): Promise<void> {
+    if (!ctx.workflowCardUrl) return;
+    await this.patchWorkflowCard(ctx, {
+      attributes: { prCreationError: null, failedStep: null },
+    });
+  }
+
   private async recordWorkflowFailure(
     ctx: WorkflowContext,
     err: unknown,
@@ -457,6 +467,46 @@ export class PrListingWorkflowHandler implements BotCommandHandler {
     }
   }
 
+  private async recordRetryContextFailure(
+    runAs: string,
+    eventContent: BotTriggerEventContent,
+    err: unknown,
+  ): Promise<void> {
+    let input = eventContent.input as Record<string, unknown> | null;
+    let workflowCardUrl =
+      typeof input?.workflowCardUrl === 'string' && input.workflowCardUrl.trim()
+        ? input.workflowCardUrl.trim()
+        : null;
+    if (!workflowCardUrl) return;
+    let workflowCardRealm =
+      (typeof input?.workflowCardRealm === 'string' &&
+      input.workflowCardRealm.trim()
+        ? input.workflowCardRealm.trim()
+        : null) ?? (eventContent.realm as string);
+    let message = err instanceof Error ? err.message : String(err);
+    try {
+      let result = await this.enqueueRunCommand({
+        runAs,
+        realmURL: workflowCardRealm,
+        command: PATCH_CARD_INSTANCE_COMMAND,
+        commandInput: {
+          cardId: workflowCardUrl,
+          patch: {
+            attributes: {
+              prCreationError: `PR retry failed: ${message}`,
+            },
+          },
+        },
+      });
+      requireReady(result, 'patch-card-instance (retry pre-context failure)');
+    } catch (patchError: any) {
+      log.error(
+        'pr-listing-retry: failed to patch workflow card after retry context build failed',
+        { patchError: patchError?.message ?? patchError },
+      );
+    }
+  }
+
   // ── Realm helpers ──
 
   private async patchWorkflowCard(

packages/bot-runner/tests/command-runner-test.ts

@@ -263,8 +263,8 @@ module('command runner', () => {
 
     assert.strictEqual(
       publishedJobs.length,
-      4,
-      'enqueues collect-files, lintStatus=passed patch, create-pr-card, and prCard-link patch (lint step skipped)',
+      5,
+      'enqueues collect-files, lintStatus=passed patch, create-pr-card, prCard-link patch, and clear-error patch (lint step skipped)',
     );
     assert.strictEqual(createdBranches.length, 1, 'creates branch');
     assert.strictEqual(branchWrites.length, 1, 'writes files to branch');
@@ -296,12 +296,19 @@ module('command runner', () => {
       `command:${SUBMISSION_REALM_URL}`,
       'Job 3 (create-pr-card) uses submissions realm concurrency group',
     );
-    // Job 4: prCard link patch — user realm
+    // Job 4: prCard link patch — user realm. Persisted immediately after
+    // create-pr-card so retry on a later GitHub failure can find it.
     assert.strictEqual(
       (publishedJobs[3] as { concurrencyGroup: string }).concurrencyGroup,
       'command:http://localhost:4201/test/',
       'Job 4 (prCard link patch) uses default realm concurrency group',
     );
+    // Job 5: clear-error patch — user realm
+    assert.strictEqual(
+      (publishedJobs[4] as { concurrencyGroup: string }).concurrencyGroup,
+      'command:http://localhost:4201/test/',
+      'Job 5 (clear-error patch) uses default realm concurrency group',
+    );
 
     assert.deepEqual(
       (publishedJobs[2] as { args: Record<string, unknown> }).args,
@@ -335,10 +342,6 @@ module('command runner', () => {
         commandInput: {
           cardId: submissionCardUrl,
           patch: {
-            attributes: {
-              prCreationError: null,
-              failedStep: null,
-            },
             relationships: {
               prCard: {
                 links: {
@@ -349,7 +352,26 @@ module('command runner', () => {
           },
         },
       },
-      'enqueues workflow card patch in the user realm (clears any prior error state and links PrCard)',
+      'persists prCard link on the workflow card immediately after create-pr-card succeeds (so retry on later failure can reuse the existing PrCard)',
+    );
+    assert.deepEqual(
+      (publishedJobs[4] as { args: Record<string, unknown> }).args,
+      {
+        realmURL: 'http://localhost:4201/test/',
+        realmUsername: '@alice:localhost',
+        runAs: '@alice:localhost',
+        command: '@cardstack/boxel-host/commands/patch-card-instance/default',
+        commandInput: {
+          cardId: submissionCardUrl,
+          patch: {
+            attributes: {
+              prCreationError: null,
+              failedStep: null,
+            },
+          },
+        },
+      },
+      'clears prior error attributes on the workflow card after the GitHub PR succeeds',
     );
     let prBody =
       (
@@ -765,7 +787,8 @@ module('command runner', () => {
                             // title is the display-formatted "Submit <X>";
                             // retry must NOT derive branchName from it.
                             title: 'Submit My Listing',
-                            branchName: 'room-IWFiYzEyMzpsb2NhbGhvc3Q/my-listing',
+                            branchName:
+                              'room-IWFiYzEyMzpsb2NhbGhvc3Q/my-listing',
                           },
                           relationships: {
                             listing: {

packages/host/app/commands/create-submission-workflow.ts

@@ -155,6 +155,7 @@ export default class CreateSubmissionWorkflowCommand extends HostBaseCommand<
         realm,
         listingId,
         workflowCardUrl: workflowCardId,
+        workflowCardRealm: workflowRealm,
         branchName,
         ...(listingName ? { listingName } : {}),
         ...(listingSummary ? { listingSummary } : {}),

packages/host/app/commands/retry-submission-workflow.ts

@@ -10,7 +10,15 @@ import SendBotTriggerEventCommand from './bot-requests/send-bot-trigger-event';
 
 import type RealmService from '../services/realm';
 import type StoreService from '../services/store';
-import type { SubmissionWorkflowCard } from '@cardstack/catalog/submission-workflow-card/submission-workflow-card';
+
+// Local view: host's tsconfig maps @cardstack/catalog/* to the legacy
+// in-monorepo catalog-realm, which lacks `failedStep`. Drop this once
+// that path points at boxel-catalog (the canonical source).
+interface WorkflowCardView {
+  roomId?: string;
+  listing?: { id?: string };
+  failedStep?: string | null;
+}
 
 // Re-emits the `pr-listing-retry` bot trigger event for an existing
 // SubmissionWorkflowCard that ended in a failed state. Reads roomId + listing
@@ -38,13 +46,13 @@ export default class RetrySubmissionWorkflowCommand extends HostBaseCommand<
   ): Promise<undefined> {
     let { workflowCardId } = input;
 
-    let workflowCard =
-      await this.store.get<SubmissionWorkflowCard>(workflowCardId);
-    if (!workflowCard || !isCardInstance(workflowCard)) {
+    let result = await this.store.get(workflowCardId);
+    if (!result || !isCardInstance(result)) {
       throw new Error(
         `Cannot retry: workflow card ${workflowCardId} not found`,
       );
     }
+    let workflowCard = result as unknown as WorkflowCardView;
 
     let roomId = workflowCard.roomId;
     let listingId = workflowCard.listing?.id;
@@ -64,9 +72,12 @@ export default class RetrySubmissionWorkflowCommand extends HostBaseCommand<
       );
     }
 
-    // Clear the prior failure state up front for instant UI feedback. The
-    // bot-runner will repopulate these fields on success or on a fresh
-    // failure; we don't have to wait for it to do so.
+    // Snapshot the prior failedStep so we can restore it if the send fails
+    // — without it, the optimistic clear below would hide the Retry button
+    // (canRetry requires prCreationError || failedStep) and strand the user.
+    let priorFailedStep = workflowCard.failedStep ?? null;
+
+    // Optimistic clear for instant UI feedback. Re-set on send failure.
     await this.store.patch(
       workflowCardId,
       {
@@ -78,13 +89,31 @@ export default class RetrySubmissionWorkflowCommand extends HostBaseCommand<
       { doNotWaitForPersist: true },
     );
 
-    await new SendBotTriggerEventCommand(this.commandContext).execute({
-      roomId,
-      realm: listingRealm,
-      type: 'pr-listing-retry',
-      input: {
-        workflowCardUrl: workflowCardId,
-      },
-    });
+    try {
+      await new SendBotTriggerEventCommand(this.commandContext).execute({
+        roomId,
+        realm: listingRealm,
+        type: 'pr-listing-retry',
+        input: {
+          workflowCardUrl: workflowCardId,
+          workflowCardRealm:
+            this.realm.realmOf(rri(workflowCardId)) ?? undefined,
+        },
+      });
+    } catch (sendError: any) {
+      let message =
+        sendError instanceof Error ? sendError.message : String(sendError);
+      await this.store.patch(
+        workflowCardId,
+        {
+          attributes: {
+            prCreationError: `Failed to send retry: ${message}`,
+            failedStep: priorFailedStep,
+          },
+        },
+        { doNotWaitForPersist: true },
+      );
+      throw sendError;
+    }
   }
 }