From 8eca2b519582ac57eeef95aaab82235b26ee0f03 Mon Sep 17 00:00:00 2001 From: Efra Espada Date: Sat, 14 Feb 2026 12:54:49 +0100 Subject: [PATCH 01/13] bugfix-309-setup-crash: Refactor loadActionYaml to resolve action.yml from the copilot package root instead of the current working directory. Update type definitions and add documentation for clarity on path resolution in different contexts. --- build/cli/index.js | 8 +++++++- build/cli/src/utils/yml_utils.d.ts | 6 ++++++ build/github_action/src/utils/yml_utils.d.ts | 6 ++++++ src/data/repository/__tests__/issue_repository.test.ts | 2 +- src/utils/yml_utils.ts | 8 +++++++- 5 files changed, 27 insertions(+), 3 deletions(-) diff --git a/build/cli/index.js b/build/cli/index.js index 1eb6a8da..1382f4f4 100755 --- a/build/cli/index.js +++ b/build/cli/index.js @@ -60650,8 +60650,14 @@ exports.getActionInputsWithDefaults = getActionInputsWithDefaults; const fs = __importStar(__nccwpck_require__(7147)); const path = __importStar(__nccwpck_require__(1017)); const yaml = __importStar(__nccwpck_require__(1917)); +/** + * Resolves action.yml from the copilot package root, not cwd. + * When run as CLI from another repo, cwd is that repo; action.yml lives next to the bundle. + * - From source: __dirname is src/utils → ../../action.yml = repo root. + * - From bundle (build/cli): __dirname is bundle dir → ../../action.yml = package root. + */ function loadActionYaml() { - const actionYamlPath = path.join(process.cwd(), 'action.yml'); + const actionYamlPath = path.join(__dirname, '..', '..', 'action.yml'); const yamlContent = fs.readFileSync(actionYamlPath, 'utf8'); return yaml.load(yamlContent); } diff --git a/build/cli/src/utils/yml_utils.d.ts b/build/cli/src/utils/yml_utils.d.ts index 2d4e8817..559a9f0a 100644 --- a/build/cli/src/utils/yml_utils.d.ts +++ b/build/cli/src/utils/yml_utils.d.ts @@ -8,6 +8,12 @@ interface ActionYaml { author: string; inputs: Record; } +/** + * Resolves action.yml from the copilot package root, not cwd. + * When run as CLI from another repo, cwd is that repo; action.yml lives next to the bundle. + * - From source: __dirname is src/utils → ../../action.yml = repo root. + * - From bundle (build/cli): __dirname is bundle dir → ../../action.yml = package root. + */ export declare function loadActionYaml(): ActionYaml; export declare function getActionInputs(): Record; export declare function getActionInputsWithDefaults(): Record; diff --git a/build/github_action/src/utils/yml_utils.d.ts b/build/github_action/src/utils/yml_utils.d.ts index 2d4e8817..559a9f0a 100644 --- a/build/github_action/src/utils/yml_utils.d.ts +++ b/build/github_action/src/utils/yml_utils.d.ts @@ -8,6 +8,12 @@ interface ActionYaml { author: string; inputs: Record; } +/** + * Resolves action.yml from the copilot package root, not cwd. + * When run as CLI from another repo, cwd is that repo; action.yml lives next to the bundle. + * - From source: __dirname is src/utils → ../../action.yml = repo root. + * - From bundle (build/cli): __dirname is bundle dir → ../../action.yml = package root. + */ export declare function loadActionYaml(): ActionYaml; export declare function getActionInputs(): Record; export declare function getActionInputsWithDefaults(): Record; diff --git a/src/data/repository/__tests__/issue_repository.test.ts b/src/data/repository/__tests__/issue_repository.test.ts index 87ce9733..c18d6bc8 100644 --- a/src/data/repository/__tests__/issue_repository.test.ts +++ b/src/data/repository/__tests__/issue_repository.test.ts @@ -625,7 +625,7 @@ describe('IssueRepository', () => { }); describe('createLabel', () => { - it('calls issues.createLabel', async () => { + it('calls issues.createLabel with owner, repo, name, color, description', async () => { mockRest.issues.createLabel.mockResolvedValue(undefined); await repo.createLabel('o', 'r', 'new-label', 'abc123', 'Description', 'token'); expect(mockRest.issues.createLabel).toHaveBeenCalledWith({ diff --git a/src/utils/yml_utils.ts b/src/utils/yml_utils.ts index aea4d191..5b09e851 100644 --- a/src/utils/yml_utils.ts +++ b/src/utils/yml_utils.ts @@ -14,8 +14,14 @@ interface ActionYaml { inputs: Record; } +/** + * Resolves action.yml from the copilot package root, not cwd. + * When run as CLI from another repo, cwd is that repo; action.yml lives next to the bundle. + * - From source: __dirname is src/utils → ../../action.yml = repo root. + * - From bundle (build/cli): __dirname is bundle dir → ../../action.yml = package root. + */ export function loadActionYaml(): ActionYaml { - const actionYamlPath = path.join(process.cwd(), 'action.yml'); + const actionYamlPath = path.join(__dirname, '..', '..', 'action.yml'); const yamlContent = fs.readFileSync(actionYamlPath, 'utf8'); return yaml.load(yamlContent) as ActionYaml; } From 16ff6a581e99897560328d6191d9e4eadc8666e0 Mon Sep 17 00:00:00 2001 From: Efra Espada Date: Sat, 14 Feb 2026 13:41:36 +0100 Subject: [PATCH 02/13] bugfix-309-setup-crash: Enhance setup process by validating PERSONAL_ACCESS_TOKEN availability. Introduce functions to check token presence in environment and .env file, ensuring setup continuity. Update copySetupFiles to handle token checks and improve logging for user guidance on token setup. --- build/cli/index.js | 115 +++++++++-- build/cli/src/utils/setup_files.d.ts | 22 ++- build/github_action/index.js | 100 ++++++++-- .../github_action/src/utils/setup_files.d.ts | 22 ++- src/cli.ts | 15 +- .../__tests__/initial_setup_use_case.test.ts | 25 ++- src/usecase/actions/initial_setup_use_case.ts | 17 +- src/utils/__tests__/setup_files.test.ts | 182 ++++++++++++++++-- src/utils/setup_files.ts | 89 +++++++-- 9 files changed, 519 insertions(+), 68 deletions(-) diff --git a/build/cli/index.js b/build/cli/index.js index 1382f4f4..b749d77b 100755 --- a/build/cli/index.js +++ b/build/cli/index.js @@ -47252,6 +47252,7 @@ const dotenv = __importStar(__nccwpck_require__(2437)); const local_action_1 = __nccwpck_require__(7002); const issue_repository_1 = __nccwpck_require__(57); const constants_1 = __nccwpck_require__(8593); +const setup_files_1 = __nccwpck_require__(1666); const logger_1 = __nccwpck_require__(8836); const prompts_1 = __nccwpck_require__(5554); const ai_1 = __nccwpck_require__(4470); @@ -47650,12 +47651,24 @@ program process.exit(1); } (0, logger_1.logInfo)(`📦 Repository: ${gitInfo.owner}/${gitInfo.repo}`); + if (!(0, setup_files_1.hasValidSetupToken)(cwd)) { + (0, logger_1.logError)('🛑 Setup requires PERSONAL_ACCESS_TOKEN with a valid token.'); + (0, logger_1.logInfo)(' You can:'); + (0, logger_1.logInfo)(' • Add it to your environment: export PERSONAL_ACCESS_TOKEN=your_github_token'); + if ((0, setup_files_1.setupEnvFileExists)(cwd)) { + (0, logger_1.logInfo)(' • Or add PERSONAL_ACCESS_TOKEN=your_github_token to your existing .env file'); + } + else { + (0, logger_1.logInfo)(' • Or create a .env file in this repo with: PERSONAL_ACCESS_TOKEN=your_github_token'); + } + process.exit(1); + } (0, logger_1.logInfo)('⚙️ Running initial setup (labels, issue types, access)...'); const params = { [constants_1.INPUT_KEYS.DEBUG]: options.debug.toString(), [constants_1.INPUT_KEYS.SINGLE_ACTION]: constants_1.ACTIONS.INITIAL_SETUP, [constants_1.INPUT_KEYS.SINGLE_ACTION_ISSUE]: 1, - [constants_1.INPUT_KEYS.TOKEN]: options.token || process.env.PERSONAL_ACCESS_TOKEN, + [constants_1.INPUT_KEYS.TOKEN]: options.token || process.env.PERSONAL_ACCESS_TOKEN || (0, setup_files_1.getSetupToken)(cwd), repo: { owner: gitInfo.owner, repo: gitInfo.repo, @@ -54003,6 +54016,18 @@ class InitialSetupUseCase { (0, setup_files_1.ensureGitHubDirs)(process.cwd()); const filesResult = (0, setup_files_1.copySetupFiles)(process.cwd()); steps.push(`✅ Setup files: ${filesResult.copied} copied, ${filesResult.skipped} already existed`); + if (!(0, setup_files_1.hasValidSetupToken)(process.cwd())) { + (0, logger_1.logInfo)(' 🛑 Setup requires PERSONAL_ACCESS_TOKEN (environment or .env) with a valid token.'); + errors.push('PERSONAL_ACCESS_TOKEN must be set (environment or .env) with a valid token to run setup.'); + results.push(new result_1.Result({ + id: this.taskId, + success: false, + executed: true, + steps: steps, + errors: errors, + })); + return results; + } // 1. Verificar acceso a GitHub con Personal Access Token (0, logger_1.logInfo)('🔐 Checking GitHub access...'); const githubAccessResult = await this.verifyGitHubAccess(param); @@ -60342,6 +60367,10 @@ var __importStar = (this && this.__importStar) || (function () { Object.defineProperty(exports, "__esModule", ({ value: true })); exports.ensureGitHubDirs = ensureGitHubDirs; exports.copySetupFiles = copySetupFiles; +exports.ensureEnvWithToken = ensureEnvWithToken; +exports.getSetupToken = getSetupToken; +exports.hasValidSetupToken = hasValidSetupToken; +exports.setupEnvFileExists = setupEnvFileExists; const fs = __importStar(__nccwpck_require__(7147)); const path = __importStar(__nccwpck_require__(1017)); const logger_1 = __nccwpck_require__(8836); @@ -60370,11 +60399,13 @@ function ensureGitHubDirs(cwd) { * Copy setup files from setup/ to repo (.github/ workflows, ISSUE_TEMPLATE, pull_request_template.md, .env at root). * Skips files that already exist at destination (no overwrite). * Logs each file copied or skipped. No-op if setup/ does not exist. - * @param cwd - Repo root + * By default setup dir is the copilot package root (not cwd), so it works when running from another repo. + * @param cwd - Repo root (destination) + * @param setupDirOverride - Optional path to setup/ folder (for tests). If not set, uses package root. * @returns { copied, skipped } */ -function copySetupFiles(cwd) { - const setupDir = path.join(cwd, 'setup'); +function copySetupFiles(cwd, setupDirOverride) { + const setupDir = setupDirOverride ?? path.join(__dirname, '..', '..', 'setup'); if (!fs.existsSync(setupDir)) return { copied: 0, skipped: 0 }; let copied = 0; @@ -60430,20 +60461,76 @@ function copySetupFiles(cwd) { copied += 1; } } - const envSrc = path.join(setupDir, '.env'); - const envDst = path.join(cwd, '.env'); - if (fs.existsSync(envSrc) && fs.statSync(envSrc).isFile()) { - if (fs.existsSync(envDst)) { - (0, logger_1.logInfo)(' ⏭️ .env already exists; skipping.'); - skipped += 1; + ensureEnvWithToken(cwd); + return { copied, skipped }; +} +const ENV_TOKEN_KEY = 'PERSONAL_ACCESS_TOKEN'; +const ENV_PLACEHOLDER_VALUE = 'github_pat_11..'; +/** Minimum length for a token to be considered "defined" (not placeholder). */ +const MIN_VALID_TOKEN_LENGTH = 20; +function getTokenFromEnvFile(envPath) { + if (!fs.existsSync(envPath) || !fs.statSync(envPath).isFile()) + return null; + const content = fs.readFileSync(envPath, 'utf8'); + const match = content.match(new RegExp(`^${ENV_TOKEN_KEY}=(.+)$`, 'm')); + if (!match) + return null; + const value = match[1].trim().replace(/^["']|["']$/g, ''); + return value.length > 0 ? value : null; +} +/** + * Logs the current state of PERSONAL_ACCESS_TOKEN (environment or .env). Does not create .env. + */ +function ensureEnvWithToken(cwd) { + const envPath = path.join(cwd, '.env'); + const tokenInEnv = process.env[ENV_TOKEN_KEY]?.trim(); + if (tokenInEnv) { + (0, logger_1.logInfo)(' 🔑 PERSONAL_ACCESS_TOKEN is set in environment; .env not needed.'); + return; + } + if (fs.existsSync(envPath)) { + const tokenInFile = getTokenFromEnvFile(envPath); + if (tokenInFile) { + (0, logger_1.logInfo)(' ✅ .env exists and contains PERSONAL_ACCESS_TOKEN.'); } else { - fs.copyFileSync(envSrc, envDst); - (0, logger_1.logInfo)(' ✅ Copied setup/.env → .env'); - copied += 1; + (0, logger_1.logInfo)(' ⚠️ .env exists but PERSONAL_ACCESS_TOKEN is missing or empty.'); } + return; } - return { copied, skipped }; + (0, logger_1.logInfo)(' 💡 You can create a .env file here with PERSONAL_ACCESS_TOKEN=your_token or set it in your environment.'); +} +function isTokenValueValid(token) { + const t = token.trim(); + return (t.length >= MIN_VALID_TOKEN_LENGTH && + t !== ENV_PLACEHOLDER_VALUE && + !t.startsWith('github_pat_11..')); +} +/** + * Returns the PERSONAL_ACCESS_TOKEN to use for setup (from environment or .env in cwd). + * Same resolution order as hasValidSetupToken; returns undefined if no valid token is found. + */ +function getSetupToken(cwd) { + const fromEnv = process.env[ENV_TOKEN_KEY]?.trim(); + if (fromEnv && isTokenValueValid(fromEnv)) + return fromEnv; + const envPath = path.join(cwd, '.env'); + const fromFile = getTokenFromEnvFile(envPath); + if (fromFile !== null && isTokenValueValid(fromFile)) + return fromFile; + return undefined; +} +/** + * Returns true if PERSONAL_ACCESS_TOKEN is available and looks like a real token + * (from environment or .env), not the placeholder. Setup should only continue when this is true. + */ +function hasValidSetupToken(cwd) { + return getSetupToken(cwd) !== undefined; +} +/** Returns true if a .env file exists in the given directory. */ +function setupEnvFileExists(cwd) { + const envPath = path.join(cwd, '.env'); + return fs.existsSync(envPath) && fs.statSync(envPath).isFile(); } diff --git a/build/cli/src/utils/setup_files.d.ts b/build/cli/src/utils/setup_files.d.ts index c870c050..a67e0afa 100644 --- a/build/cli/src/utils/setup_files.d.ts +++ b/build/cli/src/utils/setup_files.d.ts @@ -7,10 +7,28 @@ export declare function ensureGitHubDirs(cwd: string): void; * Copy setup files from setup/ to repo (.github/ workflows, ISSUE_TEMPLATE, pull_request_template.md, .env at root). * Skips files that already exist at destination (no overwrite). * Logs each file copied or skipped. No-op if setup/ does not exist. - * @param cwd - Repo root + * By default setup dir is the copilot package root (not cwd), so it works when running from another repo. + * @param cwd - Repo root (destination) + * @param setupDirOverride - Optional path to setup/ folder (for tests). If not set, uses package root. * @returns { copied, skipped } */ -export declare function copySetupFiles(cwd: string): { +export declare function copySetupFiles(cwd: string, setupDirOverride?: string): { copied: number; skipped: number; }; +/** + * Logs the current state of PERSONAL_ACCESS_TOKEN (environment or .env). Does not create .env. + */ +export declare function ensureEnvWithToken(cwd: string): void; +/** + * Returns the PERSONAL_ACCESS_TOKEN to use for setup (from environment or .env in cwd). + * Same resolution order as hasValidSetupToken; returns undefined if no valid token is found. + */ +export declare function getSetupToken(cwd: string): string | undefined; +/** + * Returns true if PERSONAL_ACCESS_TOKEN is available and looks like a real token + * (from environment or .env), not the placeholder. Setup should only continue when this is true. + */ +export declare function hasValidSetupToken(cwd: string): boolean; +/** Returns true if a .env file exists in the given directory. */ +export declare function setupEnvFileExists(cwd: string): boolean; diff --git a/build/github_action/index.js b/build/github_action/index.js index 1f23b0f5..4df8a867 100644 --- a/build/github_action/index.js +++ b/build/github_action/index.js @@ -49087,6 +49087,18 @@ class InitialSetupUseCase { (0, setup_files_1.ensureGitHubDirs)(process.cwd()); const filesResult = (0, setup_files_1.copySetupFiles)(process.cwd()); steps.push(`✅ Setup files: ${filesResult.copied} copied, ${filesResult.skipped} already existed`); + if (!(0, setup_files_1.hasValidSetupToken)(process.cwd())) { + (0, logger_1.logInfo)(' 🛑 Setup requires PERSONAL_ACCESS_TOKEN (environment or .env) with a valid token.'); + errors.push('PERSONAL_ACCESS_TOKEN must be set (environment or .env) with a valid token to run setup.'); + results.push(new result_1.Result({ + id: this.taskId, + success: false, + executed: true, + steps: steps, + errors: errors, + })); + return results; + } // 1. Verificar acceso a GitHub con Personal Access Token (0, logger_1.logInfo)('🔐 Checking GitHub access...'); const githubAccessResult = await this.verifyGitHubAccess(param); @@ -55824,6 +55836,10 @@ var __importStar = (this && this.__importStar) || (function () { Object.defineProperty(exports, "__esModule", ({ value: true })); exports.ensureGitHubDirs = ensureGitHubDirs; exports.copySetupFiles = copySetupFiles; +exports.ensureEnvWithToken = ensureEnvWithToken; +exports.getSetupToken = getSetupToken; +exports.hasValidSetupToken = hasValidSetupToken; +exports.setupEnvFileExists = setupEnvFileExists; const fs = __importStar(__nccwpck_require__(7147)); const path = __importStar(__nccwpck_require__(1017)); const logger_1 = __nccwpck_require__(8836); @@ -55852,11 +55868,13 @@ function ensureGitHubDirs(cwd) { * Copy setup files from setup/ to repo (.github/ workflows, ISSUE_TEMPLATE, pull_request_template.md, .env at root). * Skips files that already exist at destination (no overwrite). * Logs each file copied or skipped. No-op if setup/ does not exist. - * @param cwd - Repo root + * By default setup dir is the copilot package root (not cwd), so it works when running from another repo. + * @param cwd - Repo root (destination) + * @param setupDirOverride - Optional path to setup/ folder (for tests). If not set, uses package root. * @returns { copied, skipped } */ -function copySetupFiles(cwd) { - const setupDir = path.join(cwd, 'setup'); +function copySetupFiles(cwd, setupDirOverride) { + const setupDir = setupDirOverride ?? path.join(__dirname, '..', '..', 'setup'); if (!fs.existsSync(setupDir)) return { copied: 0, skipped: 0 }; let copied = 0; @@ -55912,20 +55930,76 @@ function copySetupFiles(cwd) { copied += 1; } } - const envSrc = path.join(setupDir, '.env'); - const envDst = path.join(cwd, '.env'); - if (fs.existsSync(envSrc) && fs.statSync(envSrc).isFile()) { - if (fs.existsSync(envDst)) { - (0, logger_1.logInfo)(' ⏭️ .env already exists; skipping.'); - skipped += 1; + ensureEnvWithToken(cwd); + return { copied, skipped }; +} +const ENV_TOKEN_KEY = 'PERSONAL_ACCESS_TOKEN'; +const ENV_PLACEHOLDER_VALUE = 'github_pat_11..'; +/** Minimum length for a token to be considered "defined" (not placeholder). */ +const MIN_VALID_TOKEN_LENGTH = 20; +function getTokenFromEnvFile(envPath) { + if (!fs.existsSync(envPath) || !fs.statSync(envPath).isFile()) + return null; + const content = fs.readFileSync(envPath, 'utf8'); + const match = content.match(new RegExp(`^${ENV_TOKEN_KEY}=(.+)$`, 'm')); + if (!match) + return null; + const value = match[1].trim().replace(/^["']|["']$/g, ''); + return value.length > 0 ? value : null; +} +/** + * Logs the current state of PERSONAL_ACCESS_TOKEN (environment or .env). Does not create .env. + */ +function ensureEnvWithToken(cwd) { + const envPath = path.join(cwd, '.env'); + const tokenInEnv = process.env[ENV_TOKEN_KEY]?.trim(); + if (tokenInEnv) { + (0, logger_1.logInfo)(' 🔑 PERSONAL_ACCESS_TOKEN is set in environment; .env not needed.'); + return; + } + if (fs.existsSync(envPath)) { + const tokenInFile = getTokenFromEnvFile(envPath); + if (tokenInFile) { + (0, logger_1.logInfo)(' ✅ .env exists and contains PERSONAL_ACCESS_TOKEN.'); } else { - fs.copyFileSync(envSrc, envDst); - (0, logger_1.logInfo)(' ✅ Copied setup/.env → .env'); - copied += 1; + (0, logger_1.logInfo)(' ⚠️ .env exists but PERSONAL_ACCESS_TOKEN is missing or empty.'); } + return; } - return { copied, skipped }; + (0, logger_1.logInfo)(' 💡 You can create a .env file here with PERSONAL_ACCESS_TOKEN=your_token or set it in your environment.'); +} +function isTokenValueValid(token) { + const t = token.trim(); + return (t.length >= MIN_VALID_TOKEN_LENGTH && + t !== ENV_PLACEHOLDER_VALUE && + !t.startsWith('github_pat_11..')); +} +/** + * Returns the PERSONAL_ACCESS_TOKEN to use for setup (from environment or .env in cwd). + * Same resolution order as hasValidSetupToken; returns undefined if no valid token is found. + */ +function getSetupToken(cwd) { + const fromEnv = process.env[ENV_TOKEN_KEY]?.trim(); + if (fromEnv && isTokenValueValid(fromEnv)) + return fromEnv; + const envPath = path.join(cwd, '.env'); + const fromFile = getTokenFromEnvFile(envPath); + if (fromFile !== null && isTokenValueValid(fromFile)) + return fromFile; + return undefined; +} +/** + * Returns true if PERSONAL_ACCESS_TOKEN is available and looks like a real token + * (from environment or .env), not the placeholder. Setup should only continue when this is true. + */ +function hasValidSetupToken(cwd) { + return getSetupToken(cwd) !== undefined; +} +/** Returns true if a .env file exists in the given directory. */ +function setupEnvFileExists(cwd) { + const envPath = path.join(cwd, '.env'); + return fs.existsSync(envPath) && fs.statSync(envPath).isFile(); } diff --git a/build/github_action/src/utils/setup_files.d.ts b/build/github_action/src/utils/setup_files.d.ts index c870c050..a67e0afa 100644 --- a/build/github_action/src/utils/setup_files.d.ts +++ b/build/github_action/src/utils/setup_files.d.ts @@ -7,10 +7,28 @@ export declare function ensureGitHubDirs(cwd: string): void; * Copy setup files from setup/ to repo (.github/ workflows, ISSUE_TEMPLATE, pull_request_template.md, .env at root). * Skips files that already exist at destination (no overwrite). * Logs each file copied or skipped. No-op if setup/ does not exist. - * @param cwd - Repo root + * By default setup dir is the copilot package root (not cwd), so it works when running from another repo. + * @param cwd - Repo root (destination) + * @param setupDirOverride - Optional path to setup/ folder (for tests). If not set, uses package root. * @returns { copied, skipped } */ -export declare function copySetupFiles(cwd: string): { +export declare function copySetupFiles(cwd: string, setupDirOverride?: string): { copied: number; skipped: number; }; +/** + * Logs the current state of PERSONAL_ACCESS_TOKEN (environment or .env). Does not create .env. + */ +export declare function ensureEnvWithToken(cwd: string): void; +/** + * Returns the PERSONAL_ACCESS_TOKEN to use for setup (from environment or .env in cwd). + * Same resolution order as hasValidSetupToken; returns undefined if no valid token is found. + */ +export declare function getSetupToken(cwd: string): string | undefined; +/** + * Returns true if PERSONAL_ACCESS_TOKEN is available and looks like a real token + * (from environment or .env), not the placeholder. Setup should only continue when this is true. + */ +export declare function hasValidSetupToken(cwd: string): boolean; +/** Returns true if a .env file exists in the given directory. */ +export declare function setupEnvFileExists(cwd: string): boolean; diff --git a/src/cli.ts b/src/cli.ts index 3eb79519..b130810a 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -6,6 +6,7 @@ import * as dotenv from 'dotenv'; import { runLocalAction } from './actions/local_action'; import { IssueRepository } from './data/repository/issue_repository'; import { ACTIONS, ERRORS, INPUT_KEYS, OPENCODE_DEFAULT_MODEL, TITLE } from './utils/constants'; +import { getSetupToken, hasValidSetupToken, setupEnvFileExists } from './utils/setup_files'; import { logError, logInfo } from './utils/logger'; import { getCliDoPrompt } from './prompts'; import { Ai } from './data/model/ai'; @@ -443,13 +444,25 @@ program } logInfo(`📦 Repository: ${gitInfo.owner}/${gitInfo.repo}`); + if (!hasValidSetupToken(cwd)) { + logError('🛑 Setup requires PERSONAL_ACCESS_TOKEN with a valid token.'); + logInfo(' You can:'); + logInfo(' • Add it to your environment: export PERSONAL_ACCESS_TOKEN=your_github_token'); + if (setupEnvFileExists(cwd)) { + logInfo(' • Or add PERSONAL_ACCESS_TOKEN=your_github_token to your existing .env file'); + } else { + logInfo(' • Or create a .env file in this repo with: PERSONAL_ACCESS_TOKEN=your_github_token'); + } + process.exit(1); + } + logInfo('⚙️ Running initial setup (labels, issue types, access)...'); const params: any = { // eslint-disable-line @typescript-eslint/no-explicit-any -- CLI options map to action inputs [INPUT_KEYS.DEBUG]: options.debug.toString(), [INPUT_KEYS.SINGLE_ACTION]: ACTIONS.INITIAL_SETUP, [INPUT_KEYS.SINGLE_ACTION_ISSUE]: 1, - [INPUT_KEYS.TOKEN]: options.token || process.env.PERSONAL_ACCESS_TOKEN, + [INPUT_KEYS.TOKEN]: options.token || process.env.PERSONAL_ACCESS_TOKEN || getSetupToken(cwd), repo: { owner: gitInfo.owner, repo: gitInfo.repo, diff --git a/src/usecase/actions/__tests__/initial_setup_use_case.test.ts b/src/usecase/actions/__tests__/initial_setup_use_case.test.ts index 7c0bfe19..ff4c5913 100644 --- a/src/usecase/actions/__tests__/initial_setup_use_case.test.ts +++ b/src/usecase/actions/__tests__/initial_setup_use_case.test.ts @@ -13,9 +13,11 @@ jest.mock('../../../utils/task_emoji', () => ({ const mockEnsureGitHubDirs = jest.fn(); const mockCopySetupFiles = jest.fn(); +const mockHasValidSetupToken = jest.fn(); jest.mock('../../../utils/setup_files', () => ({ ensureGitHubDirs: (...args: unknown[]) => mockEnsureGitHubDirs(...args), copySetupFiles: (...args: unknown[]) => mockCopySetupFiles(...args), + hasValidSetupToken: (...args: unknown[]) => mockHasValidSetupToken(...args), })); const mockGetUserFromToken = jest.fn(); @@ -70,17 +72,38 @@ describe('InitialSetupUseCase', () => { useCase = new InitialSetupUseCase(); mockEnsureGitHubDirs.mockClear(); mockCopySetupFiles.mockReturnValue({ copied: 2, skipped: 0 }); + mockHasValidSetupToken.mockReturnValue(true); mockGetUserFromToken.mockResolvedValue('test-user'); mockEnsureLabels.mockResolvedValue({ success: true, created: 0, existing: 5, errors: [] }); mockEnsureProgressLabels.mockResolvedValue({ created: 0, existing: 21, errors: [] }); mockEnsureIssueTypes.mockResolvedValue({ success: true, created: 0, existing: 3, errors: [] }); }); - it('calls ensureGitHubDirs and copySetupFiles with process.cwd()', async () => { + it('calls ensureGitHubDirs, copySetupFiles and hasValidSetupToken with process.cwd()', async () => { const param = baseParam(); await useCase.invoke(param); expect(mockEnsureGitHubDirs).toHaveBeenCalledWith(process.cwd()); expect(mockCopySetupFiles).toHaveBeenCalledWith(process.cwd()); + expect(mockHasValidSetupToken).toHaveBeenCalledWith(process.cwd()); + }); + + it('returns failure and does not continue when hasValidSetupToken is false', async () => { + mockHasValidSetupToken.mockReturnValue(false); + try { + const param = baseParam(); + const results = await useCase.invoke(param); + expect(results).toHaveLength(1); + expect(results[0].success).toBe(false); + expect(results[0].errors).toContain( + 'PERSONAL_ACCESS_TOKEN must be set (environment or .env) with a valid token to run setup.' + ); + expect(results[0].steps).not.toContainEqual( + expect.stringMatching(/GitHub access verified/) + ); + expect(mockHasValidSetupToken).toHaveBeenCalledWith(process.cwd()); + } finally { + mockHasValidSetupToken.mockReturnValue(true); + } }); it('returns success and steps including setup files when all steps succeed', async () => { diff --git a/src/usecase/actions/initial_setup_use_case.ts b/src/usecase/actions/initial_setup_use_case.ts index 6689df5a..beaf865d 100644 --- a/src/usecase/actions/initial_setup_use_case.ts +++ b/src/usecase/actions/initial_setup_use_case.ts @@ -5,7 +5,7 @@ import { Result } from "../../data/model/result"; import { ParamUseCase } from "../base/param_usecase"; import { logError, logInfo } from "../../utils/logger"; import { getTaskEmoji } from "../../utils/task_emoji"; -import { copySetupFiles, ensureGitHubDirs } from "../../utils/setup_files"; +import { copySetupFiles, ensureGitHubDirs, hasValidSetupToken } from "../../utils/setup_files"; export class InitialSetupUseCase implements ParamUseCase { taskId: string = 'InitialSetupUseCase'; @@ -24,6 +24,21 @@ export class InitialSetupUseCase implements ParamUseCase { const filesResult = copySetupFiles(process.cwd()); steps.push(`✅ Setup files: ${filesResult.copied} copied, ${filesResult.skipped} already existed`); + if (!hasValidSetupToken(process.cwd())) { + logInfo(' 🛑 Setup requires PERSONAL_ACCESS_TOKEN (environment or .env) with a valid token.'); + errors.push('PERSONAL_ACCESS_TOKEN must be set (environment or .env) with a valid token to run setup.'); + results.push( + new Result({ + id: this.taskId, + success: false, + executed: true, + steps: steps, + errors: errors, + }) + ); + return results; + } + // 1. Verificar acceso a GitHub con Personal Access Token logInfo('🔐 Checking GitHub access...'); const githubAccessResult = await this.verifyGitHubAccess(param); diff --git a/src/utils/__tests__/setup_files.test.ts b/src/utils/__tests__/setup_files.test.ts index 33655963..b7652f62 100644 --- a/src/utils/__tests__/setup_files.test.ts +++ b/src/utils/__tests__/setup_files.test.ts @@ -1,12 +1,15 @@ import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; -import { ensureGitHubDirs, copySetupFiles } from '../setup_files'; +import { ensureGitHubDirs, copySetupFiles, ensureEnvWithToken, getSetupToken, hasValidSetupToken, setupEnvFileExists } from '../setup_files'; jest.mock('../logger', () => ({ logInfo: jest.fn(), })); +const ENV_TOKEN_KEY = 'PERSONAL_ACCESS_TOKEN'; +const ENV_PLACEHOLDER = 'PERSONAL_ACCESS_TOKEN=github_pat_11..'; + describe('setup_files', () => { let tmpDir: string; @@ -38,8 +41,10 @@ describe('setup_files', () => { }); describe('copySetupFiles', () => { + const setupDir = () => path.join(tmpDir, 'setup'); + it('returns { copied: 0, skipped: 0 } when setup/ does not exist', () => { - const result = copySetupFiles(tmpDir); + const result = copySetupFiles(tmpDir, setupDir()); expect(result).toEqual({ copied: 0, skipped: 0 }); }); @@ -48,7 +53,7 @@ describe('setup_files', () => { fs.mkdirSync(path.join(tmpDir, '.github', 'workflows'), { recursive: true }); const workflowContent = 'name: test'; fs.writeFileSync(path.join(tmpDir, 'setup', 'workflows', 'ci.yml'), workflowContent); - const result = copySetupFiles(tmpDir); + const result = copySetupFiles(tmpDir, setupDir()); expect(result.copied).toBe(1); expect(fs.readFileSync(path.join(tmpDir, '.github', 'workflows', 'ci.yml'), 'utf8')).toBe(workflowContent); }); @@ -58,7 +63,7 @@ describe('setup_files', () => { fs.mkdirSync(path.join(tmpDir, '.github', 'workflows'), { recursive: true }); fs.writeFileSync(path.join(tmpDir, 'setup', 'workflows', 'ci.yml'), 'from-setup'); fs.writeFileSync(path.join(tmpDir, '.github', 'workflows', 'ci.yml'), 'existing'); - const result = copySetupFiles(tmpDir); + const result = copySetupFiles(tmpDir, setupDir()); expect(result.skipped).toBe(1); expect(result.copied).toBe(0); expect(fs.readFileSync(path.join(tmpDir, '.github', 'workflows', 'ci.yml'), 'utf8')).toBe('existing'); @@ -68,7 +73,7 @@ describe('setup_files', () => { fs.mkdirSync(path.join(tmpDir, 'setup', 'ISSUE_TEMPLATE'), { recursive: true }); fs.mkdirSync(path.join(tmpDir, '.github', 'ISSUE_TEMPLATE'), { recursive: true }); fs.writeFileSync(path.join(tmpDir, 'setup', 'ISSUE_TEMPLATE', 'bug_report.yml'), 'title: Bug'); - const result = copySetupFiles(tmpDir); + const result = copySetupFiles(tmpDir, setupDir()); expect(result.copied).toBe(1); expect(fs.readFileSync(path.join(tmpDir, '.github', 'ISSUE_TEMPLATE', 'bug_report.yml'), 'utf8')).toBe('title: Bug'); }); @@ -77,26 +82,34 @@ describe('setup_files', () => { 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'), '# PR template'); - const result = copySetupFiles(tmpDir); + const result = copySetupFiles(tmpDir, setupDir()); expect(result.copied).toBe(1); expect(fs.readFileSync(path.join(tmpDir, '.github', 'pull_request_template.md'), 'utf8')).toBe('# PR template'); }); - it('copies .env when it exists in setup/ and is a file', () => { - fs.mkdirSync(path.join(tmpDir, 'setup'), { recursive: true }); - fs.writeFileSync(path.join(tmpDir, 'setup', '.env'), 'SECRET=xxx'); - const result = copySetupFiles(tmpDir); - expect(result.copied).toBe(1); - expect(fs.readFileSync(path.join(tmpDir, '.env'), 'utf8')).toBe('SECRET=xxx'); + 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]; + try { + fs.mkdirSync(path.join(tmpDir, 'setup'), { recursive: true }); + copySetupFiles(tmpDir, setupDir()); + expect(fs.existsSync(path.join(tmpDir, '.env'))).toBe(false); + } finally { + if (saved !== undefined) process.env[ENV_TOKEN_KEY] = saved; + } }); - it('skips .env when destination .env already exists', () => { - fs.mkdirSync(path.join(tmpDir, 'setup'), { recursive: true }); - fs.writeFileSync(path.join(tmpDir, 'setup', '.env'), 'from-setup'); - fs.writeFileSync(path.join(tmpDir, '.env'), 'existing'); - const result = copySetupFiles(tmpDir); - expect(result.skipped).toBe(1); - expect(fs.readFileSync(path.join(tmpDir, '.env'), 'utf8')).toBe('existing'); + it('does not overwrite .env when it already exists', () => { + const saved = process.env[ENV_TOKEN_KEY]; + delete process.env[ENV_TOKEN_KEY]; + try { + fs.mkdirSync(path.join(tmpDir, 'setup'), { recursive: true }); + fs.writeFileSync(path.join(tmpDir, '.env'), 'PERSONAL_ACCESS_TOKEN=existing_token'); + copySetupFiles(tmpDir, setupDir()); + expect(fs.readFileSync(path.join(tmpDir, '.env'), 'utf8')).toBe('PERSONAL_ACCESS_TOKEN=existing_token'); + } finally { + if (saved !== undefined) process.env[ENV_TOKEN_KEY] = saved; + } }); it('skips existing ISSUE_TEMPLATE file and copies non-existing one', () => { @@ -105,7 +118,7 @@ describe('setup_files', () => { fs.writeFileSync(path.join(tmpDir, 'setup', 'ISSUE_TEMPLATE', 'existing.yml'), 'existing'); fs.writeFileSync(path.join(tmpDir, '.github', 'ISSUE_TEMPLATE', 'existing.yml'), 'already-there'); fs.writeFileSync(path.join(tmpDir, 'setup', 'ISSUE_TEMPLATE', 'new.yml'), 'new'); - const result = copySetupFiles(tmpDir); + const result = copySetupFiles(tmpDir, setupDir()); expect(result.copied).toBe(1); expect(result.skipped).toBe(1); expect(fs.readFileSync(path.join(tmpDir, '.github', 'ISSUE_TEMPLATE', 'existing.yml'), 'utf8')).toBe('already-there'); @@ -116,9 +129,136 @@ describe('setup_files', () => { fs.mkdirSync(path.join(tmpDir, 'setup', 'workflows'), { recursive: true }); fs.mkdirSync(path.join(tmpDir, '.github', 'workflows'), { recursive: true }); fs.mkdirSync(path.join(tmpDir, 'setup', 'workflows', 'ci.yml'), { recursive: true }); - const result = copySetupFiles(tmpDir); + const result = copySetupFiles(tmpDir, setupDir()); expect(result.copied).toBe(0); expect(fs.statSync(path.join(tmpDir, 'setup', 'workflows', 'ci.yml')).isDirectory()).toBe(true); }); }); + + describe('ensureEnvWithToken', () => { + let savedToken: string | undefined; + + beforeEach(() => { + savedToken = process.env[ENV_TOKEN_KEY]; + }); + + afterEach(() => { + if (savedToken !== undefined) { + process.env[ENV_TOKEN_KEY] = savedToken; + } else { + delete process.env[ENV_TOKEN_KEY]; + } + }); + + it('does not create .env when PERSONAL_ACCESS_TOKEN is set in environment', () => { + process.env[ENV_TOKEN_KEY] = 'env_token'; + ensureEnvWithToken(tmpDir); + expect(fs.existsSync(path.join(tmpDir, '.env'))).toBe(false); + }); + + it('does not create .env when no token in env and no .env exists (only suggests via log)', () => { + delete process.env[ENV_TOKEN_KEY]; + ensureEnvWithToken(tmpDir); + expect(fs.existsSync(path.join(tmpDir, '.env'))).toBe(false); + }); + + it('does not overwrite .env when it exists with PERSONAL_ACCESS_TOKEN set', () => { + delete process.env[ENV_TOKEN_KEY]; + fs.writeFileSync(path.join(tmpDir, '.env'), 'PERSONAL_ACCESS_TOKEN=my_gh_token'); + ensureEnvWithToken(tmpDir); + expect(fs.readFileSync(path.join(tmpDir, '.env'), 'utf8')).toBe('PERSONAL_ACCESS_TOKEN=my_gh_token'); + }); + + it('does not overwrite .env when it exists but PERSONAL_ACCESS_TOKEN is empty', () => { + delete process.env[ENV_TOKEN_KEY]; + fs.writeFileSync(path.join(tmpDir, '.env'), 'PERSONAL_ACCESS_TOKEN=\n'); + ensureEnvWithToken(tmpDir); + expect(fs.readFileSync(path.join(tmpDir, '.env'), 'utf8')).toBe('PERSONAL_ACCESS_TOKEN=\n'); + }); + }); + + describe('hasValidSetupToken', () => { + let savedToken: string | undefined; + + beforeEach(() => { + savedToken = process.env[ENV_TOKEN_KEY]; + }); + + afterEach(() => { + if (savedToken !== undefined) { + process.env[ENV_TOKEN_KEY] = savedToken; + } else { + delete process.env[ENV_TOKEN_KEY]; + } + }); + + it('returns true when PERSONAL_ACCESS_TOKEN in env has length >= 20 and is not placeholder', () => { + process.env[ENV_TOKEN_KEY] = 'ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'; + expect(hasValidSetupToken(tmpDir)).toBe(true); + }); + + it('returns false when PERSONAL_ACCESS_TOKEN in env is the placeholder', () => { + process.env[ENV_TOKEN_KEY] = 'github_pat_11..'; + expect(hasValidSetupToken(tmpDir)).toBe(false); + }); + + it('returns false when PERSONAL_ACCESS_TOKEN in env is too short', () => { + process.env[ENV_TOKEN_KEY] = 'short'; + expect(hasValidSetupToken(tmpDir)).toBe(false); + }); + + it('returns true when .env has valid token and env is not set', () => { + delete process.env[ENV_TOKEN_KEY]; + fs.writeFileSync(path.join(tmpDir, '.env'), 'PERSONAL_ACCESS_TOKEN=ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'); + expect(hasValidSetupToken(tmpDir)).toBe(true); + }); + + it('returns false when .env has only placeholder and env is not set', () => { + delete process.env[ENV_TOKEN_KEY]; + fs.writeFileSync(path.join(tmpDir, '.env'), 'PERSONAL_ACCESS_TOKEN=github_pat_11..'); + expect(hasValidSetupToken(tmpDir)).toBe(false); + }); + + it('falls back to .env when env is set but invalid (placeholder); then returns true', () => { + process.env[ENV_TOKEN_KEY] = 'github_pat_11..'; + fs.writeFileSync(path.join(tmpDir, '.env'), 'PERSONAL_ACCESS_TOKEN=ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'); + expect(hasValidSetupToken(tmpDir)).toBe(true); + }); + }); + + describe('getSetupToken', () => { + it('returns token from env when valid', () => { + process.env[ENV_TOKEN_KEY] = 'ghp_abcdefghijklmnopqrstuvwxyz123456'; + expect(getSetupToken(tmpDir)).toBe('ghp_abcdefghijklmnopqrstuvwxyz123456'); + delete process.env[ENV_TOKEN_KEY]; + }); + + it('returns token from .env when env is not set', () => { + delete process.env[ENV_TOKEN_KEY]; + fs.writeFileSync(path.join(tmpDir, '.env'), 'PERSONAL_ACCESS_TOKEN=ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'); + expect(getSetupToken(tmpDir)).toBe('ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'); + }); + + it('returns undefined when no valid token', () => { + delete process.env[ENV_TOKEN_KEY]; + fs.writeFileSync(path.join(tmpDir, '.env'), 'PERSONAL_ACCESS_TOKEN=github_pat_11..'); + expect(getSetupToken(tmpDir)).toBeUndefined(); + }); + }); + + describe('setupEnvFileExists', () => { + it('returns true when .env file exists', () => { + fs.writeFileSync(path.join(tmpDir, '.env'), 'PERSONAL_ACCESS_TOKEN=token'); + expect(setupEnvFileExists(tmpDir)).toBe(true); + }); + + it('returns false when .env does not exist', () => { + expect(setupEnvFileExists(tmpDir)).toBe(false); + }); + + it('returns false when .env is a directory', () => { + fs.mkdirSync(path.join(tmpDir, '.env'), { recursive: true }); + expect(setupEnvFileExists(tmpDir)).toBe(false); + }); + }); }); diff --git a/src/utils/setup_files.ts b/src/utils/setup_files.ts index 50407f5b..3860cb67 100644 --- a/src/utils/setup_files.ts +++ b/src/utils/setup_files.ts @@ -28,11 +28,13 @@ export function ensureGitHubDirs(cwd: string): void { * Copy setup files from setup/ to repo (.github/ workflows, ISSUE_TEMPLATE, pull_request_template.md, .env at root). * Skips files that already exist at destination (no overwrite). * Logs each file copied or skipped. No-op if setup/ does not exist. - * @param cwd - Repo root + * By default setup dir is the copilot package root (not cwd), so it works when running from another repo. + * @param cwd - Repo root (destination) + * @param setupDirOverride - Optional path to setup/ folder (for tests). If not set, uses package root. * @returns { copied, skipped } */ -export function copySetupFiles(cwd: string): { copied: number; skipped: number } { - const setupDir = path.join(cwd, 'setup'); +export function copySetupFiles(cwd: string, setupDirOverride?: string): { copied: number; skipped: number } { + const setupDir = setupDirOverride ?? path.join(__dirname, '..', '..', 'setup'); if (!fs.existsSync(setupDir)) return { copied: 0, skipped: 0 }; let copied = 0; @@ -85,17 +87,78 @@ export function copySetupFiles(cwd: string): { copied: number; skipped: number } copied += 1; } } - const envSrc = path.join(setupDir, '.env'); - const envDst = path.join(cwd, '.env'); - if (fs.existsSync(envSrc) && fs.statSync(envSrc).isFile()) { - if (fs.existsSync(envDst)) { - logInfo(' ⏭️ .env already exists; skipping.'); - skipped += 1; + ensureEnvWithToken(cwd); + return { copied, skipped }; +} + +const ENV_TOKEN_KEY = 'PERSONAL_ACCESS_TOKEN'; +const ENV_PLACEHOLDER_VALUE = 'github_pat_11..'; +/** Minimum length for a token to be considered "defined" (not placeholder). */ +const MIN_VALID_TOKEN_LENGTH = 20; + +function getTokenFromEnvFile(envPath: string): string | null { + if (!fs.existsSync(envPath) || !fs.statSync(envPath).isFile()) return null; + const content = fs.readFileSync(envPath, 'utf8'); + const match = content.match(new RegExp(`^${ENV_TOKEN_KEY}=(.+)$`, 'm')); + if (!match) return null; + const value = match[1].trim().replace(/^["']|["']$/g, ''); + return value.length > 0 ? value : null; +} + +/** + * Logs the current state of PERSONAL_ACCESS_TOKEN (environment or .env). Does not create .env. + */ +export function ensureEnvWithToken(cwd: string): void { + const envPath = path.join(cwd, '.env'); + const tokenInEnv = process.env[ENV_TOKEN_KEY]?.trim(); + if (tokenInEnv) { + logInfo(' 🔑 PERSONAL_ACCESS_TOKEN is set in environment; .env not needed.'); + return; + } + if (fs.existsSync(envPath)) { + const tokenInFile = getTokenFromEnvFile(envPath); + if (tokenInFile) { + logInfo(' ✅ .env exists and contains PERSONAL_ACCESS_TOKEN.'); } else { - fs.copyFileSync(envSrc, envDst); - logInfo(' ✅ Copied setup/.env → .env'); - copied += 1; + logInfo(' ⚠️ .env exists but PERSONAL_ACCESS_TOKEN is missing or empty.'); } + return; } - return { copied, skipped }; + logInfo(' 💡 You can create a .env file here with PERSONAL_ACCESS_TOKEN=your_token or set it in your environment.'); +} + +function isTokenValueValid(token: string): boolean { + const t = token.trim(); + return ( + t.length >= MIN_VALID_TOKEN_LENGTH && + t !== ENV_PLACEHOLDER_VALUE && + !t.startsWith('github_pat_11..') + ); +} + +/** + * Returns the PERSONAL_ACCESS_TOKEN to use for setup (from environment or .env in cwd). + * Same resolution order as hasValidSetupToken; returns undefined if no valid token is found. + */ +export function getSetupToken(cwd: string): string | undefined { + const fromEnv = process.env[ENV_TOKEN_KEY]?.trim(); + if (fromEnv && isTokenValueValid(fromEnv)) return fromEnv; + const envPath = path.join(cwd, '.env'); + const fromFile = getTokenFromEnvFile(envPath); + if (fromFile !== null && isTokenValueValid(fromFile)) return fromFile; + return undefined; +} + +/** + * Returns true if PERSONAL_ACCESS_TOKEN is available and looks like a real token + * (from environment or .env), not the placeholder. Setup should only continue when this is true. + */ +export function hasValidSetupToken(cwd: string): boolean { + return getSetupToken(cwd) !== undefined; +} + +/** Returns true if a .env file exists in the given directory. */ +export function setupEnvFileExists(cwd: string): boolean { + const envPath = path.join(cwd, '.env'); + return fs.existsSync(envPath) && fs.statSync(envPath).isFile(); } From fd735bea93bd1d43179557e35b1f281303ca7460 Mon Sep 17 00:00:00 2001 From: Efra Espada Date: Sat, 14 Feb 2026 13:46:19 +0100 Subject: [PATCH 03/13] bugfix-309-setup-crash: Enhance CLI setup test by verifying token presence in parameters and adding comments for clarity on token validation coverage in related tests. --- src/__tests__/cli.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/__tests__/cli.test.ts b/src/__tests__/cli.test.ts index 1e216405..047997f8 100644 --- a/src/__tests__/cli.test.ts +++ b/src/__tests__/cli.test.ts @@ -140,12 +140,16 @@ describe('CLI', () => { }); describe('setup', () => { + // Token check: hasValidSetupToken/setupEnvFileExists and message variants are covered in + // setup_files.test.ts and initial_setup_use_case.test.ts. Full "exit with proposal" path + // is hard to test here because Commander captures options.token default at CLI load time. it('calls runLocalAction with INITIAL_SETUP', async () => { await program.parseAsync(['node', 'cli', 'setup']); expect(runLocalAction).toHaveBeenCalledTimes(1); const params = (runLocalAction as jest.Mock).mock.calls[0][0]; expect(params[INPUT_KEYS.SINGLE_ACTION]).toBe(ACTIONS.INITIAL_SETUP); + expect(params[INPUT_KEYS.TOKEN]).toBeTruthy(); expect(params[INPUT_KEYS.WELCOME_TITLE]).toContain('Initial Setup'); }); From e11b036f96bea6eb4152a8e966e36a9efe8ef10e Mon Sep 17 00:00:00 2001 From: Efra Espada Date: Sat, 14 Feb 2026 13:55:01 +0100 Subject: [PATCH 04/13] bugfix-309-setup-crash: Add tests for various exit conditions in CLI commands, including handling non-GitHub URLs, errors in `getGitInfo`, and missing issue numbers. Introduce emoji-based title formatting in `IssueRepository` based on labels, ensuring proper representation for different issue types. Improve test coverage for issue state transitions and context fallbacks. --- src/__tests__/cli.test.ts | 184 ++++++++++++++++++ src/data/model/__tests__/issue.test.ts | 41 ++++ .../__tests__/issue_repository.test.ts | 178 +++++++++++++++++ 3 files changed, 403 insertions(+) diff --git a/src/__tests__/cli.test.ts b/src/__tests__/cli.test.ts index 047997f8..a1608da7 100644 --- a/src/__tests__/cli.test.ts +++ b/src/__tests__/cli.test.ts @@ -73,6 +73,15 @@ describe('CLI', () => { expect(exitSpy).toHaveBeenCalledWith(1); }); + it('exits when getGitInfo returns non-GitHub URL', async () => { + (execSync as jest.Mock).mockReturnValue(Buffer.from('https://gitlab.com/foo/bar.git')); + const { logError } = require('../utils/logger'); + + await program.parseAsync(['node', 'cli', 'think', '-q', 'hello']); + + expect(logError).toHaveBeenCalled(); + expect(exitSpy).toHaveBeenCalledWith(1); + }); }); describe('do', () => { @@ -104,6 +113,51 @@ describe('CLI', () => { expect(errMsg).toMatch(/error|Error/i); consoleSpy.mockRestore(); }); + + it('exits when getGitInfo fails in do', async () => { + (execSync as jest.Mock).mockImplementation(() => { + throw new Error('git not found'); + }); + const { logError } = require('../utils/logger'); + (runLocalAction as jest.Mock).mockClear(); + + await program.parseAsync(['node', 'cli', 'do', '-p', 'hello']); + + expect(logError).toHaveBeenCalled(); + expect(exitSpy).toHaveBeenCalledWith(1); + expect(runLocalAction).not.toHaveBeenCalled(); + }); + + it('exits when copilotMessage returns null', async () => { + const { AiRepository } = require('../data/repository/ai_repository'); + AiRepository.mockImplementation(() => ({ + copilotMessage: jest.fn().mockResolvedValue(null), + })); + const consoleSpy = jest.spyOn(console, 'error').mockImplementation(); + + await program.parseAsync(['node', 'cli', 'do', '-p', 'hello']); + + expect(exitSpy).toHaveBeenCalledWith(1); + expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('Request failed')); + consoleSpy.mockRestore(); + }); + + it('logs error and exits with debug when do throws and --debug', async () => { + const err = new Error('OpenCode down'); + const { AiRepository } = require('../data/repository/ai_repository'); + AiRepository.mockImplementation(() => ({ + copilotMessage: jest.fn().mockRejectedValue(err), + })); + const consoleSpy = jest.spyOn(console, 'error').mockImplementation(); + + await program.parseAsync(['node', 'cli', 'do', '-p', 'hello', '--debug']); + + expect(exitSpy).toHaveBeenCalledWith(1); + const messages = consoleSpy.mock.calls.flat().map(String); + expect(messages.some((m) => m.includes('Error executing do'))).toBe(true); + expect(consoleSpy).toHaveBeenCalledWith(err); + consoleSpy.mockRestore(); + }); }); describe('check-progress', () => { @@ -126,6 +180,49 @@ describe('CLI', () => { expect(logSpy).toHaveBeenCalledWith(expect.stringContaining('Invalid issue number')); logSpy.mockRestore(); }); + + it('shows message when issue number is missing', async () => { + const logSpy = jest.spyOn(console, 'log').mockImplementation(); + (runLocalAction as jest.Mock).mockClear(); + + await program.parseAsync(['node', 'cli', 'check-progress']); + + expect(logSpy).toHaveBeenCalledWith(expect.stringContaining('issue number')); + expect(runLocalAction).not.toHaveBeenCalled(); + logSpy.mockRestore(); + }); + + it('exits when getGitInfo fails in check-progress', async () => { + (execSync as jest.Mock).mockImplementation(() => { + throw new Error('git not found'); + }); + const { logError } = require('../utils/logger'); + + await program.parseAsync(['node', 'cli', 'check-progress', '-i', '1']); + + expect(logError).toHaveBeenCalled(); + expect(exitSpy).toHaveBeenCalledWith(1); + }); + + it('passes branch in params when -b is provided', async () => { + await program.parseAsync(['node', 'cli', 'check-progress', '-i', '5', '-b', 'feature/foo']); + + expect(runLocalAction).toHaveBeenCalledTimes(1); + const params = (runLocalAction as jest.Mock).mock.calls[0][0]; + expect(params.commits?.ref).toBe('refs/heads/feature/foo'); + }); + + it('exits when runLocalAction rejects in check-progress', async () => { + (runLocalAction as jest.Mock).mockRejectedValueOnce(new Error('API error')); + const consoleSpy = jest.spyOn(console, 'error').mockImplementation(); + + await program.parseAsync(['node', 'cli', 'check-progress', '-i', '1']); + + expect(exitSpy).toHaveBeenCalledWith(1); + const messages = consoleSpy.mock.calls.flat().map(String); + expect(messages.some((m) => m.includes('Error checking progress'))).toBe(true); + consoleSpy.mockRestore(); + }); }); describe('recommend-steps', () => { @@ -137,6 +234,33 @@ describe('CLI', () => { expect(params[INPUT_KEYS.SINGLE_ACTION]).toBe(ACTIONS.RECOMMEND_STEPS); expect(params.issue?.number).toBe(5); }); + + it('exits when getGitInfo fails', async () => { + (execSync as jest.Mock).mockImplementation(() => { + throw new Error('git not found'); + }); + const { logError } = require('../utils/logger'); + (runLocalAction as jest.Mock).mockClear(); + + await program.parseAsync(['node', 'cli', 'recommend-steps', '-i', '1']); + + expect(logError).toHaveBeenCalled(); + expect(exitSpy).toHaveBeenCalledWith(1); + const runCalls = (runLocalAction as jest.Mock).mock.calls; + const ranWithValidRepo = runCalls.some((c) => c[0]?.repo?.owner && c[0]?.repo?.repo); + expect(ranWithValidRepo).toBe(false); + }); + + it('shows message when issue number is invalid', async () => { + const logSpy = jest.spyOn(console, 'log').mockImplementation(); + (runLocalAction as jest.Mock).mockClear(); + + await program.parseAsync(['node', 'cli', 'recommend-steps', '-i', 'x']); + + expect(logSpy).toHaveBeenCalledWith(expect.stringContaining('valid issue number')); + expect(runLocalAction).not.toHaveBeenCalled(); + logSpy.mockRestore(); + }); }); describe('setup', () => { @@ -165,6 +289,24 @@ describe('CLI', () => { const { logError } = require('../utils/logger'); expect(logError).toHaveBeenCalledWith(expect.stringContaining('Not a git repository')); }); + + it('exits when getGitInfo returns error in setup', async () => { + (execSync as jest.Mock).mockImplementation((cmd: string) => { + if (typeof cmd === 'string' && cmd.includes('is-inside-work-tree')) return Buffer.from('true'); + if (typeof cmd === 'string' && cmd.includes('remote.origin.url')) throw new Error('no remote'); + return Buffer.from('https://github.com/o/r.git'); + }); + const { logError } = require('../utils/logger'); + (runLocalAction as jest.Mock).mockClear(); + + await program.parseAsync(['node', 'cli', 'setup']); + + expect(logError).toHaveBeenCalled(); + expect(exitSpy).toHaveBeenCalledWith(1); + const runCalls = (runLocalAction as jest.Mock).mock.calls; + const ranWithValidRepo = runCalls.length > 0 && runCalls[0][0]?.repo?.owner && runCalls[0][0]?.repo?.repo; + expect(ranWithValidRepo).not.toBe(true); + }); }); describe('detect-potential-problems', () => { @@ -188,6 +330,48 @@ describe('CLI', () => { expect(runLocalAction).not.toHaveBeenCalled(); logSpy.mockRestore(); }); + + it('exits when getGitInfo fails in detect-potential-problems', async () => { + (execSync as jest.Mock).mockImplementation(() => { + throw new Error('git not found'); + }); + const { logError } = require('../utils/logger'); + (runLocalAction as jest.Mock).mockClear(); + + await program.parseAsync(['node', 'cli', 'detect-potential-problems', '-i', '1']); + + expect(logError).toHaveBeenCalled(); + expect(exitSpy).toHaveBeenCalledWith(1); + const runCalls = (runLocalAction as jest.Mock).mock.calls; + const ranWithValidRepo = runCalls.some((c) => c[0]?.repo?.owner && c[0]?.repo?.repo); + expect(ranWithValidRepo).toBe(false); + }); + + it('uses getCurrentBranch when -b is not provided', async () => { + (execSync as jest.Mock).mockImplementation((cmd: string) => { + if (typeof cmd === 'string' && cmd.includes('rev-parse') && cmd.includes('abbrev-ref')) + return Buffer.from('feature/xyz'); + return Buffer.from('https://github.com/test-owner/test-repo.git'); + }); + + await program.parseAsync(['node', 'cli', 'detect-potential-problems', '-i', '3']); + + expect(runLocalAction).toHaveBeenCalledTimes(1); + const params = (runLocalAction as jest.Mock).mock.calls[0][0]; + expect(params.commits?.ref).toBe('refs/heads/feature/xyz'); + }); + + it('exits when runLocalAction rejects in detect-potential-problems', async () => { + (runLocalAction as jest.Mock).mockRejectedValueOnce(new Error('API error')); + const consoleSpy = jest.spyOn(console, 'error').mockImplementation(); + + await program.parseAsync(['node', 'cli', 'detect-potential-problems', '-i', '1']); + + expect(exitSpy).toHaveBeenCalledWith(1); + const messages = consoleSpy.mock.calls.flat().map(String); + expect(messages.some((m) => m.includes('Error running detect-potential-problems'))).toBe(true); + consoleSpy.mockRestore(); + }); }); describe('do --output json', () => { diff --git a/src/data/model/__tests__/issue.test.ts b/src/data/model/__tests__/issue.test.ts index 29c270be..b4b7cd2a 100644 --- a/src/data/model/__tests__/issue.test.ts +++ b/src/data/model/__tests__/issue.test.ts @@ -69,4 +69,45 @@ describe('Issue', () => { expect(i.commentAuthor).toBe('alice'); expect(i.commentUrl).toBe('url'); }); + + it('opened is true when action is reopened', () => { + const inputs = { action: 'reopened', issue: issuePayload, eventName: 'issues' }; + const i = new Issue(false, false, 1, inputs); + expect(i.opened).toBe(true); + }); + + it('opened is false when action is closed', () => { + getContext().payload = { action: 'closed', issue: issuePayload }; + const i = new Issue(false, false, 1, undefined); + expect(i.opened).toBe(false); + }); + + it('falls back to context for creator when inputs.issue has no user', () => { + getContext().payload = { action: 'opened', issue: { ...issuePayload, user: { login: 'context-user' } } }; + const i = new Issue(false, false, 1, { action: 'opened', issue: { title: 'x', number: 1, body: '', html_url: '' }, eventName: 'issues' }); + expect(i.creator).toBe('context-user'); + const i2 = new Issue(false, false, 1, undefined); + expect(i2.creator).toBe('context-user'); + }); + + it('falls back to context for commentBody and commentAuthor when inputs has eventName but comment from context', () => { + getContext().payload = { + action: 'created', + issue: issuePayload, + comment: { id: 99, body: 'From context', user: { login: 'ctx-commenter' }, html_url: 'https://comment.url' }, + }; + getContext().eventName = 'issue_comment'; + const i = new Issue(false, false, 1, { eventName: 'issue_comment', issue: issuePayload }); + expect(i.commentBody).toBe('From context'); + expect(i.commentAuthor).toBe('ctx-commenter'); + expect(i.commentUrl).toBe('https://comment.url'); + expect(i.commentId).toBe(99); + }); + + it('labelAdded falls back to context when inputs has labeled but no label', () => { + getContext().payload = { action: 'labeled', issue: issuePayload, label: { name: 'from-ctx' } }; + const i = new Issue(false, false, 1, undefined); + expect(i.labeled).toBe(true); + expect(i.labelAdded).toBe('from-ctx'); + }); }); diff --git a/src/data/repository/__tests__/issue_repository.test.ts b/src/data/repository/__tests__/issue_repository.test.ts index c18d6bc8..ce105459 100644 --- a/src/data/repository/__tests__/issue_repository.test.ts +++ b/src/data/repository/__tests__/issue_repository.test.ts @@ -168,6 +168,62 @@ describe('IssueRepository', () => { expect(result).toBeUndefined(); expect(mockSetFailed).toHaveBeenCalled(); }); + + it('uses hotfix emoji only (no branched) when labels are hotfix only', async () => { + const labels = makeLabels({ currentIssueLabels: ['hotfix'] }); + mockRest.issues.update.mockResolvedValue(undefined); + const result = await repo.updateTitleIssueFormat('o', 'r', '', 'Fix prod', 1, false, 'x', labels, 'token'); + expect(result).toBe('🔥 - Fix prod'); + }); + + it('uses release emoji only when labels are release only', async () => { + const labels = makeLabels({ currentIssueLabels: ['release'] }); + mockRest.issues.update.mockResolvedValue(undefined); + const result = await repo.updateTitleIssueFormat('o', 'r', '', 'Release v2', 1, false, 'x', labels, 'token'); + expect(result).toBe('🚀 - Release v2'); + }); + + it('uses docs emoji when labels are docs only', async () => { + const labels = makeLabels({ currentIssueLabels: ['docs'] }); + mockRest.issues.update.mockResolvedValue(undefined); + const result = await repo.updateTitleIssueFormat('o', 'r', '', 'Update README', 1, false, 'x', labels, 'token'); + expect(result).toBe('📝 - Update README'); + }); + + it('uses chore emoji when labels are chore only', async () => { + const labels = makeLabels({ currentIssueLabels: ['chore'] }); + mockRest.issues.update.mockResolvedValue(undefined); + const result = await repo.updateTitleIssueFormat('o', 'r', '', 'Bump deps', 1, false, 'x', labels, 'token'); + expect(result).toBe('🔧 - Bump deps'); + }); + + it('uses bugfix emoji when labels are bugfix only', async () => { + const labels = makeLabels({ currentIssueLabels: ['bugfix'] }); + mockRest.issues.update.mockResolvedValue(undefined); + const result = await repo.updateTitleIssueFormat('o', 'r', '', 'Fix crash', 1, false, 'x', labels, 'token'); + expect(result).toBe('🐛 - Fix crash'); + }); + + it('uses feature emoji when labels are feature only', async () => { + const labels = makeLabels({ currentIssueLabels: ['feature'] }); + mockRest.issues.update.mockResolvedValue(undefined); + const result = await repo.updateTitleIssueFormat('o', 'r', '', 'New API', 1, false, 'x', labels, 'token'); + expect(result).toBe('✨ - New API'); + }); + + it('uses help emoji when labels are help only', async () => { + const labels = makeLabels({ currentIssueLabels: ['help'] }); + mockRest.issues.update.mockResolvedValue(undefined); + const result = await repo.updateTitleIssueFormat('o', 'r', '', 'Need support', 1, false, 'x', labels, 'token'); + expect(result).toBe('🆘 - Need support'); + }); + + it('uses question emoji when labels are question only', async () => { + const labels = makeLabels({ currentIssueLabels: ['question'] }); + mockRest.issues.update.mockResolvedValue(undefined); + const result = await repo.updateTitleIssueFormat('o', 'r', '', 'How to X', 1, false, 'x', labels, 'token'); + expect(result).toBe('❓ - How to X'); + }); }); describe('updateTitlePullRequestFormat', () => { @@ -231,6 +287,50 @@ describe('IssueRepository', () => { expect(result).toBeUndefined(); expect(mockSetFailed).toHaveBeenCalled(); }); + + it('uses hotfix emoji only when labels are hotfix only', async () => { + const labels = makeLabels({ currentIssueLabels: ['hotfix'] }); + mockRest.issues.update.mockResolvedValue(undefined); + const result = await repo.updateTitlePullRequestFormat('o', 'r', 'PR title', 'Fix', 1, 1, false, 'x', labels, 'token'); + expect(result).toBe('[#1] 🔥 - Fix'); + }); + + it('uses release emoji when labels are release only', async () => { + const labels = makeLabels({ currentIssueLabels: ['release'] }); + mockRest.issues.update.mockResolvedValue(undefined); + const result = await repo.updateTitlePullRequestFormat('o', 'r', 'PR', 'Release', 1, 1, false, 'x', labels, 'token'); + expect(result).toBe('[#1] 🚀 - Release'); + }); + + it('uses docs and chore emoji when labels are docs or chore only', async () => { + const labelsDocs = makeLabels({ currentIssueLabels: ['docs'] }); + mockRest.issues.update.mockResolvedValue(undefined); + const r1 = await repo.updateTitlePullRequestFormat('o', 'r', 'PR', 'Docs', 1, 1, false, 'x', labelsDocs, 'token'); + expect(r1).toBe('[#1] 📝 - Docs'); + const labelsChore = makeLabels({ currentIssueLabels: ['chore'] }); + const r2 = await repo.updateTitlePullRequestFormat('o', 'r', 'PR', 'Chore', 1, 1, false, 'x', labelsChore, 'token'); + expect(r2).toBe('[#1] 🔧 - Chore'); + }); + + it('uses bugfix and feature emoji when labels only', async () => { + const labelsBug = makeLabels({ currentIssueLabels: ['bugfix'] }); + mockRest.issues.update.mockResolvedValue(undefined); + const r1 = await repo.updateTitlePullRequestFormat('o', 'r', 'PR', 'Fix', 1, 1, false, 'x', labelsBug, 'token'); + expect(r1).toBe('[#1] 🐛 - Fix'); + const labelsFeat = makeLabels({ currentIssueLabels: ['feature'] }); + const r2 = await repo.updateTitlePullRequestFormat('o', 'r', 'PR', 'Feature', 1, 1, false, 'x', labelsFeat, 'token'); + expect(r2).toBe('[#1] ✨ - Feature'); + }); + + it('uses help and question emoji when labels only', async () => { + const labelsHelp = makeLabels({ currentIssueLabels: ['help'] }); + mockRest.issues.update.mockResolvedValue(undefined); + const r1 = await repo.updateTitlePullRequestFormat('o', 'r', 'PR', 'Help', 1, 1, false, 'x', labelsHelp, 'token'); + expect(r1).toBe('[#1] 🆘 - Help'); + const labelsQ = makeLabels({ currentIssueLabels: ['question'] }); + const r2 = await repo.updateTitlePullRequestFormat('o', 'r', 'PR', 'Question', 1, 1, false, 'x', labelsQ, 'token'); + expect(r2).toBe('[#1] ❓ - Question'); + }); }); describe('getDescription', () => { @@ -767,6 +867,54 @@ describe('IssueRepository', () => { repo.setIssueType('org', 'repo', 1, labels, issueTypes, 'token') ).rejects.toThrow('GraphQL error'); }); + + it('sets documentation issue type when labels are docs', async () => { + const labels = makeLabels({ currentIssueLabels: ['docs'] }); + mockGraphql + .mockResolvedValueOnce({ repository: { issue: { id: 'I_1' } } }) + .mockResolvedValueOnce({ + organization: { id: 'O_1', issueTypes: { nodes: [{ id: 'T_DOCS', name: 'Docs' }] } }, + }) + .mockResolvedValueOnce({ updateIssueIssueType: { issue: { id: 'I_1' } } }); + await repo.setIssueType('org', 'repo', 1, labels, issueTypes, 'token'); + expect(mockGraphql).toHaveBeenCalledTimes(3); + }); + + it('sets maintenance issue type when labels are chore', async () => { + const labels = makeLabels({ currentIssueLabels: ['chore'] }); + mockGraphql + .mockResolvedValueOnce({ repository: { issue: { id: 'I_1' } } }) + .mockResolvedValueOnce({ + organization: { id: 'O_1', issueTypes: { nodes: [{ id: 'T_MAINT', name: 'Maintenance' }] } }, + }) + .mockResolvedValueOnce({ updateIssueIssueType: { issue: { id: 'I_1' } } }); + await repo.setIssueType('org', 'repo', 1, labels, issueTypes, 'token'); + expect(mockGraphql).toHaveBeenCalledTimes(3); + }); + + it('sets help issue type when labels are help', async () => { + const labels = makeLabels({ currentIssueLabels: ['help'] }); + mockGraphql + .mockResolvedValueOnce({ repository: { issue: { id: 'I_1' } } }) + .mockResolvedValueOnce({ + organization: { id: 'O_1', issueTypes: { nodes: [{ id: 'T_HELP', name: 'Help' }] } }, + }) + .mockResolvedValueOnce({ updateIssueIssueType: { issue: { id: 'I_1' } } }); + await repo.setIssueType('org', 'repo', 1, labels, issueTypes, 'token'); + expect(mockGraphql).toHaveBeenCalledTimes(3); + }); + + it('sets question issue type when labels are question', async () => { + const labels = makeLabels({ currentIssueLabels: ['question'] }); + mockGraphql + .mockResolvedValueOnce({ repository: { issue: { id: 'I_1' } } }) + .mockResolvedValueOnce({ + organization: { id: 'O_1', issueTypes: { nodes: [{ id: 'T_Q', name: 'Question' }] } }, + }) + .mockResolvedValueOnce({ updateIssueIssueType: { issue: { id: 'I_1' } } }); + await repo.setIssueType('org', 'repo', 1, labels, issueTypes, 'token'); + expect(mockGraphql).toHaveBeenCalledTimes(3); + }); }); describe('ensureLabels', () => { @@ -943,6 +1091,36 @@ describe('IssueRepository', () => { const result = await repo.ensureIssueTypes('org', issueTypesForTest, 'token'); expect(result.errors.length).toBeGreaterThan(0); }); + + it('pushes error message from thrown error in ensureIssueTypes', async () => { + const issueTypesForTest = new IssueTypes( + 'Task', 'Task desc', 'BLUE', + 'Bug', 'Bug desc', 'RED', + 'Feature', 'Feature desc', 'GREEN', + 'Docs', 'Docs desc', 'GREY', + 'Maintenance', 'Maint desc', 'GREY', + 'Hotfix', 'Hotfix desc', 'RED', + 'Release', 'Release desc', 'BLUE', + 'Question', 'Q desc', 'PURPLE', + 'Help', 'Help desc', 'PURPLE' + ); + let callCount = 0; + mockGraphql.mockImplementation(() => { + callCount++; + if (callCount === 1) { + return Promise.resolve({ + organization: { id: 'O_1', issueTypes: { nodes: [{ id: 'T1', name: 'Task' }] } }, + }); + } + if (callCount === 2) { + return Promise.resolve({ organization: { id: 'O_1', issueTypes: { nodes: [] } } }); + } + return Promise.reject(new Error('Create failed')); + }); + const result = await repo.ensureIssueTypes('org', issueTypesForTest, 'token'); + expect(result.errors.length).toBeGreaterThan(0); + expect(result.errors.some((e) => e.includes('Create failed'))).toBe(true); + }); }); }); From 45a4b8727421f78012934f5c68c856f5f9615797 Mon Sep 17 00:00:00 2001 From: Efra Espada Date: Sat, 14 Feb 2026 13:59:48 +0100 Subject: [PATCH 05/13] bugfix-309-setup-crash: Add publicUrl method to ProjectDetail class and update related use cases to utilize it. Enhance tests for publicUrl functionality to ensure correct URL generation based on project details. --- .../model/__tests__/project_detail.test.ts | 22 +++++++++++++++++++ src/data/model/project_detail.ts | 12 ++++++++++ .../check_priority_issue_size_use_case.ts | 2 +- .../steps/issue/move_issue_to_in_progress.ts | 2 +- ...eck_priority_pull_request_size_use_case.ts | 2 +- 5 files changed, 37 insertions(+), 3 deletions(-) diff --git a/src/data/model/__tests__/project_detail.test.ts b/src/data/model/__tests__/project_detail.test.ts index 7555a9bf..a9ad6475 100644 --- a/src/data/model/__tests__/project_detail.test.ts +++ b/src/data/model/__tests__/project_detail.test.ts @@ -28,4 +28,26 @@ describe('ProjectDetail', () => { expect(p.url).toBe(''); expect(p.number).toBe(-1); }); + + describe('publicUrl', () => { + it('returns url when set and valid (https)', () => { + const p = new ProjectDetail({ + url: 'https://github.com/orgs/myorg/projects/2', + type: 'organization', + owner: 'myorg', + number: 2, + }); + expect(p.publicUrl).toBe('https://github.com/orgs/myorg/projects/2'); + }); + + it('builds URL from type, owner and number when url is empty', () => { + const p = new ProjectDetail({ type: 'organization', owner: 'acme', number: 1 }); + expect(p.publicUrl).toBe('https://github.com/orgs/acme/projects/1'); + }); + + it('builds users URL when type is user', () => { + const p = new ProjectDetail({ type: 'user', owner: 'jane', number: 3 }); + expect(p.publicUrl).toBe('https://github.com/users/jane/projects/3'); + }); + }); }); diff --git a/src/data/model/project_detail.ts b/src/data/model/project_detail.ts index c3a7f7df..0abc4cfa 100644 --- a/src/data/model/project_detail.ts +++ b/src/data/model/project_detail.ts @@ -15,4 +15,16 @@ export class ProjectDetail { this.url = data[`url`] ?? ''; this.number = data[`number`] ?? -1; } + + /** + * Returns the full public URL to the project (board). + * Uses the URL from the API when present and valid; otherwise builds it from owner, type and number. + */ + get publicUrl(): string { + if (this.url && typeof this.url === 'string' && this.url.startsWith('https://')) { + return this.url; + } + const path = this.type === 'organization' ? 'orgs' : 'users'; + return `https://github.com/${path}/${this.owner}/projects/${this.number}`; + } } \ No newline at end of file diff --git a/src/usecase/steps/issue/check_priority_issue_size_use_case.ts b/src/usecase/steps/issue/check_priority_issue_size_use_case.ts index 28f4dec7..773fa0eb 100644 --- a/src/usecase/steps/issue/check_priority_issue_size_use_case.ts +++ b/src/usecase/steps/issue/check_priority_issue_size_use_case.ts @@ -67,7 +67,7 @@ export class CheckPriorityIssueSizeUseCase implements ParamUseCase Date: Sat, 14 Feb 2026 14:05:10 +0100 Subject: [PATCH 06/13] bugfix-309-setup-crash: Add publicUrl method to ProjectDetail class and update related use cases to utilize it. Enhance tests to verify correct URL generation when project URL is absent. --- build/cli/index.js | 17 +++++++++++--- build/cli/src/data/model/project_detail.d.ts | 5 ++++ build/github_action/index.js | 17 +++++++++++--- .../src/data/model/project_detail.d.ts | 5 ++++ ...check_priority_issue_size_use_case.test.ts | 23 +++++++++++++++++++ ...move_issue_to_in_progress_use_case.test.ts | 23 +++++++++++++++++++ ...riority_pull_request_size_use_case.test.ts | 23 +++++++++++++++++++ 7 files changed, 107 insertions(+), 6 deletions(-) diff --git a/build/cli/index.js b/build/cli/index.js index b749d77b..9c679ce0 100755 --- a/build/cli/index.js +++ b/build/cli/index.js @@ -48725,6 +48725,17 @@ class ProjectDetail { this.url = data[`url`] ?? ''; this.number = data[`number`] ?? -1; } + /** + * Returns the full public URL to the project (board). + * Uses the URL from the API when present and valid; otherwise builds it from owner, type and number. + */ + get publicUrl() { + if (this.url && typeof this.url === 'string' && this.url.startsWith('https://')) { + return this.url; + } + const path = this.type === 'organization' ? 'orgs' : 'users'; + return `https://github.com/${path}/${this.owner}/projects/${this.number}`; + } } exports.ProjectDetail = ProjectDetail; @@ -57948,7 +57959,7 @@ class CheckPriorityIssueSizeUseCase { success: true, executed: true, steps: [ - `Priority set to \`${priorityLabel}\` in [${project.title}](https://github.com/${param.owner}/${param.repo}/projects/${project.id}).`, + `Priority set to \`${priorityLabel}\` in [${project.title}](${project.publicUrl}).`, ], })); } @@ -58389,7 +58400,7 @@ class MoveIssueToInProgressUseCase { success: true, executed: true, steps: [ - `Moved issue to \`${columnName}\` in [${project.title}](https://github.com/${param.owner}/${param.repo}/projects/${project.id}).`, + `Moved issue to \`${columnName}\` in [${project.title}](${project.publicUrl}).`, ], })); } @@ -59141,7 +59152,7 @@ class CheckPriorityPullRequestSizeUseCase { success: true, executed: true, steps: [ - `Priority set to \`${priorityLabel}\` in [${project.title}](https://github.com/${param.owner}/${param.repo}/projects/${project.id}).`, + `Priority set to \`${priorityLabel}\` in [${project.title}](${project.publicUrl}).`, ], })); } diff --git a/build/cli/src/data/model/project_detail.d.ts b/build/cli/src/data/model/project_detail.d.ts index 719d5033..a72054d6 100644 --- a/build/cli/src/data/model/project_detail.d.ts +++ b/build/cli/src/data/model/project_detail.d.ts @@ -6,4 +6,9 @@ export declare class ProjectDetail { url: string; number: number; constructor(data: any); + /** + * Returns the full public URL to the project (board). + * Uses the URL from the API when present and valid; otherwise builds it from owner, type and number. + */ + get publicUrl(): string; } diff --git a/build/github_action/index.js b/build/github_action/index.js index 4df8a867..eded4d3d 100644 --- a/build/github_action/index.js +++ b/build/github_action/index.js @@ -43814,6 +43814,17 @@ class ProjectDetail { this.url = data[`url`] ?? ''; this.number = data[`number`] ?? -1; } + /** + * Returns the full public URL to the project (board). + * Uses the URL from the API when present and valid; otherwise builds it from owner, type and number. + */ + get publicUrl() { + if (this.url && typeof this.url === 'string' && this.url.startsWith('https://')) { + return this.url; + } + const path = this.type === 'organization' ? 'orgs' : 'users'; + return `https://github.com/${path}/${this.owner}/projects/${this.number}`; + } } exports.ProjectDetail = ProjectDetail; @@ -53238,7 +53249,7 @@ class CheckPriorityIssueSizeUseCase { success: true, executed: true, steps: [ - `Priority set to \`${priorityLabel}\` in [${project.title}](https://github.com/${param.owner}/${param.repo}/projects/${project.id}).`, + `Priority set to \`${priorityLabel}\` in [${project.title}](${project.publicUrl}).`, ], })); } @@ -53679,7 +53690,7 @@ class MoveIssueToInProgressUseCase { success: true, executed: true, steps: [ - `Moved issue to \`${columnName}\` in [${project.title}](https://github.com/${param.owner}/${param.repo}/projects/${project.id}).`, + `Moved issue to \`${columnName}\` in [${project.title}](${project.publicUrl}).`, ], })); } @@ -54431,7 +54442,7 @@ class CheckPriorityPullRequestSizeUseCase { success: true, executed: true, steps: [ - `Priority set to \`${priorityLabel}\` in [${project.title}](https://github.com/${param.owner}/${param.repo}/projects/${project.id}).`, + `Priority set to \`${priorityLabel}\` in [${project.title}](${project.publicUrl}).`, ], })); } diff --git a/build/github_action/src/data/model/project_detail.d.ts b/build/github_action/src/data/model/project_detail.d.ts index 719d5033..a72054d6 100644 --- a/build/github_action/src/data/model/project_detail.d.ts +++ b/build/github_action/src/data/model/project_detail.d.ts @@ -6,4 +6,9 @@ export declare class ProjectDetail { url: string; number: number; constructor(data: any); + /** + * Returns the full public URL to the project (board). + * Uses the URL from the API when present and valid; otherwise builds it from owner, type and number. + */ + get publicUrl(): string; } diff --git a/src/usecase/steps/issue/__tests__/check_priority_issue_size_use_case.test.ts b/src/usecase/steps/issue/__tests__/check_priority_issue_size_use_case.test.ts index 046bbe61..caa56cfc 100644 --- a/src/usecase/steps/issue/__tests__/check_priority_issue_size_use_case.test.ts +++ b/src/usecase/steps/issue/__tests__/check_priority_issue_size_use_case.test.ts @@ -1,3 +1,4 @@ +import { ProjectDetail } from '../../../../data/model/project_detail'; import { CheckPriorityIssueSizeUseCase } from '../check_priority_issue_size_use_case'; jest.mock('../../../../utils/logger', () => ({ @@ -144,4 +145,26 @@ describe('CheckPriorityIssueSizeUseCase', () => { expect(results[0].executed).toBe(true); expect(mockSetTaskPriority).toHaveBeenCalled(); }); + + it('step message contains built project URL when project has no url', async () => { + mockSetTaskPriority.mockResolvedValue(true); + const projectNoUrl = new ProjectDetail({ + id: 'p1', + title: 'Board', + type: 'user', + owner: 'jane', + url: '', + number: 2, + }); + const param = baseParam({ + project: { getProjects: () => [projectNoUrl] }, + }); + + const results = await useCase.invoke(param); + + const builtUrl = 'https://github.com/users/jane/projects/2'; + expect(results[0].success).toBe(true); + expect(results[0].steps?.some((s) => s.includes(builtUrl))).toBe(true); + expect(results[0].steps?.some((s) => s.includes('[Board]'))).toBe(true); + }); }); diff --git a/src/usecase/steps/issue/__tests__/move_issue_to_in_progress_use_case.test.ts b/src/usecase/steps/issue/__tests__/move_issue_to_in_progress_use_case.test.ts index 628e8b3d..34970314 100644 --- a/src/usecase/steps/issue/__tests__/move_issue_to_in_progress_use_case.test.ts +++ b/src/usecase/steps/issue/__tests__/move_issue_to_in_progress_use_case.test.ts @@ -1,3 +1,4 @@ +import { ProjectDetail } from '../../../../data/model/project_detail'; import { MoveIssueToInProgressUseCase } from '../move_issue_to_in_progress'; jest.mock('../../../../utils/logger', () => ({ @@ -72,4 +73,26 @@ describe('MoveIssueToInProgressUseCase', () => { expect(results[0].success).toBe(false); expect(results[0].steps?.some((s) => s.includes('problem'))).toBe(true); }); + + it('step message contains built project URL when project has no url', async () => { + const projectNoUrl = new ProjectDetail({ + id: 'p1', + title: 'Backlog', + type: 'organization', + owner: 'acme', + url: '', + number: 3, + }); + const param = baseParam({ + project: { + getProjects: () => [projectNoUrl], + getProjectColumnIssueInProgress: () => 'In Progress', + }, + }); + const results = await useCase.invoke(param); + const builtUrl = 'https://github.com/orgs/acme/projects/3'; + expect(results[0].success).toBe(true); + expect(results[0].steps?.some((s) => s.includes(builtUrl))).toBe(true); + expect(results[0].steps?.some((s) => s.includes('[Backlog]'))).toBe(true); + }); }); diff --git a/src/usecase/steps/pull_request/__tests__/check_priority_pull_request_size_use_case.test.ts b/src/usecase/steps/pull_request/__tests__/check_priority_pull_request_size_use_case.test.ts index ed543d50..54bf30ce 100644 --- a/src/usecase/steps/pull_request/__tests__/check_priority_pull_request_size_use_case.test.ts +++ b/src/usecase/steps/pull_request/__tests__/check_priority_pull_request_size_use_case.test.ts @@ -1,3 +1,4 @@ +import { ProjectDetail } from '../../../../data/model/project_detail'; import { CheckPriorityPullRequestSizeUseCase } from '../check_priority_pull_request_size_use_case'; jest.mock('../../../../utils/logger', () => ({ @@ -158,4 +159,26 @@ describe('CheckPriorityPullRequestSizeUseCase', () => { expect(results[0].executed).toBe(true); expect(results[0].steps).toContain('Tried to check the priority of the issue, but there was a problem.'); }); + + it('step message contains built project URL when project has no url', async () => { + mockSetTaskPriority.mockResolvedValue(true); + const projectNoUrl = new ProjectDetail({ + id: 'p1', + title: 'Sprint', + type: 'organization', + owner: 'acme', + url: '', + number: 5, + }); + const param = baseParam({ + project: { getProjects: () => [projectNoUrl] }, + }); + + const results = await useCase.invoke(param); + + const builtUrl = 'https://github.com/orgs/acme/projects/5'; + expect(results[0].success).toBe(true); + expect(results[0].steps?.some((s) => s.includes(builtUrl))).toBe(true); + expect(results[0].steps?.some((s) => s.includes('[Sprint]'))).toBe(true); + }); }); From 556172c399e1c8ffdb75914365499b33932edbb5 Mon Sep 17 00:00:00 2001 From: Efra Espada Date: Sat, 14 Feb 2026 19:06:41 +0100 Subject: [PATCH 07/13] bugfix-309-setup-crash: Enhance CLI setup process by allowing token provision via command line, improving user guidance on token requirements. Update tests to verify behavior when token is provided, ensuring proper handling of setup conditions. --- src/__tests__/cli.test.ts | 30 ++++++++++++++++++++++++++++-- src/cli.ts | 4 +++- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/__tests__/cli.test.ts b/src/__tests__/cli.test.ts index a1608da7..e2f31b16 100644 --- a/src/__tests__/cli.test.ts +++ b/src/__tests__/cli.test.ts @@ -34,8 +34,18 @@ jest.mock('../data/repository/ai_repository', () => ({ })), })); +jest.mock('../utils/setup_files', () => { + const actual = jest.requireActual('../utils/setup_files'); + return { + ...actual, + hasValidSetupToken: jest.fn((cwd: string) => actual.hasValidSetupToken(cwd)), + }; +}); + describe('CLI', () => { let exitSpy: jest.SpyInstance; + let consoleErrorSpy: jest.SpyInstance; + let consoleLogSpy: jest.SpyInstance; beforeEach(() => { jest.clearAllMocks(); @@ -43,10 +53,14 @@ describe('CLI', () => { (execSync as jest.Mock).mockReturnValue(Buffer.from('https://github.com/test-owner/test-repo.git')); (runLocalAction as jest.Mock).mockResolvedValue(undefined); mockIsIssue.mockResolvedValue(true); + consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + consoleLogSpy = jest.spyOn(console, 'log').mockImplementation(() => {}); }); afterEach(() => { exitSpy?.mockRestore(); + consoleErrorSpy?.mockRestore(); + consoleLogSpy?.mockRestore(); }); describe('think', () => { @@ -265,8 +279,7 @@ describe('CLI', () => { describe('setup', () => { // Token check: hasValidSetupToken/setupEnvFileExists and message variants are covered in - // setup_files.test.ts and initial_setup_use_case.test.ts. Full "exit with proposal" path - // is hard to test here because Commander captures options.token default at CLI load time. + // setup_files.test.ts and initial_setup_use_case.test.ts. it('calls runLocalAction with INITIAL_SETUP', async () => { await program.parseAsync(['node', 'cli', 'setup']); @@ -277,6 +290,19 @@ describe('CLI', () => { expect(params[INPUT_KEYS.WELCOME_TITLE]).toContain('Initial Setup'); }); + it('proceeds when --token is provided even if env/.env has no token', async () => { + const { hasValidSetupToken } = require('../utils/setup_files'); + (hasValidSetupToken as jest.Mock).mockReturnValue(false); + + await program.parseAsync(['node', 'cli', 'setup', '--token', 'ghp_abcdefghijklmnopqrstuvwxyz12']); + + expect(exitSpy).not.toHaveBeenCalled(); + expect(runLocalAction).toHaveBeenCalledTimes(1); + const params = (runLocalAction as jest.Mock).mock.calls[0][0]; + expect(params[INPUT_KEYS.TOKEN]).toBe('ghp_abcdefghijklmnopqrstuvwxyz12'); + expect(params[INPUT_KEYS.SINGLE_ACTION]).toBe(ACTIONS.INITIAL_SETUP); + }); + it('exits when not inside a git repo', async () => { (execSync as jest.Mock).mockImplementation((cmd: string) => { if (typeof cmd === 'string' && cmd.includes('is-inside-work-tree')) throw new Error('not a repo'); diff --git a/src/cli.ts b/src/cli.ts index b130810a..ba71b77b 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -444,9 +444,11 @@ program } logInfo(`📦 Repository: ${gitInfo.owner}/${gitInfo.repo}`); - if (!hasValidSetupToken(cwd)) { + const hasTokenFromCli = Boolean(options.token && String(options.token).trim()); + if (!hasTokenFromCli && !hasValidSetupToken(cwd)) { logError('🛑 Setup requires PERSONAL_ACCESS_TOKEN with a valid token.'); logInfo(' You can:'); + logInfo(' • Pass it on the command line: copilot setup --token '); logInfo(' • Add it to your environment: export PERSONAL_ACCESS_TOKEN=your_github_token'); if (setupEnvFileExists(cwd)) { logInfo(' • Or add PERSONAL_ACCESS_TOKEN=your_github_token to your existing .env file'); From b7c59bb2c5bd1255c950c732bef578d38b1fb394 Mon Sep 17 00:00:00 2001 From: Efra Espada Date: Sat, 14 Feb 2026 19:13:41 +0100 Subject: [PATCH 08/13] bugfix-309-setup-crash: Refactor CLI token handling to prioritize command line input, enhancing user guidance on token requirements. Update tests to ensure correct behavior for token validation and fallback mechanisms. --- build/cli/index.js | 4 ++- src/__tests__/cli.test.ts | 11 +------- src/cli.ts | 8 +++--- .../model/__tests__/project_detail.test.ts | 11 ++++++++ src/data/model/project_detail.ts | 4 +++ src/utils/__tests__/setup_files.test.ts | 26 +++++++++++++++++++ src/utils/setup_files.ts | 25 +++++++++--------- 7 files changed, 62 insertions(+), 27 deletions(-) diff --git a/build/cli/index.js b/build/cli/index.js index 9c679ce0..9f0015c8 100755 --- a/build/cli/index.js +++ b/build/cli/index.js @@ -47651,9 +47651,11 @@ program process.exit(1); } (0, logger_1.logInfo)(`📦 Repository: ${gitInfo.owner}/${gitInfo.repo}`); - if (!(0, setup_files_1.hasValidSetupToken)(cwd)) { + const hasTokenFromCli = Boolean(options.token && String(options.token).trim()); + if (!hasTokenFromCli && !(0, setup_files_1.hasValidSetupToken)(cwd)) { (0, logger_1.logError)('🛑 Setup requires PERSONAL_ACCESS_TOKEN with a valid token.'); (0, logger_1.logInfo)(' You can:'); + (0, logger_1.logInfo)(' • Pass it on the command line: copilot setup --token '); (0, logger_1.logInfo)(' • Add it to your environment: export PERSONAL_ACCESS_TOKEN=your_github_token'); if ((0, setup_files_1.setupEnvFileExists)(cwd)) { (0, logger_1.logInfo)(' • Or add PERSONAL_ACCESS_TOKEN=your_github_token to your existing .env file'); diff --git a/src/__tests__/cli.test.ts b/src/__tests__/cli.test.ts index e2f31b16..ff44e813 100644 --- a/src/__tests__/cli.test.ts +++ b/src/__tests__/cli.test.ts @@ -34,13 +34,7 @@ jest.mock('../data/repository/ai_repository', () => ({ })), })); -jest.mock('../utils/setup_files', () => { - const actual = jest.requireActual('../utils/setup_files'); - return { - ...actual, - hasValidSetupToken: jest.fn((cwd: string) => actual.hasValidSetupToken(cwd)), - }; -}); +jest.mock('../utils/setup_files', () => jest.requireActual('../utils/setup_files')); describe('CLI', () => { let exitSpy: jest.SpyInstance; @@ -291,9 +285,6 @@ describe('CLI', () => { }); it('proceeds when --token is provided even if env/.env has no token', async () => { - const { hasValidSetupToken } = require('../utils/setup_files'); - (hasValidSetupToken as jest.Mock).mockReturnValue(false); - await program.parseAsync(['node', 'cli', 'setup', '--token', 'ghp_abcdefghijklmnopqrstuvwxyz12']); expect(exitSpy).not.toHaveBeenCalled(); diff --git a/src/cli.ts b/src/cli.ts index ba71b77b..7a695745 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -6,7 +6,7 @@ import * as dotenv from 'dotenv'; import { runLocalAction } from './actions/local_action'; import { IssueRepository } from './data/repository/issue_repository'; import { ACTIONS, ERRORS, INPUT_KEYS, OPENCODE_DEFAULT_MODEL, TITLE } from './utils/constants'; -import { getSetupToken, hasValidSetupToken, setupEnvFileExists } from './utils/setup_files'; +import { getSetupToken, setupEnvFileExists } from './utils/setup_files'; import { logError, logInfo } from './utils/logger'; import { getCliDoPrompt } from './prompts'; import { Ai } from './data/model/ai'; @@ -444,8 +444,8 @@ program } logInfo(`📦 Repository: ${gitInfo.owner}/${gitInfo.repo}`); - const hasTokenFromCli = Boolean(options.token && String(options.token).trim()); - if (!hasTokenFromCli && !hasValidSetupToken(cwd)) { + const token = getSetupToken(cwd, options.token); + if (!token) { logError('🛑 Setup requires PERSONAL_ACCESS_TOKEN with a valid token.'); logInfo(' You can:'); logInfo(' • Pass it on the command line: copilot setup --token '); @@ -464,7 +464,7 @@ program [INPUT_KEYS.DEBUG]: options.debug.toString(), [INPUT_KEYS.SINGLE_ACTION]: ACTIONS.INITIAL_SETUP, [INPUT_KEYS.SINGLE_ACTION_ISSUE]: 1, - [INPUT_KEYS.TOKEN]: options.token || process.env.PERSONAL_ACCESS_TOKEN || getSetupToken(cwd), + [INPUT_KEYS.TOKEN]: token, repo: { owner: gitInfo.owner, repo: gitInfo.repo, diff --git a/src/data/model/__tests__/project_detail.test.ts b/src/data/model/__tests__/project_detail.test.ts index a9ad6475..e6057e0e 100644 --- a/src/data/model/__tests__/project_detail.test.ts +++ b/src/data/model/__tests__/project_detail.test.ts @@ -49,5 +49,16 @@ describe('ProjectDetail', () => { const p = new ProjectDetail({ type: 'user', owner: 'jane', number: 3 }); expect(p.publicUrl).toBe('https://github.com/users/jane/projects/3'); }); + + it('returns empty string when number is invalid (missing or <= 0)', () => { + const pEmpty = new ProjectDetail({ type: 'organization', owner: 'acme' }); + expect(pEmpty.publicUrl).toBe(''); + + const pZero = new ProjectDetail({ type: 'organization', owner: 'acme', number: 0 }); + expect(pZero.publicUrl).toBe(''); + + const pNegative = new ProjectDetail({ type: 'organization', owner: 'acme', number: -1 }); + expect(pNegative.publicUrl).toBe(''); + }); }); }); diff --git a/src/data/model/project_detail.ts b/src/data/model/project_detail.ts index 0abc4cfa..aa08919f 100644 --- a/src/data/model/project_detail.ts +++ b/src/data/model/project_detail.ts @@ -19,11 +19,15 @@ export class ProjectDetail { /** * Returns the full public URL to the project (board). * Uses the URL from the API when present and valid; otherwise builds it from owner, type and number. + * Returns empty string when project number is invalid (e.g. missing from API). */ get publicUrl(): string { if (this.url && typeof this.url === 'string' && this.url.startsWith('https://')) { return this.url; } + if (typeof this.number !== 'number' || this.number <= 0) { + return ''; + } const path = this.type === 'organization' ? 'orgs' : 'users'; return `https://github.com/${path}/${this.owner}/projects/${this.number}`; } diff --git a/src/utils/__tests__/setup_files.test.ts b/src/utils/__tests__/setup_files.test.ts index b7652f62..9f6b64fc 100644 --- a/src/utils/__tests__/setup_files.test.ts +++ b/src/utils/__tests__/setup_files.test.ts @@ -224,6 +224,18 @@ describe('setup_files', () => { fs.writeFileSync(path.join(tmpDir, '.env'), 'PERSONAL_ACCESS_TOKEN=ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'); expect(hasValidSetupToken(tmpDir)).toBe(true); }); + + it('returns true when valid override is passed (e.g. CLI --token)', () => { + delete process.env[ENV_TOKEN_KEY]; + expect(hasValidSetupToken(tmpDir, 'ghp_override_token_xxxxxxxxxxxxxxxxxx')).toBe(true); + }); + + it('falls back to env/.env when override is invalid (placeholder or too short)', () => { + delete process.env[ENV_TOKEN_KEY]; + fs.writeFileSync(path.join(tmpDir, '.env'), 'PERSONAL_ACCESS_TOKEN=ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'); + expect(hasValidSetupToken(tmpDir, 'github_pat_11..')).toBe(true); + expect(hasValidSetupToken(tmpDir, 'short')).toBe(true); + }); }); describe('getSetupToken', () => { @@ -244,6 +256,20 @@ describe('setup_files', () => { fs.writeFileSync(path.join(tmpDir, '.env'), 'PERSONAL_ACCESS_TOKEN=github_pat_11..'); expect(getSetupToken(tmpDir)).toBeUndefined(); }); + + it('returns valid override first (CLI token priority)', () => { + delete process.env[ENV_TOKEN_KEY]; + fs.writeFileSync(path.join(tmpDir, '.env'), 'PERSONAL_ACCESS_TOKEN=ghp_from_env_file_xxxxxxxxxxxxxxxxxx'); + expect(getSetupToken(tmpDir, 'ghp_cli_token_xxxxxxxxxxxxxxxxxxxxxxxx')).toBe('ghp_cli_token_xxxxxxxxxxxxxxxxxxxxxxxx'); + }); + + it('falls back to env then .env when override is invalid', () => { + process.env[ENV_TOKEN_KEY] = 'ghp_from_env_xxxxxxxxxxxxxxxxxxxxxxxxxx'; + fs.writeFileSync(path.join(tmpDir, '.env'), 'PERSONAL_ACCESS_TOKEN=ghp_from_file_xxxxxxxxxxxxxxxxxxxx'); + expect(getSetupToken(tmpDir, 'github_pat_11..')).toBe('ghp_from_env_xxxxxxxxxxxxxxxxxxxxxxxxxx'); + delete process.env[ENV_TOKEN_KEY]; + expect(getSetupToken(tmpDir, 'short')).toBe('ghp_from_file_xxxxxxxxxxxxxxxxxxxx'); + }); }); describe('setupEnvFileExists', () => { diff --git a/src/utils/setup_files.ts b/src/utils/setup_files.ts index 3860cb67..c0f35c3e 100644 --- a/src/utils/setup_files.ts +++ b/src/utils/setup_files.ts @@ -129,18 +129,19 @@ export function ensureEnvWithToken(cwd: string): void { function isTokenValueValid(token: string): boolean { const t = token.trim(); - return ( - t.length >= MIN_VALID_TOKEN_LENGTH && - t !== ENV_PLACEHOLDER_VALUE && - !t.startsWith('github_pat_11..') - ); + return t.length >= MIN_VALID_TOKEN_LENGTH && t !== ENV_PLACEHOLDER_VALUE; } /** - * Returns the PERSONAL_ACCESS_TOKEN to use for setup (from environment or .env in cwd). - * Same resolution order as hasValidSetupToken; returns undefined if no valid token is found. + * Resolves the PERSONAL_ACCESS_TOKEN for setup from a single priority order: + * 1. override (e.g. CLI --token) if provided and valid, + * 2. process.env.PERSONAL_ACCESS_TOKEN, + * 3. .env file in cwd. + * Returns undefined if no valid token is found. */ -export function getSetupToken(cwd: string): string | undefined { +export function getSetupToken(cwd: string, override?: string): string | undefined { + const overrideTrimmed = override?.trim(); + if (overrideTrimmed && isTokenValueValid(overrideTrimmed)) return overrideTrimmed; const fromEnv = process.env[ENV_TOKEN_KEY]?.trim(); if (fromEnv && isTokenValueValid(fromEnv)) return fromEnv; const envPath = path.join(cwd, '.env'); @@ -150,11 +151,11 @@ export function getSetupToken(cwd: string): string | undefined { } /** - * Returns true if PERSONAL_ACCESS_TOKEN is available and looks like a real token - * (from environment or .env), not the placeholder. Setup should only continue when this is true. + * Returns true if a valid setup token is available (same resolution order as getSetupToken). + * Pass an optional override (e.g. CLI --token) so validation considers all sources consistently. */ -export function hasValidSetupToken(cwd: string): boolean { - return getSetupToken(cwd) !== undefined; +export function hasValidSetupToken(cwd: string, override?: string): boolean { + return getSetupToken(cwd, override) !== undefined; } /** Returns true if a .env file exists in the given directory. */ From 3eac306eb3ab7ebd5739c274609499f6211b3d0c Mon Sep 17 00:00:00 2001 From: Efra Espada Date: Sat, 14 Feb 2026 19:35:46 +0100 Subject: [PATCH 09/13] bugfix-309-setup-crash: Refactor token handling in CLI setup to prioritize command line input and improve validation logic. Enhance error logging for project item retrieval and update tests to cover pagination scenarios and invalid project IDs. --- build/cli/index.js | 72 +++++++++---- build/cli/src/data/model/project_detail.d.ts | 1 + build/cli/src/utils/setup_files.d.ts | 15 +-- build/github_action/index.js | 66 +++++++++--- .../src/data/model/project_detail.d.ts | 1 + .../github_action/src/utils/setup_files.d.ts | 15 +-- .../__tests__/project_repository.test.ts | 100 +++++++++++++++++- src/data/repository/project_repository.ts | 55 ++++++++-- 8 files changed, 268 insertions(+), 57 deletions(-) diff --git a/build/cli/index.js b/build/cli/index.js index 9f0015c8..afaefdf5 100755 --- a/build/cli/index.js +++ b/build/cli/index.js @@ -47651,8 +47651,8 @@ program process.exit(1); } (0, logger_1.logInfo)(`📦 Repository: ${gitInfo.owner}/${gitInfo.repo}`); - const hasTokenFromCli = Boolean(options.token && String(options.token).trim()); - if (!hasTokenFromCli && !(0, setup_files_1.hasValidSetupToken)(cwd)) { + const token = (0, setup_files_1.getSetupToken)(cwd, options.token); + if (!token) { (0, logger_1.logError)('🛑 Setup requires PERSONAL_ACCESS_TOKEN with a valid token.'); (0, logger_1.logInfo)(' You can:'); (0, logger_1.logInfo)(' • Pass it on the command line: copilot setup --token '); @@ -47670,7 +47670,7 @@ program [constants_1.INPUT_KEYS.DEBUG]: options.debug.toString(), [constants_1.INPUT_KEYS.SINGLE_ACTION]: constants_1.ACTIONS.INITIAL_SETUP, [constants_1.INPUT_KEYS.SINGLE_ACTION_ISSUE]: 1, - [constants_1.INPUT_KEYS.TOKEN]: options.token || process.env.PERSONAL_ACCESS_TOKEN || (0, setup_files_1.getSetupToken)(cwd), + [constants_1.INPUT_KEYS.TOKEN]: token, repo: { owner: gitInfo.owner, repo: gitInfo.repo, @@ -48730,11 +48730,15 @@ class ProjectDetail { /** * Returns the full public URL to the project (board). * Uses the URL from the API when present and valid; otherwise builds it from owner, type and number. + * Returns empty string when project number is invalid (e.g. missing from API). */ get publicUrl() { if (this.url && typeof this.url === 'string' && this.url.startsWith('https://')) { return this.url; } + if (typeof this.number !== 'number' || this.number <= 0) { + return ''; + } const path = this.type === 'organization' ? 'orgs' : 'users'; return `https://github.com/${path}/${this.owner}/projects/${this.number}`; } @@ -51582,14 +51586,22 @@ class ProjectRepository { number: issueOrPullRequestNumber }); if (!issueOrPrResult.repository.issueOrPullRequest) { - console.error(`Issue or PR #${issueOrPullRequestNumber} not found.`); + (0, logger_1.logError)(`Issue or PR #${issueOrPullRequestNumber} not found in repository.`); return undefined; } const contentId = issueOrPrResult.repository.issueOrPullRequest.id; // Search for the item ID in the project with pagination let cursor = null; let projectItemId = undefined; + let totalItemsChecked = 0; + const maxPages = 100; // 100 * 100 = 10_000 items max to avoid runaway loops + let pageCount = 0; do { + if (pageCount >= maxPages) { + (0, logger_1.logError)(`Stopped after ${maxPages} pages (${totalItemsChecked} items). Issue or PR #${issueOrPullRequestNumber} not found in project.`); + break; + } + pageCount += 1; const projectQuery = ` query($projectId: ID!, $cursor: String) { node(id: $projectId) { @@ -51618,16 +51630,36 @@ class ProjectRepository { projectId: project.id, cursor }); - const items = projectResult.node.items.nodes; + if (projectResult.node === null) { + (0, logger_1.logError)(`Project not found for ID "${project.id}". Ensure the project is loaded via getProjectDetail (GraphQL node ID), not the project number.`); + throw new Error(`Project not found or invalid project ID. The project ID must be the GraphQL node ID from the API (e.g. PVT_...), not the project number.`); + } + const items = projectResult.node.items?.nodes ?? []; + totalItemsChecked += items.length; + const pageInfo = projectResult.node.items?.pageInfo; const foundItem = items.find((item) => item.content?.id === contentId); if (foundItem) { projectItemId = foundItem.id; break; } - cursor = projectResult.node.items.pageInfo.hasNextPage - ? projectResult.node.items.pageInfo.endCursor - : null; + // Advance cursor only when there is a next page AND a non-null cursor (avoid missing pages) + const hasNextPage = pageInfo?.hasNextPage === true; + const endCursor = pageInfo?.endCursor ?? null; + if (hasNextPage && endCursor) { + cursor = endCursor; + } + else { + if (hasNextPage && !endCursor) { + (0, logger_1.logError)(`Project items pagination: hasNextPage is true but endCursor is null (page ${pageCount}, ${totalItemsChecked} items so far). Cannot fetch more.`); + } + cursor = null; + } } while (cursor); + if (projectItemId === undefined) { + (0, logger_1.logError)(`Issue or PR #${issueOrPullRequestNumber} not found in project after checking ${totalItemsChecked} items (${pageCount} page(s)). ` + + `Link it to the project first, or wait for the board to sync.`); + throw new Error(`Issue or pull request #${issueOrPullRequestNumber} is not in the project yet (checked ${totalItemsChecked} items). Link it to the project first, or wait for the board to sync.`); + } return projectItemId; }; this.isContentLinked = async (project, contentId, token) => { @@ -60515,15 +60547,19 @@ function ensureEnvWithToken(cwd) { } function isTokenValueValid(token) { const t = token.trim(); - return (t.length >= MIN_VALID_TOKEN_LENGTH && - t !== ENV_PLACEHOLDER_VALUE && - !t.startsWith('github_pat_11..')); + return t.length >= MIN_VALID_TOKEN_LENGTH && t !== ENV_PLACEHOLDER_VALUE; } /** - * Returns the PERSONAL_ACCESS_TOKEN to use for setup (from environment or .env in cwd). - * Same resolution order as hasValidSetupToken; returns undefined if no valid token is found. + * Resolves the PERSONAL_ACCESS_TOKEN for setup from a single priority order: + * 1. override (e.g. CLI --token) if provided and valid, + * 2. process.env.PERSONAL_ACCESS_TOKEN, + * 3. .env file in cwd. + * Returns undefined if no valid token is found. */ -function getSetupToken(cwd) { +function getSetupToken(cwd, override) { + const overrideTrimmed = override?.trim(); + if (overrideTrimmed && isTokenValueValid(overrideTrimmed)) + return overrideTrimmed; const fromEnv = process.env[ENV_TOKEN_KEY]?.trim(); if (fromEnv && isTokenValueValid(fromEnv)) return fromEnv; @@ -60534,11 +60570,11 @@ function getSetupToken(cwd) { return undefined; } /** - * Returns true if PERSONAL_ACCESS_TOKEN is available and looks like a real token - * (from environment or .env), not the placeholder. Setup should only continue when this is true. + * Returns true if a valid setup token is available (same resolution order as getSetupToken). + * Pass an optional override (e.g. CLI --token) so validation considers all sources consistently. */ -function hasValidSetupToken(cwd) { - return getSetupToken(cwd) !== undefined; +function hasValidSetupToken(cwd, override) { + return getSetupToken(cwd, override) !== undefined; } /** Returns true if a .env file exists in the given directory. */ function setupEnvFileExists(cwd) { diff --git a/build/cli/src/data/model/project_detail.d.ts b/build/cli/src/data/model/project_detail.d.ts index a72054d6..cfcdc85f 100644 --- a/build/cli/src/data/model/project_detail.d.ts +++ b/build/cli/src/data/model/project_detail.d.ts @@ -9,6 +9,7 @@ export declare class ProjectDetail { /** * Returns the full public URL to the project (board). * Uses the URL from the API when present and valid; otherwise builds it from owner, type and number. + * Returns empty string when project number is invalid (e.g. missing from API). */ get publicUrl(): string; } diff --git a/build/cli/src/utils/setup_files.d.ts b/build/cli/src/utils/setup_files.d.ts index a67e0afa..dd38d247 100644 --- a/build/cli/src/utils/setup_files.d.ts +++ b/build/cli/src/utils/setup_files.d.ts @@ -21,14 +21,17 @@ export declare function copySetupFiles(cwd: string, setupDirOverride?: string): */ export declare function ensureEnvWithToken(cwd: string): void; /** - * Returns the PERSONAL_ACCESS_TOKEN to use for setup (from environment or .env in cwd). - * Same resolution order as hasValidSetupToken; returns undefined if no valid token is found. + * Resolves the PERSONAL_ACCESS_TOKEN for setup from a single priority order: + * 1. override (e.g. CLI --token) if provided and valid, + * 2. process.env.PERSONAL_ACCESS_TOKEN, + * 3. .env file in cwd. + * Returns undefined if no valid token is found. */ -export declare function getSetupToken(cwd: string): string | undefined; +export declare function getSetupToken(cwd: string, override?: string): string | undefined; /** - * Returns true if PERSONAL_ACCESS_TOKEN is available and looks like a real token - * (from environment or .env), not the placeholder. Setup should only continue when this is true. + * Returns true if a valid setup token is available (same resolution order as getSetupToken). + * Pass an optional override (e.g. CLI --token) so validation considers all sources consistently. */ -export declare function hasValidSetupToken(cwd: string): boolean; +export declare function hasValidSetupToken(cwd: string, override?: string): boolean; /** Returns true if a .env file exists in the given directory. */ export declare function setupEnvFileExists(cwd: string): boolean; diff --git a/build/github_action/index.js b/build/github_action/index.js index eded4d3d..f2d088a0 100644 --- a/build/github_action/index.js +++ b/build/github_action/index.js @@ -43817,11 +43817,15 @@ class ProjectDetail { /** * Returns the full public URL to the project (board). * Uses the URL from the API when present and valid; otherwise builds it from owner, type and number. + * Returns empty string when project number is invalid (e.g. missing from API). */ get publicUrl() { if (this.url && typeof this.url === 'string' && this.url.startsWith('https://')) { return this.url; } + if (typeof this.number !== 'number' || this.number <= 0) { + return ''; + } const path = this.type === 'organization' ? 'orgs' : 'users'; return `https://github.com/${path}/${this.owner}/projects/${this.number}`; } @@ -46651,14 +46655,22 @@ class ProjectRepository { number: issueOrPullRequestNumber }); if (!issueOrPrResult.repository.issueOrPullRequest) { - console.error(`Issue or PR #${issueOrPullRequestNumber} not found.`); + (0, logger_1.logError)(`Issue or PR #${issueOrPullRequestNumber} not found in repository.`); return undefined; } const contentId = issueOrPrResult.repository.issueOrPullRequest.id; // Search for the item ID in the project with pagination let cursor = null; let projectItemId = undefined; + let totalItemsChecked = 0; + const maxPages = 100; // 100 * 100 = 10_000 items max to avoid runaway loops + let pageCount = 0; do { + if (pageCount >= maxPages) { + (0, logger_1.logError)(`Stopped after ${maxPages} pages (${totalItemsChecked} items). Issue or PR #${issueOrPullRequestNumber} not found in project.`); + break; + } + pageCount += 1; const projectQuery = ` query($projectId: ID!, $cursor: String) { node(id: $projectId) { @@ -46687,16 +46699,36 @@ class ProjectRepository { projectId: project.id, cursor }); - const items = projectResult.node.items.nodes; + if (projectResult.node === null) { + (0, logger_1.logError)(`Project not found for ID "${project.id}". Ensure the project is loaded via getProjectDetail (GraphQL node ID), not the project number.`); + throw new Error(`Project not found or invalid project ID. The project ID must be the GraphQL node ID from the API (e.g. PVT_...), not the project number.`); + } + const items = projectResult.node.items?.nodes ?? []; + totalItemsChecked += items.length; + const pageInfo = projectResult.node.items?.pageInfo; const foundItem = items.find((item) => item.content?.id === contentId); if (foundItem) { projectItemId = foundItem.id; break; } - cursor = projectResult.node.items.pageInfo.hasNextPage - ? projectResult.node.items.pageInfo.endCursor - : null; + // Advance cursor only when there is a next page AND a non-null cursor (avoid missing pages) + const hasNextPage = pageInfo?.hasNextPage === true; + const endCursor = pageInfo?.endCursor ?? null; + if (hasNextPage && endCursor) { + cursor = endCursor; + } + else { + if (hasNextPage && !endCursor) { + (0, logger_1.logError)(`Project items pagination: hasNextPage is true but endCursor is null (page ${pageCount}, ${totalItemsChecked} items so far). Cannot fetch more.`); + } + cursor = null; + } } while (cursor); + if (projectItemId === undefined) { + (0, logger_1.logError)(`Issue or PR #${issueOrPullRequestNumber} not found in project after checking ${totalItemsChecked} items (${pageCount} page(s)). ` + + `Link it to the project first, or wait for the board to sync.`); + throw new Error(`Issue or pull request #${issueOrPullRequestNumber} is not in the project yet (checked ${totalItemsChecked} items). Link it to the project first, or wait for the board to sync.`); + } return projectItemId; }; this.isContentLinked = async (project, contentId, token) => { @@ -55982,15 +56014,19 @@ function ensureEnvWithToken(cwd) { } function isTokenValueValid(token) { const t = token.trim(); - return (t.length >= MIN_VALID_TOKEN_LENGTH && - t !== ENV_PLACEHOLDER_VALUE && - !t.startsWith('github_pat_11..')); + return t.length >= MIN_VALID_TOKEN_LENGTH && t !== ENV_PLACEHOLDER_VALUE; } /** - * Returns the PERSONAL_ACCESS_TOKEN to use for setup (from environment or .env in cwd). - * Same resolution order as hasValidSetupToken; returns undefined if no valid token is found. + * Resolves the PERSONAL_ACCESS_TOKEN for setup from a single priority order: + * 1. override (e.g. CLI --token) if provided and valid, + * 2. process.env.PERSONAL_ACCESS_TOKEN, + * 3. .env file in cwd. + * Returns undefined if no valid token is found. */ -function getSetupToken(cwd) { +function getSetupToken(cwd, override) { + const overrideTrimmed = override?.trim(); + if (overrideTrimmed && isTokenValueValid(overrideTrimmed)) + return overrideTrimmed; const fromEnv = process.env[ENV_TOKEN_KEY]?.trim(); if (fromEnv && isTokenValueValid(fromEnv)) return fromEnv; @@ -56001,11 +56037,11 @@ function getSetupToken(cwd) { return undefined; } /** - * Returns true if PERSONAL_ACCESS_TOKEN is available and looks like a real token - * (from environment or .env), not the placeholder. Setup should only continue when this is true. + * Returns true if a valid setup token is available (same resolution order as getSetupToken). + * Pass an optional override (e.g. CLI --token) so validation considers all sources consistently. */ -function hasValidSetupToken(cwd) { - return getSetupToken(cwd) !== undefined; +function hasValidSetupToken(cwd, override) { + return getSetupToken(cwd, override) !== undefined; } /** Returns true if a .env file exists in the given directory. */ function setupEnvFileExists(cwd) { diff --git a/build/github_action/src/data/model/project_detail.d.ts b/build/github_action/src/data/model/project_detail.d.ts index a72054d6..cfcdc85f 100644 --- a/build/github_action/src/data/model/project_detail.d.ts +++ b/build/github_action/src/data/model/project_detail.d.ts @@ -9,6 +9,7 @@ export declare class ProjectDetail { /** * Returns the full public URL to the project (board). * Uses the URL from the API when present and valid; otherwise builds it from owner, type and number. + * Returns empty string when project number is invalid (e.g. missing from API). */ get publicUrl(): string; } diff --git a/build/github_action/src/utils/setup_files.d.ts b/build/github_action/src/utils/setup_files.d.ts index a67e0afa..dd38d247 100644 --- a/build/github_action/src/utils/setup_files.d.ts +++ b/build/github_action/src/utils/setup_files.d.ts @@ -21,14 +21,17 @@ export declare function copySetupFiles(cwd: string, setupDirOverride?: string): */ export declare function ensureEnvWithToken(cwd: string): void; /** - * Returns the PERSONAL_ACCESS_TOKEN to use for setup (from environment or .env in cwd). - * Same resolution order as hasValidSetupToken; returns undefined if no valid token is found. + * Resolves the PERSONAL_ACCESS_TOKEN for setup from a single priority order: + * 1. override (e.g. CLI --token) if provided and valid, + * 2. process.env.PERSONAL_ACCESS_TOKEN, + * 3. .env file in cwd. + * Returns undefined if no valid token is found. */ -export declare function getSetupToken(cwd: string): string | undefined; +export declare function getSetupToken(cwd: string, override?: string): string | undefined; /** - * Returns true if PERSONAL_ACCESS_TOKEN is available and looks like a real token - * (from environment or .env), not the placeholder. Setup should only continue when this is true. + * Returns true if a valid setup token is available (same resolution order as getSetupToken). + * Pass an optional override (e.g. CLI --token) so validation considers all sources consistently. */ -export declare function hasValidSetupToken(cwd: string): boolean; +export declare function hasValidSetupToken(cwd: string, override?: string): boolean; /** Returns true if a .env file exists in the given directory. */ export declare function setupEnvFileExists(cwd: string): boolean; diff --git a/src/data/repository/__tests__/project_repository.test.ts b/src/data/repository/__tests__/project_repository.test.ts index ce2733af..d60f9add 100644 --- a/src/data/repository/__tests__/project_repository.test.ts +++ b/src/data/repository/__tests__/project_repository.test.ts @@ -678,6 +678,75 @@ describe("ProjectRepository.setTaskPriority", () => { expect(mockGraphql).toHaveBeenCalledTimes(5); }); + it("finds issue on second page of project items (pagination)", async () => { + const firstPage = { + node: { + items: { + nodes: [ + { id: "item_other_1", content: { id: "I_other1" } }, + { id: "item_other_2", content: { id: "I_other2" } }, + ], + pageInfo: { hasNextPage: true, endCursor: "cursor_page1" }, + }, + }, + }; + const secondPage = { + node: { + items: { + nodes: [{ id: "item_issue_1", content: { id: "I_issue1" } }], + pageInfo: { hasNextPage: false, endCursor: null }, + }, + }, + }; + const fieldResponseWithIssueItem = { + node: { + fields: { + nodes: [ + { + id: "f1", + name: "Priority", + options: [{ id: "opt_high", name: "High" }], + }, + ], + }, + items: { + nodes: [ + { + id: "item_issue_1", + fieldValues: { nodes: [] }, + }, + ], + pageInfo: { hasNextPage: false, endCursor: null }, + }, + }, + }; + mockGraphql + .mockResolvedValueOnce({ + repository: { issueOrPullRequest: { id: "I_issue1" } }, + }) + .mockResolvedValueOnce(firstPage) + .mockResolvedValueOnce(secondPage) + .mockResolvedValueOnce(fieldResponseWithIssueItem) + .mockResolvedValueOnce(fieldResponseWithIssueItem) + .mockResolvedValueOnce({ + updateProjectV2ItemFieldValue: { projectV2Item: { id: "item_issue_1" } }, + }); + const result = await repo.setTaskPriority( + project, + "owner", + "repo", + 1, + "High", + "token" + ); + expect(result).toBe(true); + expect(mockGraphql).toHaveBeenCalledTimes(6); + const firstItemsCall = mockGraphql.mock.calls[1]; + const secondItemsCall = mockGraphql.mock.calls[2]; + expect((firstItemsCall[1] as { cursor?: string | null }).cursor).toBeNull(); + expect((secondItemsCall[1] as { cursor?: string | null }).cursor).toBe("cursor_page1"); + }); + it("returns false when field already set to target value", async () => { const fieldQueryResponseAlreadySet = { node: { @@ -734,7 +803,7 @@ describe("ProjectRepository.setTaskPriority", () => { expect(mockGraphql).toHaveBeenCalledTimes(4); }); - it("throws when content id not found for issue", async () => { + it("throws when content id not found for issue (issue/PR not in repo)", async () => { mockGraphql.mockResolvedValueOnce({ repository: { issueOrPullRequest: null }, }); @@ -742,4 +811,33 @@ describe("ProjectRepository.setTaskPriority", () => { repo.setTaskPriority(project, "owner", "repo", 999, "High", "token") ).rejects.toThrow("Content ID not found"); }); + + it("throws when project node is null (invalid project ID)", async () => { + mockGraphql + .mockResolvedValueOnce({ + repository: { issueOrPullRequest: { id: "I_issue1" } }, + }) + .mockResolvedValueOnce({ node: null }); + await expect( + repo.setTaskPriority(project, "owner", "repo", 1, "High", "token") + ).rejects.toThrow("Project not found or invalid project ID"); + }); + + it("throws when issue/PR not in project yet", async () => { + mockGraphql + .mockResolvedValueOnce({ + repository: { issueOrPullRequest: { id: "I_issue1" } }, + }) + .mockResolvedValueOnce({ + node: { + items: { + nodes: [], // issue not in project + pageInfo: { hasNextPage: false, endCursor: null }, + }, + }, + }); + await expect( + repo.setTaskPriority(project, "owner", "repo", 1, "High", "token") + ).rejects.toThrow("not in the project yet"); + }); }); diff --git a/src/data/repository/project_repository.ts b/src/data/repository/project_repository.ts index 05ef0722..1ab2d302 100644 --- a/src/data/repository/project_repository.ts +++ b/src/data/repository/project_repository.ts @@ -111,7 +111,7 @@ export class ProjectRepository { }); if (!issueOrPrResult.repository.issueOrPullRequest) { - console.error(`Issue or PR #${issueOrPullRequestNumber} not found.`); + logError(`Issue or PR #${issueOrPullRequestNumber} not found in repository.`); return undefined; } @@ -120,8 +120,17 @@ export class ProjectRepository { // Search for the item ID in the project with pagination let cursor: string | null = null; let projectItemId: string | undefined = undefined; - + let totalItemsChecked = 0; + const maxPages = 100; // 100 * 100 = 10_000 items max to avoid runaway loops + let pageCount = 0; + do { + if (pageCount >= maxPages) { + logError(`Stopped after ${maxPages} pages (${totalItemsChecked} items). Issue or PR #${issueOrPullRequestNumber} not found in project.`); + break; + } + pageCount += 1; + const projectQuery = ` query($projectId: ID!, $cursor: String) { node(id: $projectId) { @@ -148,26 +157,50 @@ export class ProjectRepository { }`; interface ProjectItemsNode { id: string; content?: { id?: string } } - type ProjectItemsResponse = { node: { items: { nodes: ProjectItemsNode[]; pageInfo: { hasNextPage: boolean; endCursor: string | null } } } }; + type ProjectItemsResponse = { node: { items?: { nodes: ProjectItemsNode[]; pageInfo: { hasNextPage: boolean; endCursor: string | null } } } | null }; const projectResult: ProjectItemsResponse = await octokit.graphql(projectQuery, { projectId: project.id, cursor }); - const items = projectResult.node.items.nodes; + if (projectResult.node === null) { + logError(`Project not found for ID "${project.id}". Ensure the project is loaded via getProjectDetail (GraphQL node ID), not the project number.`); + throw new Error( + `Project not found or invalid project ID. The project ID must be the GraphQL node ID from the API (e.g. PVT_...), not the project number.` + ); + } + const items = projectResult.node.items?.nodes ?? []; + totalItemsChecked += items.length; + const pageInfo = projectResult.node.items?.pageInfo; const foundItem = items.find((item: ProjectItemsNode) => item.content?.id === contentId); - + if (foundItem) { projectItemId = foundItem.id; break; } - - cursor = projectResult.node.items.pageInfo.hasNextPage - ? projectResult.node.items.pageInfo.endCursor - : null; - + + // Advance cursor only when there is a next page AND a non-null cursor (avoid missing pages) + const hasNextPage = pageInfo?.hasNextPage === true; + const endCursor = pageInfo?.endCursor ?? null; + if (hasNextPage && endCursor) { + cursor = endCursor; + } else { + if (hasNextPage && !endCursor) { + logError(`Project items pagination: hasNextPage is true but endCursor is null (page ${pageCount}, ${totalItemsChecked} items so far). Cannot fetch more.`); + } + cursor = null; + } } while (cursor); - + + if (projectItemId === undefined) { + logError( + `Issue or PR #${issueOrPullRequestNumber} not found in project after checking ${totalItemsChecked} items (${pageCount} page(s)). ` + + `Link it to the project first, or wait for the board to sync.` + ); + throw new Error( + `Issue or pull request #${issueOrPullRequestNumber} is not in the project yet (checked ${totalItemsChecked} items). Link it to the project first, or wait for the board to sync.` + ); + } return projectItemId; }; From 6325e66733fceb2b4d28b40c986fcd07eb61700a Mon Sep 17 00:00:00 2001 From: Efra Espada Date: Sat, 14 Feb 2026 19:40:08 +0100 Subject: [PATCH 10/13] bugfix-309-setup-crash: Enhance CLI setup test to explicitly check for command line token input, ensuring correct token handling in parameters. Update test to reflect the expected token value for improved clarity and validation. --- src/__tests__/cli.test.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/__tests__/cli.test.ts b/src/__tests__/cli.test.ts index ff44e813..88157f74 100644 --- a/src/__tests__/cli.test.ts +++ b/src/__tests__/cli.test.ts @@ -275,12 +275,18 @@ describe('CLI', () => { // Token check: hasValidSetupToken/setupEnvFileExists and message variants are covered in // setup_files.test.ts and initial_setup_use_case.test.ts. it('calls runLocalAction with INITIAL_SETUP', async () => { - await program.parseAsync(['node', 'cli', 'setup']); + await program.parseAsync([ + 'node', + 'cli', + 'setup', + '--token', + 'ghp_setup_test_token_xxxxxxxxxxxxxxxxxxxx', + ]); expect(runLocalAction).toHaveBeenCalledTimes(1); const params = (runLocalAction as jest.Mock).mock.calls[0][0]; expect(params[INPUT_KEYS.SINGLE_ACTION]).toBe(ACTIONS.INITIAL_SETUP); - expect(params[INPUT_KEYS.TOKEN]).toBeTruthy(); + expect(params[INPUT_KEYS.TOKEN]).toBe('ghp_setup_test_token_xxxxxxxxxxxxxxxxxxxx'); expect(params[INPUT_KEYS.WELCOME_TITLE]).toContain('Initial Setup'); }); From b9082e93bfa8406b0fbaf31a70013503d967002e Mon Sep 17 00:00:00 2001 From: Efra Espada Date: Sun, 15 Feb 2026 12:19:38 +0100 Subject: [PATCH 11/13] 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. --- build/cli/index.js | 1 + src/__tests__/cli.test.ts | 44 ++++++++++++- src/cli.ts | 1 + .../__tests__/project_repository.test.ts | 66 +++++++++++++++++++ src/utils/__tests__/setup_files.test.ts | 10 +++ 5 files changed, 121 insertions(+), 1 deletion(-) diff --git a/build/cli/index.js b/build/cli/index.js index afaefdf5..734e447e 100755 --- a/build/cli/index.js +++ b/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 = { diff --git a/src/__tests__/cli.test.ts b/src/__tests__/cli.test.ts index 88157f74..f8102322 100644 --- a/src/__tests__/cli.test.ts +++ b/src/__tests__/cli.test.ts @@ -34,7 +34,16 @@ jest.mock('../data/repository/ai_repository', () => ({ })), })); -jest.mock('../utils/setup_files', () => jest.requireActual('../utils/setup_files')); +const mockGetSetupToken = jest.fn(); +const mockSetupEnvFileExists = jest.fn(); +jest.mock('../utils/setup_files', () => { + const actual = jest.requireActual('../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', () => { diff --git a/src/cli.ts b/src/cli.ts index 7a695745..e7f82049 100644 --- a/src/cli.ts +++ b/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)...'); diff --git a/src/data/repository/__tests__/project_repository.test.ts b/src/data/repository/__tests__/project_repository.test.ts index d60f9add..761c4f6a 100644 --- a/src/data/repository/__tests__/project_repository.test.ts +++ b/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") + ); }); }); diff --git a/src/utils/__tests__/setup_files.test.ts b/src/utils/__tests__/setup_files.test.ts index 9f6b64fc..e5980d41 100644 --- a/src/utils/__tests__/setup_files.test.ts +++ b/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]; From 178170157278c61caca7bda5f8db6d9c937d7176 Mon Sep 17 00:00:00 2001 From: vypbot Date: Sun, 15 Feb 2026 11:56:16 +0000 Subject: [PATCH 12/13] gh-action: updated compiled files and bumped version to 2.0.2 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 9d8b824f..07cb4c36 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "copilot", - "version": "2.0.1", + "version": "2.0.2", "description": "Automates branch management, GitHub project linking, and issue/PR tracking with Git-Flow methodology.", "main": "build/github_action/index.js", "bin": { From 5683c5a19c2d876701808586adfa2995a737b943 Mon Sep 17 00:00:00 2001 From: vypbot Date: Sun, 15 Feb 2026 11:57:12 +0000 Subject: [PATCH 13/13] release-2.0.2: updated compiled files --- package-lock.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index 50067ddb..6840efce 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "copilot", - "version": "2.0.1", + "version": "2.0.2", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "copilot", - "version": "2.0.1", + "version": "2.0.2", "hasInstallScript": true, "license": "ISC", "dependencies": {