bugfix-309-setup-crash: Improve CLI error handling by ensuring early exit when no valid PERSONAL_ACCESS_TOKEN is provided. Update tests to verify correct logging and exit behavior based on the presence of a .env file, enhancing user guidance during setup.

Author: efraespada

build/cli/index.js

@@ -47664,6 +47664,7 @@ program
             (0, logger_1.logInfo)('   • Or create a .env file in this repo with: PERSONAL_ACCESS_TOKEN=your_github_token');
         }
         process.exit(1);
+        return;
     }
     (0, logger_1.logInfo)('⚙️  Running initial setup (labels, issue types, access)...');
     const params = {

src/__tests__/cli.test.ts

@@ -34,7 +34,16 @@ jest.mock('../data/repository/ai_repository', () => ({
   })),
 }));
 
-jest.mock('../utils/setup_files', () => jest.requireActual<typeof import('../utils/setup_files')>('../utils/setup_files'));
+const mockGetSetupToken = jest.fn();
+const mockSetupEnvFileExists = jest.fn();
+jest.mock('../utils/setup_files', () => {
+  const actual = jest.requireActual<typeof import('../utils/setup_files')>('../utils/setup_files');
+  return {
+    ...actual,
+    getSetupToken: (...args: unknown[]) => mockGetSetupToken(...args),
+    setupEnvFileExists: (...args: unknown[]) => mockSetupEnvFileExists(...args),
+  };
+});
 
 describe('CLI', () => {
   let exitSpy: jest.SpyInstance;
@@ -49,6 +58,11 @@ describe('CLI', () => {
     mockIsIssue.mockResolvedValue(true);
     consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
     consoleLogSpy = jest.spyOn(console, 'log').mockImplementation(() => {});
+    // Default: return override token when present (so setup --token ... still works)
+    mockGetSetupToken.mockImplementation((_cwd: string, override?: string) =>
+      override?.trim() && override.trim().length >= 20 ? override.trim() : undefined
+    );
+    mockSetupEnvFileExists.mockReturnValue(false);
   });
 
   afterEach(() => {
@@ -330,6 +344,34 @@ describe('CLI', () => {
       const ranWithValidRepo = runCalls.length > 0 && runCalls[0][0]?.repo?.owner && runCalls[0][0]?.repo?.repo;
       expect(ranWithValidRepo).not.toBe(true);
     });
+
+    it('exits when no valid token and suggests creating .env when .env does not exist', async () => {
+      mockGetSetupToken.mockReturnValue(undefined);
+      mockSetupEnvFileExists.mockReturnValue(false);
+      const { logError, logInfo } = require('../utils/logger');
+      (runLocalAction as jest.Mock).mockClear();
+
+      await program.parseAsync(['node', 'cli', 'setup']);
+
+      expect(logError).toHaveBeenCalledWith(expect.stringContaining('Setup requires PERSONAL_ACCESS_TOKEN'));
+      expect(logInfo).toHaveBeenCalledWith(expect.stringContaining('create a .env file'));
+      expect(runLocalAction).not.toHaveBeenCalled();
+      expect(exitSpy).toHaveBeenCalledWith(1);
+    });
+
+    it('exits when no valid token and suggests adding to existing .env when .env exists', async () => {
+      mockGetSetupToken.mockReturnValue(undefined);
+      mockSetupEnvFileExists.mockReturnValue(true);
+      const { logError, logInfo } = require('../utils/logger');
+      (runLocalAction as jest.Mock).mockClear();
+
+      await program.parseAsync(['node', 'cli', 'setup']);
+
+      expect(logError).toHaveBeenCalledWith(expect.stringContaining('Setup requires PERSONAL_ACCESS_TOKEN'));
+      expect(logInfo).toHaveBeenCalledWith(expect.stringContaining('existing .env file'));
+      expect(runLocalAction).not.toHaveBeenCalled();
+      expect(exitSpy).toHaveBeenCalledWith(1);
+    });
   });
 
   describe('detect-potential-problems', () => {

src/cli.ts

@@ -456,6 +456,7 @@ program
         logInfo('   • Or create a .env file in this repo with: PERSONAL_ACCESS_TOKEN=your_github_token');
       }
       process.exit(1);
+      return;
     }
 
     logInfo('⚙️  Running initial setup (labels, issue types, access)...');

src/data/repository/__tests__/project_repository.test.ts

@@ -804,15 +804,22 @@ describe("ProjectRepository.setTaskPriority", () => {
     });
 
     it("throws when content id not found for issue (issue/PR not in repo)", async () => {
+        const { logError } = require("../../../utils/logger");
+        (logError as jest.Mock).mockClear();
         mockGraphql.mockResolvedValueOnce({
             repository: { issueOrPullRequest: null },
         });
         await expect(
             repo.setTaskPriority(project, "owner", "repo", 999, "High", "token")
         ).rejects.toThrow("Content ID not found");
+        expect(logError).toHaveBeenCalledWith(
+            expect.stringContaining("999 not found in repository")
+        );
     });
 
     it("throws when project node is null (invalid project ID)", async () => {
+        const { logError } = require("../../../utils/logger");
+        (logError as jest.Mock).mockClear();
         mockGraphql
             .mockResolvedValueOnce({
                 repository: { issueOrPullRequest: { id: "I_issue1" } },
@@ -821,9 +828,14 @@ describe("ProjectRepository.setTaskPriority", () => {
         await expect(
             repo.setTaskPriority(project, "owner", "repo", 1, "High", "token")
         ).rejects.toThrow("Project not found or invalid project ID");
+        expect(logError).toHaveBeenCalledWith(
+            expect.stringContaining("Project not found for ID")
+        );
     });
 
     it("throws when issue/PR not in project yet", async () => {
+        const { logError } = require("../../../utils/logger");
+        (logError as jest.Mock).mockClear();
         mockGraphql
             .mockResolvedValueOnce({
                 repository: { issueOrPullRequest: { id: "I_issue1" } },
@@ -839,5 +851,59 @@ describe("ProjectRepository.setTaskPriority", () => {
         await expect(
             repo.setTaskPriority(project, "owner", "repo", 1, "High", "token")
         ).rejects.toThrow("not in the project yet");
+        expect(logError).toHaveBeenCalledWith(
+            expect.stringContaining("not found in project after checking")
+        );
+    });
+
+    it("logs error when hasNextPage is true but endCursor is null", async () => {
+        const { logError } = require("../../../utils/logger");
+        (logError as jest.Mock).mockClear();
+        mockGraphql
+            .mockResolvedValueOnce({
+                repository: { issueOrPullRequest: { id: "I_issue1" } },
+            })
+            .mockResolvedValueOnce({
+                node: {
+                    items: {
+                        nodes: [{ id: "other", content: { id: "I_other" } }],
+                        pageInfo: { hasNextPage: true, endCursor: null },
+                    },
+                },
+            });
+        await expect(
+            repo.setTaskPriority(project, "owner", "repo", 1, "High", "token")
+        ).rejects.toThrow("not in the project yet");
+        expect(logError).toHaveBeenCalledWith(
+            expect.stringContaining("hasNextPage is true but endCursor is null")
+        );
+    });
+
+    it("stops after maxPages and throws when item not found", async () => {
+        const { logError } = require("../../../utils/logger");
+        (logError as jest.Mock).mockClear();
+        const pageWithNext = {
+            node: {
+                items: {
+                    nodes: Array.from({ length: 100 }, (_, i) => ({
+                        id: `item_${i}`,
+                        content: { id: `I_other_${i}` },
+                    })),
+                    pageInfo: { hasNextPage: true, endCursor: "next" },
+                },
+            },
+        };
+        mockGraphql.mockResolvedValueOnce({
+            repository: { issueOrPullRequest: { id: "I_issue1" } },
+        });
+        for (let p = 0; p < 100; p++) {
+            mockGraphql.mockResolvedValueOnce(pageWithNext);
+        }
+        await expect(
+            repo.setTaskPriority(project, "owner", "repo", 1, "High", "token")
+        ).rejects.toThrow("not in the project yet");
+        expect(logError).toHaveBeenCalledWith(
+            expect.stringContaining("Stopped after 100 pages")
+        );
     });
 });

src/utils/__tests__/setup_files.test.ts

@@ -87,6 +87,16 @@ describe('setup_files', () => {
       expect(fs.readFileSync(path.join(tmpDir, '.github', 'pull_request_template.md'), 'utf8')).toBe('# PR template');
     });
 
+    it('skips pull_request_template.md when destination already exists', () => {
+      fs.mkdirSync(path.join(tmpDir, 'setup'), { recursive: true });
+      fs.mkdirSync(path.join(tmpDir, '.github'), { recursive: true });
+      fs.writeFileSync(path.join(tmpDir, 'setup', 'pull_request_template.md'), '# from setup');
+      fs.writeFileSync(path.join(tmpDir, '.github', 'pull_request_template.md'), '# existing');
+      const result = copySetupFiles(tmpDir, setupDir());
+      expect(result.skipped).toBe(1);
+      expect(fs.readFileSync(path.join(tmpDir, '.github', 'pull_request_template.md'), 'utf8')).toBe('# existing');
+    });
+
     it('does not create .env when no token in env and no .env (only suggests via log)', () => {
       const saved = process.env[ENV_TOKEN_KEY];
       delete process.env[ENV_TOKEN_KEY];