bugfix-319-setup-crash-on-repository-with-no-issues: Improve configuration merging to preserve all undefined stored keys and prevent saving results, adding robustness tests.

efraespada · efraespada · commit 3deee7c779f0 · 2026-02-27T21:13:03.000+01:00
diff --git a/build/cli/index.js b/build/cli/index.js
@@ -52955,15 +52955,6 @@ exports.ConfigurationHandler = void 0;
 const config_1 = __nccwpck_require__(1106);
 const logger_1 = __nccwpck_require__(8836);
 const issue_content_interface_1 = __nccwpck_require__(9913);
-/** Keys that must be preserved from stored config when current has undefined (e.g. when branch already existed). */
-const CONFIG_KEYS_TO_PRESERVE = [
-    'parentBranch',
-    'workingBranch',
-    'releaseBranch',
-    'hotfixBranch',
-    'hotfixOriginBranch',
-    'branchType',
-];
 class ConfigurationHandler extends issue_content_interface_1.IssueContentInterface {
     constructor() {
         super(...arguments);
@@ -52983,7 +52974,8 @@ class ConfigurationHandler extends issue_content_interface_1.IssueContentInterfa
                 if (storedRaw != null && storedRaw.trim().length > 0) {
                     try {
                         const stored = JSON.parse(storedRaw);
-                        for (const key of CONFIG_KEYS_TO_PRESERVE) {
+                        // Merge all fields from stored that are undefined in current payload
+                        for (const key in stored) {
                             if (payload[key] === undefined && stored[key] !== undefined) {
                                 payload[key] = stored[key];
                             }
@@ -52993,6 +52985,8 @@ class ConfigurationHandler extends issue_content_interface_1.IssueContentInterfa
                         /* ignore parse errors, save current as-is */
                     }
                 }
+                // Ensure results is never saved to prevent payload bloat
+                delete payload['results'];
                 return await this.internalUpdate(execution, JSON.stringify(payload, null, 4));
             }
             catch (error) {
diff --git a/build/github_action/index.js b/build/github_action/index.js
@@ -48036,15 +48036,6 @@ exports.ConfigurationHandler = void 0;
 const config_1 = __nccwpck_require__(1106);
 const logger_1 = __nccwpck_require__(8836);
 const issue_content_interface_1 = __nccwpck_require__(9913);
-/** Keys that must be preserved from stored config when current has undefined (e.g. when branch already existed). */
-const CONFIG_KEYS_TO_PRESERVE = [
-    'parentBranch',
-    'workingBranch',
-    'releaseBranch',
-    'hotfixBranch',
-    'hotfixOriginBranch',
-    'branchType',
-];
 class ConfigurationHandler extends issue_content_interface_1.IssueContentInterface {
     constructor() {
         super(...arguments);
@@ -48064,7 +48055,8 @@ class ConfigurationHandler extends issue_content_interface_1.IssueContentInterfa
                 if (storedRaw != null && storedRaw.trim().length > 0) {
                     try {
                         const stored = JSON.parse(storedRaw);
-                        for (const key of CONFIG_KEYS_TO_PRESERVE) {
+                        // Merge all fields from stored that are undefined in current payload
+                        for (const key in stored) {
                             if (payload[key] === undefined && stored[key] !== undefined) {
                                 payload[key] = stored[key];
                             }
@@ -48074,6 +48066,8 @@ class ConfigurationHandler extends issue_content_interface_1.IssueContentInterfa
                         /* ignore parse errors, save current as-is */
                     }
                 }
+                // Ensure results is never saved to prevent payload bloat
+                delete payload['results'];
                 return await this.internalUpdate(execution, JSON.stringify(payload, null, 4));
             }
             catch (error) {
diff --git a/src/manager/description/__tests__/configuration_handler.test.ts b/src/manager/description/__tests__/configuration_handler.test.ts
@@ -106,23 +106,18 @@ describe('ConfigurationHandler', () => {
       expect(updatedDesc).toMatch(/"workingBranch":\s*"feature\/123"/);
     });
 
-    it('preserves stored keys when current has undefined', async () => {
+    it('preserves all stored keys (including unknown ones) when current has undefined', async () => {
       const storedJson = JSON.stringify({
         parentBranch: 'main',
-        releaseBranch: 'release/1',
-        branchType: 'hotfix',
+        unknownKey: 'preserve-me',
+        branchConfiguration: { name: 'leaf' },
       });
       mockGetDescription.mockResolvedValue(descriptionWithConfig(storedJson));
       mockUpdateDescription.mockResolvedValue(undefined);
 
       const execution = minimalExecution({
         currentConfiguration: {
-          branchType: 'feature',
-          releaseBranch: undefined,
-          workingBranch: 'feature/123',
           parentBranch: undefined,
-          hotfixOriginBranch: undefined,
-          hotfixBranch: undefined,
           branchConfiguration: undefined,
         },
       });
@@ -131,8 +126,80 @@ describe('ConfigurationHandler', () => {
 
       expect(mockUpdateDescription).toHaveBeenCalled();
       const fullDesc = mockUpdateDescription.mock.calls[0][3];
-      expect(fullDesc).toContain('"parentBranch": "main"');
-      expect(fullDesc).toContain('"releaseBranch": "release/1"');
+      const parsed = JSON.parse(handler.getContent(fullDesc)!.trim());
+      expect(parsed.parentBranch).toBe('main');
+      expect(parsed.unknownKey).toBe('preserve-me');
+      expect(parsed.branchConfiguration).toEqual({ name: 'leaf' });
+    });
+
+    it('always excludes results from the saved payload even if present in stored', async () => {
+      const storedJson = JSON.stringify({
+        results: [{ some: 'result' }],
+        parentBranch: 'main',
+      });
+      mockGetDescription.mockResolvedValue(descriptionWithConfig(storedJson));
+      mockUpdateDescription.mockResolvedValue(undefined);
+
+      const execution = minimalExecution({
+        currentConfiguration: {
+          parentBranch: 'develop',
+        },
+      });
+
+      await handler.update(execution);
+
+      const fullDesc = mockUpdateDescription.mock.calls[0][3];
+      const parsed = JSON.parse(handler.getContent(fullDesc)!.trim());
+      expect(parsed.results).toBeUndefined();
+      expect(parsed.parentBranch).toBe('develop');
+    });
+
+    it('fails safely when block is mangled (missing end tag)', async () => {
+      const mangledDesc = `body\n${CONFIG_START}\n{"x":1}\nno end tag here`;
+      mockGetDescription.mockResolvedValue(mangledDesc);
+
+      const execution = minimalExecution();
+      const result = await handler.update(execution);
+
+      // Should log error and return undefined instead of corrupting or crashing
+      expect(result).toBeUndefined();
+      const { logError } = require('../../../utils/logger');
+      expect(logError).toHaveBeenCalledWith(expect.stringContaining('problem with open-close tags'));
+    });
+
+    it('handles malformed JSON in stored config gracefully', async () => {
+      mockGetDescription.mockResolvedValue(descriptionWithConfig('invalid { json'));
+      mockUpdateDescription.mockResolvedValue(undefined);
+
+      const execution = minimalExecution({
+        currentConfiguration: {
+          branchType: 'feature',
+          workingBranch: 'feat/new',
+        },
+      });
+
+      await handler.update(execution);
+
+      expect(mockUpdateDescription).toHaveBeenCalled();
+      const fullDesc = mockUpdateDescription.mock.calls[0][3];
+      expect(fullDesc).toContain('"branchType": "feature"');
+      expect(fullDesc).toContain('"workingBranch": "feat/new"');
+    });
+
+    it('handles empty stored config block gracefully', async () => {
+      mockGetDescription.mockResolvedValue(descriptionWithConfig('  '));
+      mockUpdateDescription.mockResolvedValue(undefined);
+
+      const execution = minimalExecution({
+        currentConfiguration: {
+          branchType: 'feature',
+        },
+      });
+
+      await handler.update(execution);
+
+      expect(mockUpdateDescription).toHaveBeenCalled();
+      expect(mockUpdateDescription.mock.calls[0][3]).toContain('"branchType": "feature"');
     });
 
     it('returns undefined on error', async () => {
@@ -144,4 +211,19 @@ describe('ConfigurationHandler', () => {
       expect(result).toBeUndefined();
     });
   });
+
+  describe('edge cases', () => {
+    it('get returns undefined when internalGetter returns empty string', async () => {
+      mockGetDescription.mockResolvedValue('');
+      const execution = minimalExecution();
+      const result = await handler.get(execution);
+      expect(result).toBeUndefined();
+    });
+
+    it('get throws informative error on invalid JSON', async () => {
+      mockGetDescription.mockResolvedValue(descriptionWithConfig('{ "broken": '));
+      const execution = minimalExecution();
+      await expect(handler.get(execution)).rejects.toThrow(/Unexpected end of JSON input|SyntaxError/);
+    });
+  });
 });
diff --git a/src/manager/description/configuration_handler.ts b/src/manager/description/configuration_handler.ts
@@ -3,15 +3,6 @@ import { Execution } from "../../data/model/execution";
 import { logError } from "../../utils/logger";
 import { IssueContentInterface } from "./base/issue_content_interface";
 
-/** Keys that must be preserved from stored config when current has undefined (e.g. when branch already existed). */
-const CONFIG_KEYS_TO_PRESERVE = [
-    'parentBranch',
-    'workingBranch',
-    'releaseBranch',
-    'hotfixBranch',
-    'hotfixOriginBranch',
-    'branchType',
-] as const;
 
 export class ConfigurationHandler extends IssueContentInterface {
     get id(): string {
@@ -39,7 +30,8 @@ export class ConfigurationHandler extends IssueContentInterface {
             if (storedRaw != null && storedRaw.trim().length > 0) {
                 try {
                     const stored = JSON.parse(storedRaw) as Record<string, unknown>;
-                    for (const key of CONFIG_KEYS_TO_PRESERVE) {
+                    // Merge all fields from stored that are undefined in current payload
+                    for (const key in stored) {
                         if (payload[key] === undefined && stored[key] !== undefined) {
                             payload[key] = stored[key];
                         }
@@ -49,6 +41,9 @@ export class ConfigurationHandler extends IssueContentInterface {
                 }
             }
 
+            // Ensure results is never saved to prevent payload bloat
+            delete payload['results'];
+
             return await this.internalUpdate(execution, JSON.stringify(payload, null, 4));
         } catch (error) {
             logError(`Error updating issue description: ${error}`);
