Address PR review feedback

Copilot suggestions:
- _getBooleanInput returns the default for empty or unrecognized values
- _setOutputs only sets change outputs when a report was captured
- gate deployment-report capture on reporting being enabled
- paginate the sticky-comment lookup so it is found past 100 comments
- redact secret-looking arguments from the summary
- pin @actions/github to 6.0.0
- remove the unused EMPTY_REPORT constant

Reviewer notes:
- friendlyType uses startsWith instead of a regex
- Dropped icon changed to a cross mark
- use string methods (replaceAll, trimEnd, startsWith) where a regex was not needed

Add es2019/es2021 string libs, update tests, and rebuild the bundle.

Author: chhateauuu

__tests__/DeploymentSummary.test.ts

@@ -63,7 +63,7 @@ describe('DeploymentSummary tests', () => {
         const markdown = buildSummary({ ...baseContext, report });
         expect(markdown).toContain('<summary>➕ Created (1)</summary>');
         expect(markdown).toContain('<summary>🔄 Altered (1)</summary>');
-        expect(markdown).toContain('<summary>🗑️ Dropped (1)</summary>');
+        expect(markdown).toContain('<summary>❌ Dropped (1)</summary>');
         expect(markdown).toContain('| `[dbo].[Reactions]` | Table |');
     });
 
@@ -120,7 +120,7 @@ describe('DeploymentSummary tests', () => {
 
     it('ends with the sticky comment marker', () => {
         const markdown = buildSummary({ ...baseContext, report });
-        expect(markdown.replace(/\s+$/, '').endsWith(SUMMARY_MARKER)).toBe(true);
+        expect(markdown.trimEnd().endsWith(SUMMARY_MARKER)).toBe(true);
     });
 
     it('never includes a password even if one is present in the context object', () => {

__tests__/Reporter.test.ts

@@ -42,7 +42,8 @@ describe('Reporter tests', () => {
                     createComment: jest.fn().mockResolvedValue({}),
                     updateComment: jest.fn().mockResolvedValue({})
                 }
-            }
+            },
+            paginate: jest.fn().mockResolvedValue(comments)
         };
         (github.getOctokit as jest.Mock).mockReturnValue(octokit);
         return octokit;
@@ -75,6 +76,33 @@ describe('Reporter tests', () => {
         expect(core.setOutput).toHaveBeenCalledWith('objects-changed', '4');
     });
 
+    it('does not set the change outputs when no report was captured', async () => {
+        mockInputs({ summary: 'true', 'comment-pr': 'off' });
+
+        await Reporter.report(inputs, {});
+
+        expect(core.setOutput).not.toHaveBeenCalledWith('changes-detected', expect.anything());
+        expect(core.setOutput).not.toHaveBeenCalledWith('objects-changed', expect.anything());
+    });
+
+    it('redacts secret-looking additional arguments from the summary', async () => {
+        mockInputs({ summary: 'true', 'comment-pr': 'off' });
+
+        await Reporter.report({ ...inputs, additionalArguments: '/TargetPassword:hunter2' }, {});
+
+        const markdown = (core.summary.addRaw as jest.Mock).mock.calls[0][0];
+        expect(markdown).toContain('[redacted]');
+        expect(markdown).not.toContain('hunter2');
+    });
+
+    it('falls back to the default when the summary value is unrecognized', async () => {
+        mockInputs({ summary: 'yes', 'comment-pr': 'off' });
+
+        await Reporter.report(inputs, {});
+
+        expect(core.summary.addRaw).toHaveBeenCalled();
+    });
+
     it('creates a new comment when none exists', async () => {
         mockInputs({ summary: 'false', 'comment-pr': 'auto', 'github-token': 'token' });
         const octokit = mockOctokit([]);
@@ -125,7 +153,8 @@ describe('Reporter tests', () => {
     it('warns without throwing when the comment API fails', async () => {
         mockInputs({ summary: 'false', 'comment-pr': 'auto', 'github-token': 'token' });
         (github.getOctokit as jest.Mock).mockReturnValue({
-            rest: { issues: { listComments: jest.fn().mockRejectedValue(new Error('forbidden')) } }
+            rest: { issues: { listComments: jest.fn() } },
+            paginate: jest.fn().mockRejectedValue(new Error('forbidden'))
         });
 
         await expect(Reporter.report(inputs, {})).resolves.toBeUndefined();

__tests__/main.test.ts

@@ -58,7 +58,7 @@ describe('main.ts tests', () => {
 
         expect(detectIPAddressSpy).toHaveBeenCalled();
         expect(getAuthorizerSpy).not.toHaveBeenCalled();
-        expect(getInputSpy).toHaveBeenCalledTimes(6);
+        expect(getInputSpy).toHaveBeenCalledTimes(8);
         expect(resolveFilePathSpy).toHaveBeenCalled();
         expect(addFirewallRuleSpy).not.toHaveBeenCalled();
         expect(actionExecuteSpy).toHaveBeenCalled();
@@ -102,7 +102,7 @@ describe('main.ts tests', () => {
 
         expect(detectIPAddressSpy).toHaveBeenCalled();
         expect(getAuthorizerSpy).not.toHaveBeenCalled();
-        expect(getInputSpy).toHaveBeenCalledTimes(5);
+        expect(getInputSpy).toHaveBeenCalledTimes(7);
         expect(resolveFilePathSpy).toHaveBeenCalled();
         expect(addFirewallRuleSpy).not.toHaveBeenCalled();
         expect(actionExecuteSpy).toHaveBeenCalled();
@@ -148,7 +148,7 @@ describe('main.ts tests', () => {
 
         expect(detectIPAddressSpy).toHaveBeenCalled();
         expect(getAuthorizerSpy).not.toHaveBeenCalled();
-        expect(getInputSpy).toHaveBeenCalledTimes(5);
+        expect(getInputSpy).toHaveBeenCalledTimes(7);
         expect(resolveFilePathSpy).toHaveBeenCalled();
         expect(addFirewallRuleSpy).not.toHaveBeenCalled();
         expect(actionExecuteSpy).toHaveBeenCalled();

lib/main.js

@@ -22,7 +22,7 @@
   "dependencies": {
     "@actions/core": "^1.9.1",
     "@actions/exec": "^1.0.1",
-    "@actions/github": "^6.0.0",
+    "@actions/github": "6.0.0",
     "@actions/tool-cache": "^2.0.1",
     "@tediousjs/connection-string": "^0.5.0",
     "azure-actions-webclient": "^1.0.3",

package-lock.json

@@ -33,8 +33,6 @@ export interface DeploymentReport {
     alerts: DeploymentAlert[];
 }
 
-const EMPTY_REPORT: DeploymentReport = { operations: [], alerts: [] };
-
 /**
  * Parses the XML of a SqlPackage deployment report into a structured summary.
  * The parse is namespace-agnostic and tolerant of missing or malformed input:
@@ -120,7 +118,7 @@ function friendlyType(type: string | undefined): string {
         return 'Object';
     }
 
-    const withoutPrefix = type.replace(/^Sql/, '');
+    const withoutPrefix = type.startsWith('Sql') ? type.slice(3) : type;
     const spaced = withoutPrefix.replace(/([a-z])([A-Z])/g, '$1 $2');
     return spaced || type;
 }

package.json

@@ -23,7 +23,7 @@ interface OperationGroup {
 const OPERATION_GROUPS: OperationGroup[] = [
     { key: 'create', label: 'Created', icon: '➕' },
     { key: 'alter', label: 'Altered', icon: '🔄' },
-    { key: 'drop', label: 'Dropped', icon: '🗑️' }
+    { key: 'drop', label: 'Dropped', icon: '❌' }
 ];
 
 /**
@@ -280,7 +280,7 @@ function renderScript(script: string): string[] {
     lines.push(`<summary>📄 Deployment T-SQL script · ${metaParts.join(' · ')}</summary>`);
     lines.push('');
     lines.push('```sql');
-    lines.push(content.replace(/\s+$/, ''));
+    lines.push(content.trimEnd());
     if (truncated) {
         lines.push('');
         lines.push('-- Script truncated for display. See the deployment-script-path output for the full script.');
@@ -368,7 +368,7 @@ function formatCounts(counts: Array<[string, number]>): string {
  * unescaped pipe would otherwise be interpreted as a column separator.
  */
 function escapeCell(value: string): string {
-    return value.replace(/\|/g, '\\|');
+    return value.replaceAll('|', '\\|');
 }
 
 /**
@@ -410,7 +410,7 @@ function pastTenseAction(action: string): string {
  * of the deployment script file.
  */
 function stripLeadingBom(value: string): string {
-    return value.replace(/^\uFEFF/, '');
+    return value.startsWith('\uFEFF') ? value.slice(1) : value;
 }
 
 /**

src/DeploymentReport.ts

@@ -74,15 +74,19 @@ export default class Reporter {
     }
 
     /**
-     * Reads a boolean input, returning the default when the input is not set.
-     * Unlike core.getBooleanInput, this does not throw on an empty value.
+     * Reads a boolean input, returning the default when the input is empty or
+     * unrecognized. Unlike core.getBooleanInput, this does not throw on an empty
+     * or unexpected value.
      */
     private static _getBooleanInput(name: string, defaultValue: boolean): boolean {
         const raw = core.getInput(name).trim().toLowerCase();
-        if (raw === '') {
-            return defaultValue;
+        if (raw === 'true') {
+            return true;
         }
-        return raw === 'true';
+        if (raw === 'false') {
+            return false;
+        }
+        return defaultValue;
     }
 
     /**
@@ -98,7 +102,7 @@ export default class Reporter {
             report: this._readReport(result.reportPath),
             script: this._readScript(result.scriptPath),
             durationMs: result.durationMs,
-            options: inputs.additionalArguments,
+            options: this._redactSecrets(inputs.additionalArguments),
             actor: context.actor || undefined,
             commit: context.sha ? context.sha.substring(0, 7) : undefined,
             runUrl: this._buildRunUrl(context)
@@ -116,6 +120,18 @@ export default class Reporter {
         return `${serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}`;
     }
 
+    /**
+     * Redacts additional arguments that may contain secrets before they are
+     * rendered into the summary or a pull request comment, neither of which is
+     * covered by GitHub secret masking.
+     */
+    private static _redactSecrets(args?: string): string | undefined {
+        if (args && /password|pwd|secret|token/i.test(args)) {
+            return '[redacted]';
+        }
+        return args;
+    }
+
     /**
      * Returns a human-readable label for the action that ran.
      */
@@ -163,9 +179,11 @@ export default class Reporter {
      * Publishes the action outputs describing the deployment.
      */
     private static _setOutputs(context: SummaryContext, result: IActionResult): void {
-        const operations = context.report ? context.report.operations : [];
-        core.setOutput('changes-detected', operations.length > 0 ? 'true' : 'false');
-        core.setOutput('objects-changed', operations.length.toString());
+        if (context.report) {
+            const operations = context.report.operations;
+            core.setOutput('changes-detected', operations.length > 0 ? 'true' : 'false');
+            core.setOutput('objects-changed', operations.length.toString());
+        }
 
         if (result.reportPath) {
             core.setOutput('deployment-report-path', result.reportPath);
@@ -216,7 +234,7 @@ export default class Reporter {
      * Finds the id of a previously posted summary comment by its hidden marker.
      */
     private static async _findExistingComment(octokit: ReturnType<typeof github.getOctokit>, owner: string, repo: string, issueNumber: number): Promise<number | undefined> {
-        const { data: comments } = await octokit.rest.issues.listComments({ owner, repo, issue_number: issueNumber, per_page: 100 });
+        const comments = await octokit.paginate(octokit.rest.issues.listComments, { owner, repo, issue_number: issueNumber, per_page: 100 });
         const existing = comments.find(comment => !!comment.body && comment.body.includes(SUMMARY_MARKER));
         return existing ? existing.id : undefined;
     }

src/DeploymentSummary.ts

@@ -96,7 +96,7 @@ function getInputs(): IActionInputs {
                 sqlpackagePath: core.getInput('sqlpackage-path') || undefined,
                 additionalArguments: core.getInput('arguments') || undefined,
                 skipFirewallCheck: core.getBooleanInput('skip-firewall-check'),
-                captureDeploymentReport: true
+                captureDeploymentReport: isReportingEnabled()
             } as IDacpacActionInputs;
 
         case Constants.sqlprojExtension:
@@ -113,12 +113,26 @@ function getInputs(): IActionInputs {
                 sqlpackagePath: core.getInput('sqlpackage-path') || undefined,
                 additionalArguments: core.getInput('arguments') || undefined,
                 skipFirewallCheck: core.getBooleanInput('skip-firewall-check'),
-                captureDeploymentReport: true
+                captureDeploymentReport: isReportingEnabled()
             } as IBuildAndPublishInputs;
 
         default:
             throw new Error(`Invalid file type provided as input ${filePath}. File must be a .sql, .dacpac, or .sqlproj file.`)
     }
 }
 
+/**
+ * Determines whether deployment reporting is enabled, so the action only captures
+ * a deployment report and script when a summary or pull request comment will be
+ * produced. Mirrors the defaults used by the reporter: the summary is on unless
+ * explicitly set to false, and the pull request comment is on unless set to off.
+ */
+function isReportingEnabled(): boolean {
+    const summary = core.getInput('summary').trim().toLowerCase();
+    const commentPr = core.getInput('comment-pr').trim().toLowerCase();
+    const summaryEnabled = summary !== 'false';
+    const commentEnabled = commentPr !== 'off';
+    return summaryEnabled || commentEnabled;
+}
+
 run();