better messaging (#8794)

2026-03-10 22:21:22 -07:00 · 2025-09-18 19:00:18 -07:00
parent bfa4140593
commit ec01b1f29f
4 changed files with 375 additions and 27 deletions
--- a/.github/workflows/release-patch-1-create-pr.yml
+++ b/.github/workflows/release-patch-1-create-pr.yml
@@ -71,9 +71,15 @@ jobs:
          GH_TOKEN: '${{ steps.generate_token.outputs.token }}'
        continue-on-error: true
        run: |
-          node scripts/releasing/create-patch-pr.js --commit=${{ github.event.inputs.commit }} --channel=${{ github.event.inputs.channel }} --dry-run=${{ github.event.inputs.dry_run }} > patch_output.log 2>&1
-          echo "EXIT_CODE=$?" >> "$GITHUB_OUTPUT"
-          cat patch_output.log
+          # Capture output directly to environment variable
+          {
+            node scripts/releasing/create-patch-pr.js --commit=${{ github.event.inputs.commit }} --channel=${{ github.event.inputs.channel }} --dry-run=${{ github.event.inputs.dry_run }}
+            echo "EXIT_CODE=$?" >> "$GITHUB_OUTPUT"
+          } 2>&1 | {
+            echo "LOG_CONTENT<<EOF" >> "$GITHUB_ENV"
+            cat >> "$GITHUB_ENV"
+            echo "EOF" >> "$GITHUB_ENV"
+          }

      - name: 'Comment on Original PR'
        if: '!inputs.dry_run && inputs.original_pr'
@@ -81,7 +87,6 @@ jobs:
          GH_TOKEN: '${{ steps.generate_token.outputs.token }}'
          ORIGINAL_PR: '${{ github.event.inputs.original_pr }}'
          EXIT_CODE: '${{ steps.create_patch.outputs.EXIT_CODE }}'
-          OUTPUT_LOG: 'patch_output.log'
          COMMIT: '${{ github.event.inputs.commit }}'
          CHANNEL: '${{ github.event.inputs.channel }}'
          REPOSITORY: '${{ github.repository }}'
--- a/.github/workflows/release-patch-from-comment.yml
+++ b/.github/workflows/release-patch-from-comment.yml
@@ -52,6 +52,7 @@ jobs:

      - name: 'Dispatch if Merged'
        if: "steps.pr_status.outputs.STATE == 'MERGED'"
+        id: 'dispatch_patch'
        uses: 'actions/github-script@00f12e3e20659f42342b1c0226afda7f7c042325'
        env:
          COMMENT_BODY: '${{ github.event.comment.body }}'
@@ -82,7 +83,7 @@ jobs:

            console.log('Detected channel:', channel);

-            github.rest.actions.createWorkflowDispatch({
+            const response = await github.rest.actions.createWorkflowDispatch({
              owner: context.repo.owner,
              repo: context.repo.repo,
              workflow_id: 'release-patch-1-create-pr.yml',
@@ -93,7 +94,30 @@ jobs:
                dry_run: 'false',
                original_pr: '${{ github.event.issue.number }}'
              }
-            })
+            });
+
+            // Wait a moment for the workflow to be created, then find it
+            await new Promise(resolve => setTimeout(resolve, 2000));
+
+            const runs = await github.rest.actions.listWorkflowRuns({
+              owner: context.repo.owner,
+              repo: context.repo.repo,
+              workflow_id: 'release-patch-1-create-pr.yml',
+              per_page: 10
+            });
+
+            // Find the most recent run that matches our trigger
+            const dispatchedRun = runs.data.workflow_runs.find(run =>
+              run.event === 'workflow_dispatch' &&
+              new Date(run.created_at) > new Date(Date.now() - 10000) // Within last 10 seconds
+            );
+
+            if (dispatchedRun) {
+              core.setOutput('dispatched_run_id', dispatchedRun.id);
+              core.setOutput('dispatched_run_url', dispatchedRun.html_url);
+            }
+
+            core.setOutput('channel', channel);

      - name: 'Comment on Failure'
        if: "startsWith(github.event.comment.body, '/patch') && steps.pr_status.outputs.STATE != 'MERGED'"
@@ -102,3 +126,54 @@ jobs:
          issue-number: '${{ github.event.issue.number }}'
          body: |
            :x: The `/patch` command failed. This pull request must be merged before a patch can be created.
+
+      - name: 'Final Status Comment - Success'
+        if: "always() && startsWith(github.event.comment.body, '/patch') && steps.dispatch_patch.outcome == 'success' && steps.dispatch_patch.outputs.dispatched_run_url"
+        uses: 'peter-evans/create-or-update-comment@67dcc547d311b736a8e6c5c236542148a47adc3d'
+        with:
+          issue-number: '${{ github.event.issue.number }}'
+          body: |
+            ✅ **Patch workflow dispatched successfully!**
+
+            **📋 Details:**
+            - **Channel**: `${{ steps.dispatch_patch.outputs.channel }}`
+            - **Commit**: `${{ steps.pr_status.outputs.MERGE_COMMIT_SHA }}`
+
+            **🔗 Track Progress:**
+            - [Dispatched patch workflow](${{ steps.dispatch_patch.outputs.dispatched_run_url }})
+            - [This workflow run](https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }})
+
+      - name: 'Final Status Comment - Dispatch Success (No URL)'
+        if: "always() && startsWith(github.event.comment.body, '/patch') && steps.dispatch_patch.outcome == 'success' && !steps.dispatch_patch.outputs.dispatched_run_url"
+        uses: 'peter-evans/create-or-update-comment@67dcc547d311b736a8e6c5c236542148a47adc3d'
+        with:
+          issue-number: '${{ github.event.issue.number }}'
+          body: |
+            ✅ **Patch workflow dispatched successfully!**
+
+            **📋 Details:**
+            - **Channel**: `${{ steps.dispatch_patch.outputs.channel }}`
+            - **Commit**: `${{ steps.pr_status.outputs.MERGE_COMMIT_SHA }}`
+
+            **🔗 Track Progress:**
+            - [View patch workflows](https://github.com/${{ github.repository }}/actions/workflows/release-patch-1-create-pr.yml)
+            - [This workflow run](https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }})
+
+      - name: 'Final Status Comment - Failure'
+        if: "always() && startsWith(github.event.comment.body, '/patch') && (steps.dispatch_patch.outcome == 'failure' || steps.dispatch_patch.outcome == 'cancelled')"
+        uses: 'peter-evans/create-or-update-comment@67dcc547d311b736a8e6c5c236542148a47adc3d'
+        with:
+          issue-number: '${{ github.event.issue.number }}'
+          body: |
+            ❌ **Patch workflow dispatch failed!**
+
+            There was an error dispatching the patch creation workflow.
+
+            **🔍 Troubleshooting:**
+            - Check that the PR is properly merged
+            - Verify workflow permissions
+            - Review error logs in the workflow run
+
+            **🔗 Debug Links:**
+            - [This workflow run](https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }})
+            - [Patch workflow history](https://github.com/${{ github.repository }}/actions/workflows/release-patch-1-create-pr.yml)
--- a/scripts/releasing/patch-create-comment.js
+++ b/scripts/releasing/patch-create-comment.js
@@ -13,7 +13,6 @@

 import yargs from 'yargs';
 import { hideBin } from 'yargs/helpers';
-import { readFileSync } from 'node:fs';

 async function main() {
  const argv = await yargs(hideBin(process.argv))
@@ -27,11 +26,6 @@ async function main() {
      type: 'number',
      demandOption: !process.env.GITHUB_ACTIONS,
    })
-    .option('output-log', {
-      description: 'Path to the patch output log file',
-      type: 'string',
-      default: 'patch_output.log',
-    })
    .option('commit', {
      description: 'The commit SHA being patched',
      type: 'string',
@@ -86,8 +80,6 @@ async function main() {
    argv.exitCode !== undefined
      ? argv.exitCode
      : parseInt(process.env.EXIT_CODE || '1');
-  const outputLog =
-    argv.outputLog || process.env.OUTPUT_LOG || 'patch_output.log';
  const commit = argv.commit || process.env.COMMIT;
  const channel = argv.channel || process.env.CHANNEL;
  const repository =
@@ -111,7 +103,6 @@ async function main() {
    console.log('\n📋 Inputs:');
    console.log(`  - Original PR: ${originalPr}`);
    console.log(`  - Exit Code: ${exitCode}`);
-    console.log(`  - Output Log: ${outputLog}`);
    console.log(`  - Commit: ${commit}`);
    console.log(`  - Channel: ${channel} → npm tag: ${npmTag}`);
    console.log(`  - Repository: ${repository}`);
@@ -121,20 +112,17 @@ async function main() {
  let commentBody;
  let logContent = '';

-  // Try to read the output log
-  try {
-    if (testMode) {
-      // Create mock log content for testing
-      if (exitCode === 0) {
-        logContent = `Creating hotfix branch hotfix/v0.5.3/${channel}/cherry-pick-${commit.substring(0, 7)} from release/v0.5.3`;
-      } else {
-        logContent = 'Error: Failed to create patch';
-      }
+  // Get log content from environment variable or generate mock content for testing
+  if (testMode && !process.env.LOG_CONTENT) {
+    // Create mock log content for testing only if LOG_CONTENT is not provided
+    if (exitCode === 0) {
+      logContent = `Creating hotfix branch hotfix/v0.5.3/${channel}/cherry-pick-${commit.substring(0, 7)} from release/v0.5.3`;
    } else {
-      logContent = readFileSync(outputLog, 'utf8');
+      logContent = 'Error: Failed to create patch';
    }
-  } catch (error) {
-    console.log(`Could not read output log ${outputLog}:`, error.message);
+  } else {
+    // Use log content from environment variable
+    logContent = process.env.LOG_CONTENT || '';
  }

  if (logContent.includes('already has an open PR')) {
--- a/scripts/tests/patch-create-comment.test.js
+++ b/scripts/tests/patch-create-comment.test.js
@@ -0,0 +1,280 @@
+/**
+ * @license
+ * Copyright 2025 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest';
+import { execSync } from 'node:child_process';
+import { join } from 'node:path';
+
+/**
+ * Helper function to run the patch-create-comment script with given parameters
+ */
+function runPatchCreateComment(args, env = {}) {
+  const scriptPath = join(
+    process.cwd(),
+    'scripts/releasing/patch-create-comment.js',
+  );
+  const fullEnv = {
+    ...process.env,
+    TEST_MODE: 'true', // Always run in test mode to avoid GitHub API calls
+    ...env,
+  };
+
+  try {
+    const result = execSync(`node ${scriptPath} ${args}`, {
+      encoding: 'utf8',
+      env: fullEnv,
+    });
+    return { stdout: result, stderr: '', success: true };
+  } catch (error) {
+    return {
+      stdout: error.stdout || '',
+      stderr: error.stderr || '',
+      success: false,
+      code: error.status,
+    };
+  }
+}
+
+describe('patch-create-comment', () => {
+  let originalEnv;
+
+  beforeEach(() => {
+    // Save original environment
+    originalEnv = { ...process.env };
+  });
+
+  afterEach(() => {
+    // Restore original environment
+    process.env = originalEnv;
+    vi.clearAllMocks();
+  });
+  describe('Environment Variable vs File Reading', () => {
+    it('should prefer LOG_CONTENT environment variable over file', () => {
+      const result = runPatchCreateComment(
+        '--original-pr 8655 --exit-code 0 --commit abc1234 --channel preview --repository google-gemini/gemini-cli --test',
+        {
+          LOG_CONTENT:
+            'Creating hotfix branch hotfix/v0.5.3/preview/cherry-pick-abc1234 from release/v0.5.3',
+        },
+      );
+
+      expect(result.success).toBe(true);
+      expect(result.stdout).toContain('🚀 **Patch PR Created!**');
+      expect(result.stdout).toContain('Channel**: `preview`');
+      expect(result.stdout).toContain('Commit**: `abc1234`');
+    });
+
+    it('should use empty log content when LOG_CONTENT is not set', () => {
+      const result = runPatchCreateComment(
+        '--original-pr 8655 --exit-code 1 --commit abc1234 --channel stable --repository google-gemini/gemini-cli --test',
+        {}, // No LOG_CONTENT env var
+      );
+
+      expect(result.success).toBe(true);
+      expect(result.stdout).toContain('❌ **Patch creation failed!**');
+      expect(result.stdout).toContain(
+        'There was an error creating the patch release',
+      );
+    });
+  });
+
+  describe('Log Content Parsing - Success Scenarios', () => {
+    it('should generate success comment for clean cherry-pick', () => {
+      const result = runPatchCreateComment(
+        '--original-pr 8655 --exit-code 0 --commit abc1234 --channel stable --repository google-gemini/gemini-cli --test',
+        {
+          LOG_CONTENT:
+            'Creating hotfix branch hotfix/v0.4.1/stable/cherry-pick-abc1234 from release/v0.4.1\n✅ Cherry-pick successful - no conflicts detected',
+        },
+      );
+
+      expect(result.success).toBe(true);
+      expect(result.stdout).toContain('🚀 **Patch PR Created!**');
+      expect(result.stdout).toContain('Channel**: `stable`');
+      expect(result.stdout).toContain('Commit**: `abc1234`');
+      expect(result.stdout).not.toContain('⚠️ Status');
+    });
+
+    it('should generate conflict comment for cherry-pick with conflicts', () => {
+      const result = runPatchCreateComment(
+        '--original-pr 8655 --exit-code 0 --commit def5678 --channel preview --repository google-gemini/gemini-cli --test',
+        {
+          LOG_CONTENT:
+            'Creating hotfix branch hotfix/v0.5.0-preview.2/preview/cherry-pick-def5678 from release/v0.5.0-preview.2\nCherry-pick has conflicts in 2 file(s):\nCONFLICT (content): Merge conflict in package.json',
+        },
+      );
+
+      expect(result.success).toBe(true);
+      expect(result.stdout).toContain('🚀 **Patch PR Created!**');
+      expect(result.stdout).toContain(
+        '⚠️ Status**: Cherry-pick conflicts detected',
+      );
+      expect(result.stdout).toContain(
+        '⚠️ **Resolve conflicts** in the hotfix PR first',
+      );
+      expect(result.stdout).toContain('Channel**: `preview`');
+    });
+  });
+
+  describe('Log Content Parsing - Existing PR Scenarios', () => {
+    it('should detect existing PR and generate appropriate comment', () => {
+      const result = runPatchCreateComment(
+        '--original-pr 8655 --exit-code 0 --commit ghi9012 --channel stable --repository google-gemini/gemini-cli --test',
+        {
+          LOG_CONTENT:
+            'Hotfix branch hotfix/v0.4.1/stable/cherry-pick-ghi9012 already has an open PR.\nFound existing PR #8700: https://github.com/google-gemini/gemini-cli/pull/8700',
+        },
+      );
+
+      expect(result.success).toBe(true);
+      expect(result.stdout).toContain('ℹ️ **Patch PR already exists!**');
+      expect(result.stdout).toContain(
+        'A patch PR for this change already exists: [#8700](https://github.com/google-gemini/gemini-cli/pull/8700)',
+      );
+      expect(result.stdout).toContain(
+        'Review and approve the existing patch PR',
+      );
+    });
+
+    it('should detect branch exists but no PR scenario', () => {
+      const result = runPatchCreateComment(
+        '--original-pr 8655 --exit-code 0 --commit jkl3456 --channel preview --repository google-gemini/gemini-cli --test',
+        {
+          LOG_CONTENT:
+            'Hotfix branch hotfix/v0.5.0-preview.2/preview/cherry-pick-jkl3456 exists but has no open PR.\nHotfix branch hotfix/v0.5.0-preview.2/preview/cherry-pick-jkl3456 already exists.',
+        },
+      );
+
+      expect(result.success).toBe(true);
+      expect(result.stdout).toContain(
+        'ℹ️ **Patch branch exists but no PR found!**',
+      );
+      expect(result.stdout).toContain(
+        'Delete the branch: `git branch -D hotfix/v0.5.0-preview.2/preview/cherry-pick-jkl3456`',
+      );
+      expect(result.stdout).toContain('Run the patch command again');
+    });
+  });
+
+  describe('Log Content Parsing - Failure Scenarios', () => {
+    it('should generate failure comment when exit code is non-zero', () => {
+      const result = runPatchCreateComment(
+        '--original-pr 8655 --exit-code 1 --commit mno7890 --channel stable --repository google-gemini/gemini-cli --run-id 12345 --test',
+        {
+          LOG_CONTENT: 'Error: Failed to create patch',
+        },
+      );
+
+      expect(result.success).toBe(true);
+      expect(result.stdout).toContain('❌ **Patch creation failed!**');
+      expect(result.stdout).toContain(
+        'There was an error creating the patch release',
+      );
+      expect(result.stdout).toContain(
+        'View workflow run](https://github.com/google-gemini/gemini-cli/actions/runs/12345)',
+      );
+    });
+
+    it('should generate fallback failure comment when no output is generated', () => {
+      const result = runPatchCreateComment(
+        '--original-pr 8655 --exit-code 1 --commit pqr4567 --channel preview --repository google-gemini/gemini-cli --run-id 67890 --test',
+        {
+          LOG_CONTENT: '',
+        },
+      );
+
+      expect(result.success).toBe(true);
+      expect(result.stdout).toContain('❌ **Patch creation failed!**');
+      expect(result.stdout).toContain(
+        'There was an error creating the patch release',
+      );
+    });
+  });
+
+  describe('Channel and NPM Tag Detection', () => {
+    it('should correctly map stable channel to latest npm tag', () => {
+      const result = runPatchCreateComment(
+        '--original-pr 8655 --exit-code 0 --commit stu8901 --channel stable --repository google-gemini/gemini-cli --test',
+        {
+          LOG_CONTENT:
+            'Creating hotfix branch hotfix/v0.4.1/stable/cherry-pick-stu8901 from release/v0.4.1',
+        },
+      );
+
+      expect(result.success).toBe(true);
+      expect(result.stdout).toContain('will publish to npm tag `latest`');
+    });
+
+    it('should correctly map preview channel to preview npm tag', () => {
+      const result = runPatchCreateComment(
+        '--original-pr 8655 --exit-code 0 --commit vwx2345 --channel preview --repository google-gemini/gemini-cli --test',
+        {
+          LOG_CONTENT:
+            'Creating hotfix branch hotfix/v0.5.0-preview.2/preview/cherry-pick-vwx2345 from release/v0.5.0-preview.2',
+        },
+      );
+
+      expect(result.success).toBe(true);
+      expect(result.stdout).toContain('will publish to npm tag `preview`');
+    });
+  });
+
+  describe('No Original PR Scenario', () => {
+    it('should skip comment when no original PR is specified', () => {
+      const result = runPatchCreateComment(
+        '--original-pr 0 --exit-code 0 --commit yza6789 --channel stable --repository google-gemini/gemini-cli --test',
+        {
+          LOG_CONTENT:
+            'Creating hotfix branch hotfix/v0.4.1/stable/cherry-pick-yza6789 from release/v0.4.1',
+          ORIGINAL_PR: '', // Override with empty PR
+        },
+      );
+
+      expect(result.success).toBe(true);
+      expect(result.stdout).toContain(
+        'No original PR specified, skipping comment',
+      );
+    });
+  });
+
+  describe('Error Handling', () => {
+    it('should handle empty LOG_CONTENT gracefully', () => {
+      const result = runPatchCreateComment(
+        '--original-pr 8655 --exit-code 1 --commit bcd0123 --channel stable --repository google-gemini/gemini-cli --test',
+        { LOG_CONTENT: '' }, // Empty log content
+      );
+
+      expect(result.success).toBe(true);
+      expect(result.stdout).toContain('❌ **Patch creation failed!**');
+      expect(result.stdout).toContain(
+        'There was an error creating the patch release',
+      );
+    });
+  });
+
+  describe('Test Mode Flag', () => {
+    it('should generate mock content in test mode for success', () => {
+      const result = runPatchCreateComment(
+        '--original-pr 8655 --exit-code 0 --commit efg4567 --channel preview --repository google-gemini/gemini-cli --test',
+      );
+
+      expect(result.success).toBe(true);
+      expect(result.stdout).toContain(
+        '🧪 TEST MODE - No API calls will be made',
+      );
+      expect(result.stdout).toContain('🚀 **Patch PR Created!**');
+    });
+
+    it('should generate mock content in test mode for failure', () => {
+      const result = runPatchCreateComment(
+        '--original-pr 8655 --exit-code 1 --commit hij8901 --channel stable --repository google-gemini/gemini-cli --test',
+      );
+
+      expect(result.success).toBe(true);
+      expect(result.stdout).toContain('❌ **Patch creation failed!**');
+    });
+  });
+});