Skip to content

Commit b7c59bb

Browse files
committed
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.
1 parent 556172c commit b7c59bb

7 files changed

Lines changed: 62 additions & 27 deletions

File tree

build/cli/index.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47651,9 +47651,11 @@ program
4765147651
process.exit(1);
4765247652
}
4765347653
(0, logger_1.logInfo)(`📦 Repository: ${gitInfo.owner}/${gitInfo.repo}`);
47654-
if (!(0, setup_files_1.hasValidSetupToken)(cwd)) {
47654+
const hasTokenFromCli = Boolean(options.token && String(options.token).trim());
47655+
if (!hasTokenFromCli && !(0, setup_files_1.hasValidSetupToken)(cwd)) {
4765547656
(0, logger_1.logError)('🛑 Setup requires PERSONAL_ACCESS_TOKEN with a valid token.');
4765647657
(0, logger_1.logInfo)(' You can:');
47658+
(0, logger_1.logInfo)(' • Pass it on the command line: copilot setup --token <your_github_token>');
4765747659
(0, logger_1.logInfo)(' • Add it to your environment: export PERSONAL_ACCESS_TOKEN=your_github_token');
4765847660
if ((0, setup_files_1.setupEnvFileExists)(cwd)) {
4765947661
(0, logger_1.logInfo)(' • Or add PERSONAL_ACCESS_TOKEN=your_github_token to your existing .env file');

src/__tests__/cli.test.ts

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,7 @@ jest.mock('../data/repository/ai_repository', () => ({
3434
})),
3535
}));
3636

37-
jest.mock('../utils/setup_files', () => {
38-
const actual = jest.requireActual<typeof import('../utils/setup_files')>('../utils/setup_files');
39-
return {
40-
...actual,
41-
hasValidSetupToken: jest.fn((cwd: string) => actual.hasValidSetupToken(cwd)),
42-
};
43-
});
37+
jest.mock('../utils/setup_files', () => jest.requireActual<typeof import('../utils/setup_files')>('../utils/setup_files'));
4438

4539
describe('CLI', () => {
4640
let exitSpy: jest.SpyInstance;
@@ -291,9 +285,6 @@ describe('CLI', () => {
291285
});
292286

293287
it('proceeds when --token is provided even if env/.env has no token', async () => {
294-
const { hasValidSetupToken } = require('../utils/setup_files');
295-
(hasValidSetupToken as jest.Mock).mockReturnValue(false);
296-
297288
await program.parseAsync(['node', 'cli', 'setup', '--token', 'ghp_abcdefghijklmnopqrstuvwxyz12']);
298289

299290
expect(exitSpy).not.toHaveBeenCalled();

src/cli.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import * as dotenv from 'dotenv';
66
import { runLocalAction } from './actions/local_action';
77
import { IssueRepository } from './data/repository/issue_repository';
88
import { ACTIONS, ERRORS, INPUT_KEYS, OPENCODE_DEFAULT_MODEL, TITLE } from './utils/constants';
9-
import { getSetupToken, hasValidSetupToken, setupEnvFileExists } from './utils/setup_files';
9+
import { getSetupToken, setupEnvFileExists } from './utils/setup_files';
1010
import { logError, logInfo } from './utils/logger';
1111
import { getCliDoPrompt } from './prompts';
1212
import { Ai } from './data/model/ai';
@@ -444,8 +444,8 @@ program
444444
}
445445
logInfo(`📦 Repository: ${gitInfo.owner}/${gitInfo.repo}`);
446446

447-
const hasTokenFromCli = Boolean(options.token && String(options.token).trim());
448-
if (!hasTokenFromCli && !hasValidSetupToken(cwd)) {
447+
const token = getSetupToken(cwd, options.token);
448+
if (!token) {
449449
logError('🛑 Setup requires PERSONAL_ACCESS_TOKEN with a valid token.');
450450
logInfo(' You can:');
451451
logInfo(' • Pass it on the command line: copilot setup --token <your_github_token>');
@@ -464,7 +464,7 @@ program
464464
[INPUT_KEYS.DEBUG]: options.debug.toString(),
465465
[INPUT_KEYS.SINGLE_ACTION]: ACTIONS.INITIAL_SETUP,
466466
[INPUT_KEYS.SINGLE_ACTION_ISSUE]: 1,
467-
[INPUT_KEYS.TOKEN]: options.token || process.env.PERSONAL_ACCESS_TOKEN || getSetupToken(cwd),
467+
[INPUT_KEYS.TOKEN]: token,
468468
repo: {
469469
owner: gitInfo.owner,
470470
repo: gitInfo.repo,

src/data/model/__tests__/project_detail.test.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,5 +49,16 @@ describe('ProjectDetail', () => {
4949
const p = new ProjectDetail({ type: 'user', owner: 'jane', number: 3 });
5050
expect(p.publicUrl).toBe('https://github.com/users/jane/projects/3');
5151
});
52+
53+
it('returns empty string when number is invalid (missing or <= 0)', () => {
54+
const pEmpty = new ProjectDetail({ type: 'organization', owner: 'acme' });
55+
expect(pEmpty.publicUrl).toBe('');
56+
57+
const pZero = new ProjectDetail({ type: 'organization', owner: 'acme', number: 0 });
58+
expect(pZero.publicUrl).toBe('');
59+
60+
const pNegative = new ProjectDetail({ type: 'organization', owner: 'acme', number: -1 });
61+
expect(pNegative.publicUrl).toBe('');
62+
});
5263
});
5364
});

src/data/model/project_detail.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,15 @@ export class ProjectDetail {
1919
/**
2020
* Returns the full public URL to the project (board).
2121
* Uses the URL from the API when present and valid; otherwise builds it from owner, type and number.
22+
* Returns empty string when project number is invalid (e.g. missing from API).
2223
*/
2324
get publicUrl(): string {
2425
if (this.url && typeof this.url === 'string' && this.url.startsWith('https://')) {
2526
return this.url;
2627
}
28+
if (typeof this.number !== 'number' || this.number <= 0) {
29+
return '';
30+
}
2731
const path = this.type === 'organization' ? 'orgs' : 'users';
2832
return `https://github.com/${path}/${this.owner}/projects/${this.number}`;
2933
}

src/utils/__tests__/setup_files.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,18 @@ describe('setup_files', () => {
224224
fs.writeFileSync(path.join(tmpDir, '.env'), 'PERSONAL_ACCESS_TOKEN=ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx');
225225
expect(hasValidSetupToken(tmpDir)).toBe(true);
226226
});
227+
228+
it('returns true when valid override is passed (e.g. CLI --token)', () => {
229+
delete process.env[ENV_TOKEN_KEY];
230+
expect(hasValidSetupToken(tmpDir, 'ghp_override_token_xxxxxxxxxxxxxxxxxx')).toBe(true);
231+
});
232+
233+
it('falls back to env/.env when override is invalid (placeholder or too short)', () => {
234+
delete process.env[ENV_TOKEN_KEY];
235+
fs.writeFileSync(path.join(tmpDir, '.env'), 'PERSONAL_ACCESS_TOKEN=ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx');
236+
expect(hasValidSetupToken(tmpDir, 'github_pat_11..')).toBe(true);
237+
expect(hasValidSetupToken(tmpDir, 'short')).toBe(true);
238+
});
227239
});
228240

229241
describe('getSetupToken', () => {
@@ -244,6 +256,20 @@ describe('setup_files', () => {
244256
fs.writeFileSync(path.join(tmpDir, '.env'), 'PERSONAL_ACCESS_TOKEN=github_pat_11..');
245257
expect(getSetupToken(tmpDir)).toBeUndefined();
246258
});
259+
260+
it('returns valid override first (CLI token priority)', () => {
261+
delete process.env[ENV_TOKEN_KEY];
262+
fs.writeFileSync(path.join(tmpDir, '.env'), 'PERSONAL_ACCESS_TOKEN=ghp_from_env_file_xxxxxxxxxxxxxxxxxx');
263+
expect(getSetupToken(tmpDir, 'ghp_cli_token_xxxxxxxxxxxxxxxxxxxxxxxx')).toBe('ghp_cli_token_xxxxxxxxxxxxxxxxxxxxxxxx');
264+
});
265+
266+
it('falls back to env then .env when override is invalid', () => {
267+
process.env[ENV_TOKEN_KEY] = 'ghp_from_env_xxxxxxxxxxxxxxxxxxxxxxxxxx';
268+
fs.writeFileSync(path.join(tmpDir, '.env'), 'PERSONAL_ACCESS_TOKEN=ghp_from_file_xxxxxxxxxxxxxxxxxxxx');
269+
expect(getSetupToken(tmpDir, 'github_pat_11..')).toBe('ghp_from_env_xxxxxxxxxxxxxxxxxxxxxxxxxx');
270+
delete process.env[ENV_TOKEN_KEY];
271+
expect(getSetupToken(tmpDir, 'short')).toBe('ghp_from_file_xxxxxxxxxxxxxxxxxxxx');
272+
});
247273
});
248274

249275
describe('setupEnvFileExists', () => {

src/utils/setup_files.ts

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -129,18 +129,19 @@ export function ensureEnvWithToken(cwd: string): void {
129129

130130
function isTokenValueValid(token: string): boolean {
131131
const t = token.trim();
132-
return (
133-
t.length >= MIN_VALID_TOKEN_LENGTH &&
134-
t !== ENV_PLACEHOLDER_VALUE &&
135-
!t.startsWith('github_pat_11..')
136-
);
132+
return t.length >= MIN_VALID_TOKEN_LENGTH && t !== ENV_PLACEHOLDER_VALUE;
137133
}
138134

139135
/**
140-
* Returns the PERSONAL_ACCESS_TOKEN to use for setup (from environment or .env in cwd).
141-
* Same resolution order as hasValidSetupToken; returns undefined if no valid token is found.
136+
* Resolves the PERSONAL_ACCESS_TOKEN for setup from a single priority order:
137+
* 1. override (e.g. CLI --token) if provided and valid,
138+
* 2. process.env.PERSONAL_ACCESS_TOKEN,
139+
* 3. .env file in cwd.
140+
* Returns undefined if no valid token is found.
142141
*/
143-
export function getSetupToken(cwd: string): string | undefined {
142+
export function getSetupToken(cwd: string, override?: string): string | undefined {
143+
const overrideTrimmed = override?.trim();
144+
if (overrideTrimmed && isTokenValueValid(overrideTrimmed)) return overrideTrimmed;
144145
const fromEnv = process.env[ENV_TOKEN_KEY]?.trim();
145146
if (fromEnv && isTokenValueValid(fromEnv)) return fromEnv;
146147
const envPath = path.join(cwd, '.env');
@@ -150,11 +151,11 @@ export function getSetupToken(cwd: string): string | undefined {
150151
}
151152

152153
/**
153-
* Returns true if PERSONAL_ACCESS_TOKEN is available and looks like a real token
154-
* (from environment or .env), not the placeholder. Setup should only continue when this is true.
154+
* Returns true if a valid setup token is available (same resolution order as getSetupToken).
155+
* Pass an optional override (e.g. CLI --token) so validation considers all sources consistently.
155156
*/
156-
export function hasValidSetupToken(cwd: string): boolean {
157-
return getSetupToken(cwd) !== undefined;
157+
export function hasValidSetupToken(cwd: string, override?: string): boolean {
158+
return getSetupToken(cwd, override) !== undefined;
158159
}
159160

160161
/** Returns true if a .env file exists in the given directory. */

0 commit comments

Comments
 (0)