diff --git a/skill/SKILL.md b/skill/SKILL.md index 26a68ad..d08d2fa 100644 --- a/skill/SKILL.md +++ b/skill/SKILL.md @@ -150,6 +150,8 @@ polylane automation list `integration connect` and `cloud connect` dispatch on `--type` / `--provider`. Some options open a browser for an install URL; others take API credentials directly. Use `--help` on each to see the required flags and optional `--no-browser`. Browser flows wait in the terminal until the connection appears (interactive TTY only); with `--output json` or in non-interactive runs they print the URL and exit, so poll `integration list` / `cloud list` to confirm. +`integration connect --type github` asks one question before opening GitHub: review pull requests for production impact on the repositories this connection brings in (default yes). Pass `--no-pr-reviews` to opt out, or `--pr-reviews` to answer yes, without the prompt; non-interactive runs without either flag keep the default. Each repository can be changed later in the console. + ### Investigating an issue ```bash diff --git a/src/commands/integration/connect.ts b/src/commands/integration/connect.ts index feec29f..820df37 100644 --- a/src/commands/integration/connect.ts +++ b/src/commands/integration/connect.ts @@ -33,6 +33,7 @@ import { promptSelectOrBack, promptPasswordOrBack, promptTextOrBack, + promptYesNoOrBack, } from '../../utils/prompt'; import { printSlackChannelsLater, runSlackChannelStep } from './slack-channels'; @@ -103,6 +104,49 @@ export function shouldOfferCodeAgent( return category === 'code-agent' && !typeFromFlag && interactive; } +// The one question the GitHub connect asks: review pull requests for production impact on +// the repositories this connection brings in. A flag answers it without the prompt; an +// interactive run asks (default yes); a non-interactive run without a flag sends nothing, so +// the server keeps its default and records no answer. +export type PrReviewsChoice = 'on' | 'off'; + +export function prReviewsChoiceFromFlags( + args: Record, + interactive: boolean +): PrReviewsChoice | 'ask' | undefined { + if (getArgBoolean(args, 'noPrReviews') === true) return 'off'; + if (getArgBoolean(args, 'prReviews') === true) return 'on'; + return interactive ? 'ask' : undefined; +} + +// The flags only mean something for GitHub; on any other type they are ignored with a warning +// rather than an error, because the type may have been picked interactively after the flag. +export function prReviewsFlagsGiven(args: Record): boolean { + return getArgBoolean(args, 'noPrReviews') === true || getArgBoolean(args, 'prReviews') === true; +} + +// The console's /cli/connect page carries the answer across the GitHub install round-trip +// and hands it to the API, which records it on the integration before the repository sync. +export function githubConnectUrl(config: Config, workspaceId: string, prReviews: PrReviewsChoice | undefined): string { + const url = cliConnectUrl(config, 'github', workspaceId); + return prReviews ? `${url}&pr_reviews=${prReviews}` : url; +} + +// Asked in the installer's own `[Y/n]` shape, not a clack confirm: inside install.sh this line sits +// between the script's "Connect GitHub? [Y/n]" and "Connect Slack? [Y/n]" and must read like them. +async function askPrReviews(config: Config): Promise { + process.stdout.write( + 'Polylane comments a pass or fail production-impact verdict on every pull request in the repositories you connect; change it per repository any time in the console.\n' + ); + const keep = await promptYesNoOrBack( + { nonInteractive: config.nonInteractive }, + 'Review pull requests for production impact?', + true + ); + if (keep === BACK) return BACK; + return keep ? 'on' : 'off'; +} + // The category is validated even when --type wins, so a typo always errors // instead of being silently ignored. export function resolveTypeOptions(category: string | undefined, typeFromFlag: boolean): typeof TYPE_OPTIONS { @@ -1041,7 +1085,20 @@ async function connectType( return 'connected'; } const check = canWaitForBrowser(config) && baseline ? baseline.check : null; - await openOrPrintInstallUrl(config, cliConnectUrl(config, type, workspaceId), labels[type], noBrowser); + let url = cliConnectUrl(config, type, workspaceId); + if (type !== 'github' && prReviewsFlagsGiven(args) && config.output !== 'json') { + process.stderr.write('--pr-reviews / --no-pr-reviews only apply to --type github; ignored.\n'); + } + if (type === 'github') { + let prReviews = prReviewsChoiceFromFlags(args, canWaitForBrowser(config)); + if (prReviews === 'ask') { + const answer = await askPrReviews(config); + if (answer === BACK) return BACK; + prReviews = answer; + } + url = githubConnectUrl(config, workspaceId, prReviews); + } + await openOrPrintInstallUrl(config, url, labels[type], noBrowser); const outcome = await confirmBrowserConnect(config, check, names[type]); // The channel step needs the new integration's id; the browser wait already spotted the row, // so one more check() returns it without another wait. @@ -1099,10 +1156,21 @@ export const integrationConnectCommand: Command = { { flag: '--scope ', description: 'MCP OAuth scope', type: 'string' }, { flag: '--no-browser', description: 'GitHub / Slack / Sentry / MCP OAuth: print the URL instead of opening it', type: 'boolean' }, { flag: '--reconnect', description: 'GitHub / Slack / Sentry: run the connect flow even when the integration is already connected', type: 'boolean' }, + { + flag: '--pr-reviews', + description: 'GitHub: review pull requests for production impact on the repositories this connection brings in (the default), without the prompt', + type: 'boolean', + }, + { + flag: '--no-pr-reviews', + description: 'GitHub: do not review pull requests on the repositories this connection brings in; each repository can be changed later in the console', + type: 'boolean', + }, ], examples: [ 'polylane integration connect', 'polylane integration connect --type github', + 'polylane integration connect --type github --no-pr-reviews', 'polylane integration connect --type github --reconnect', 'polylane integration connect --category observability', 'polylane integration connect --category code-agent', diff --git a/src/utils/prompt.ts b/src/utils/prompt.ts index f27289a..067e259 100644 --- a/src/utils/prompt.ts +++ b/src/utils/prompt.ts @@ -1,4 +1,5 @@ import * as p from '@clack/prompts'; +import { createInterface } from 'node:readline'; import { CLIError } from '../errors/base'; import { ExitCode } from '../errors/codes'; import { isInteractive } from './env'; @@ -121,6 +122,34 @@ export async function promptConfirmOrBack( return p.isCancel(result) ? BACK : result; } +// The installer's own question shape: ` [Y/n] `, Enter keeps the default, and only an +// answer starting with n (or y, for a default-no question) flips it. Questions the CLI asks while +// running inside install.sh use this so they read like the script's neighbouring lines. +export function parseYesNo(answer: string, defaultYes: boolean): boolean { + const first = answer.trim().charAt(0).toLowerCase(); + if (first === 'n') return false; + if (first === 'y') return true; + return defaultYes; +} + +export async function promptYesNoOrBack( + ctx: PromptContext, + message: string, + defaultYes = true +): Promise { + ensureInteractive(ctx, message); + const rl = createInterface({ input: process.stdin, output: process.stdout }); + try { + const answer = await new Promise((resolve) => { + rl.once('close', () => resolve(null)); + rl.question(`${message} ${defaultYes ? '[Y/n]' : '[y/N]'} `, resolve); + }); + return answer === null ? BACK : parseYesNo(answer, defaultYes); + } finally { + rl.close(); + } +} + export function intro(message: string): void { p.intro(message); } diff --git a/test/integration-connect-pr-reviews.test.ts b/test/integration-connect-pr-reviews.test.ts new file mode 100644 index 0000000..1fa757c --- /dev/null +++ b/test/integration-connect-pr-reviews.test.ts @@ -0,0 +1,59 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { githubConnectUrl, prReviewsChoiceFromFlags, prReviewsFlagsGiven } from '../src/commands/integration/connect'; +import { mockConfig } from './helpers/config'; + +describe('prReviewsChoiceFromFlags', () => { + it('--no-pr-reviews opts out without asking', () => { + assert.equal(prReviewsChoiceFromFlags({ noPrReviews: true }, true), 'off'); + assert.equal(prReviewsChoiceFromFlags({ noPrReviews: true }, false), 'off'); + }); + + it('--pr-reviews records an explicit keep-on without asking', () => { + assert.equal(prReviewsChoiceFromFlags({ prReviews: true }, true), 'on'); + assert.equal(prReviewsChoiceFromFlags({ prReviews: true }, false), 'on'); + }); + + it('asks when interactive and no flag decided', () => { + assert.equal(prReviewsChoiceFromFlags({}, true), 'ask'); + }); + + it('sends no choice non-interactively so the server keeps the default and records nothing', () => { + assert.equal(prReviewsChoiceFromFlags({}, false), undefined); + }); + + it('opt-out wins when both flags are passed', () => { + assert.equal(prReviewsChoiceFromFlags({ prReviews: true, noPrReviews: true }, true), 'off'); + }); +}); + +describe('prReviewsFlagsGiven', () => { + it('is true for either flag, so a non-GitHub type can warn that it is ignored', () => { + assert.equal(prReviewsFlagsGiven({ noPrReviews: true }), true); + assert.equal(prReviewsFlagsGiven({ prReviews: true }), true); + assert.equal(prReviewsFlagsGiven({}), false); + assert.equal(prReviewsFlagsGiven({ noBrowser: true }), false); + }); +}); + +describe('githubConnectUrl', () => { + const config = mockConfig(); + + it('forwards an explicit choice to the console connect page', () => { + const url = new URL(githubConnectUrl(config, 'ws_1', 'off')); + assert.equal(url.pathname, '/cli/connect'); + assert.equal(url.searchParams.get('flow'), 'github'); + assert.equal(url.searchParams.get('workspace'), 'ws_1'); + assert.equal(url.searchParams.get('pr_reviews'), 'off'); + }); + + it('forwards a kept-on answer too, so the console records that the question was asked', () => { + const url = new URL(githubConnectUrl(config, 'ws_1', 'on')); + assert.equal(url.searchParams.get('pr_reviews'), 'on'); + }); + + it('omits the parameter entirely when no choice was made', () => { + const url = new URL(githubConnectUrl(config, 'ws_1', undefined)); + assert.equal(url.searchParams.has('pr_reviews'), false); + }); +}); diff --git a/test/prompt-yes-no.test.ts b/test/prompt-yes-no.test.ts new file mode 100644 index 0000000..db9ca85 --- /dev/null +++ b/test/prompt-yes-no.test.ts @@ -0,0 +1,23 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { parseYesNo } from '../src/utils/prompt'; + +describe('parseYesNo (the installer [Y/n] shape)', () => { + it('Enter keeps the default', () => { + assert.equal(parseYesNo('', true), true); + assert.equal(parseYesNo(' ', true), true); + assert.equal(parseYesNo('', false), false); + }); + + it('anything starting with n says no, anything starting with y says yes, case-insensitively', () => { + assert.equal(parseYesNo('n', true), false); + assert.equal(parseYesNo('No thanks', true), false); + assert.equal(parseYesNo('y', false), true); + assert.equal(parseYesNo('YES', false), true); + }); + + it('an unrelated answer keeps the default, like install.sh', () => { + assert.equal(parseYesNo('maybe', true), true); + assert.equal(parseYesNo('maybe', false), false); + }); +});