fix(slack): decide the setup wizard's prompts by TTY, not by a flag

bin/commands/slack.ts

@@ -157,7 +157,14 @@ async function runSetup(): Promise<void> {
     const nonInteractive = !!values['non-interactive'];
     const skipValidate = !!values['skip-validate'];
 
-    const rl = nonInteractive ? null : createInterface({ input: process.stdin, output: process.stdout });
+    // Prompting needs a terminal, not just the absence of --non-interactive.
+    // Under a pipe or </dev/null, readline's question() never calls back — stdin
+    // ends without a line — so the wizard stalls on a promise that can never
+    // settle. That is not catchable: node reports "unsettled top-level await"
+    // and exits 13 with the validated tokens still unwritten (#475). The fix is
+    // to never create the interface when there is no TTY to answer it.
+    const interactive = !nonInteractive && !!process.stdin.isTTY;
+    const rl = interactive ? createInterface({ input: process.stdin, output: process.stdout }) : null;
     const ask = (question: string, defaultVal = ''): Promise<string> => {
         if (!rl) return Promise.resolve(defaultVal);
         return new Promise(r => rl.question(`  ${question} `, (ans) => r(ans.trim() || defaultVal)));
@@ -192,7 +199,10 @@ async function runSetup(): Promise<void> {
         // Best-effort conveniences: copy the manifest to the macOS clipboard
         // and open the app creation page. Failures are silent — the manifest
         // and URL are printed above regardless.
-        if (!nonInteractive) {
+        // Gated on `interactive`, not just the flag: the Enter wait below is a
+        // prompt like any other, and opening a browser is a courtesy aimed at a
+        // person sitting at a terminal (#475).
+        if (interactive) {
             if (process.platform === 'darwin') {
                 const pbcopy = execFile('pbcopy', [], () => { });
                 pbcopy.stdin?.end(slackManifestYaml());

structure/str_func.md

@@ -456,7 +456,7 @@ cli-jaw/
 │       ├── lock.ts           ← instance lock/unlock for process protection (96L)
 │       ├── history.ts        ← 채팅 히스토리 검색 CLI (65L)
 │       ├── init.ts           ← 초기화 마법사 + --safe/--dry-run + --help (516L)
-│       ├── slack.ts          ← `jaw slack manifest|setup` — 앱 매니페스트 출력 + 가이드 설정 (토큰 prefix 가드 + auth.test/apps.connections.open 라이브 검증 + settings 병합, channel 미변경) (394L)
+│       ├── slack.ts          ← `jaw slack manifest|setup` — 앱 매니페스트 출력 + 가이드 설정 (토큰 prefix 가드 + auth.test/apps.connections.open 라이브 검증 + settings 병합, channel 미변경, TTY 없으면 프롬프트 생략) (404L)
 │       ├── doctor.ts         ← 진단 (다중 체크 + claude-i helper/underlying claude + headless 감지, --json) (1131L)
 │       ├── jwc.ts            ← optional external-only JWC runtime install/clean/doctor helper (234L)
 │       ├── status.ts         ← 서버 상태 (--json) (115L)

tests/fixtures/force-tty-stdin.mts

@@ -0,0 +1,13 @@
+// Preload that makes stdin claim to be a terminal.
+//
+// `jaw slack setup` picks its interactive branch from process.stdin.isTTY
+// (#475), and a spawned test child has no terminal. Forcing the flag lets the
+// suite pin the prompting path without a PTY — which would mean `script`,
+// whose arguments differ between BSD and GNU and would make the test
+// platform-specific.
+//
+// One caveat drove the shape of the test that uses this: in terminal mode
+// readline drains the whole pipe on its first read, so only the FIRST
+// question can be answered this way. The caller therefore passes every other
+// value as a flag, leaving exactly one prompt.
+process.stdin.isTTY = true;

tests/unit/slack-setup.test.ts

@@ -1,9 +1,10 @@
 import test from 'node:test';
 import assert from 'node:assert/strict';
-import { spawnSync } from 'node:child_process';
-import { mkdtempSync, readFileSync, writeFileSync, existsSync, rmSync } from 'node:fs';
+import { spawn, spawnSync } from 'node:child_process';
+import { mkdtempSync, readFileSync, writeFileSync, existsSync, rmSync, chmodSync } from 'node:fs';
 import { homedir } from 'node:os';
 import { join } from 'node:path';
+import { pathToFileURL } from 'node:url';
 import { parse } from 'yaml';
 
 // BEHAVIOR tests for `jaw slack manifest|setup`, following the
@@ -52,6 +53,21 @@ function readSettings(home: string): Record<string, any> {
     return JSON.parse(readFileSync(p, 'utf8'));
 }
 
+/**
+ * Shadow the macOS conveniences `jaw slack setup` shells out to, recording
+ * each call. Used to prove a headless run launches neither (#475) — and that a
+ * terminal run still does — without a real browser window opening on whoever
+ * runs the suite. Neither stub reads stdin: execFile leaves that pipe open, so
+ * a stub blocking on it would outlive the wizard and hang the test.
+ */
+function writeLauncherStubs(dir: string, logPath: string): void {
+    for (const name of ['open', 'pbcopy']) {
+        const p = join(dir, name);
+        writeFileSync(p, `#!/bin/sh\necho "${name}" >> "${logPath}"\nexit 0\n`);
+        chmodSync(p, 0o755);
+    }
+}
+
 test('slack manifest emits the parseable manifest with socket mode on', (t) => {
     const { status, output, home } = runSlack(['manifest']);
     t.after(() => rmSync(home, { recursive: true, force: true }));
@@ -214,3 +230,141 @@ test('manifest --url embeds the manifest Slack actually validates (#396)', (t) =
     assert.equal(manifest.settings.socket_mode_enabled, true);
 });
 
+// ---------------------------------------------------------------------------
+// #475 — setup must not prompt when there is no terminal to answer.
+//
+// Every case above already runs with stdin ignored, but each one also passes
+// --non-interactive, so they only ever exercised the flag. Without it the
+// wizard used to build a readline interface anyway and await an answer that a
+// closed or piped stdin can never produce. The promise never settles, so it is
+// not throwable and not catchable: node prints "unsettled top-level await" and
+// exits 13, leaving the validated tokens unwritten.
+// ---------------------------------------------------------------------------
+
+test('setup completes over a non-TTY stdin without --non-interactive (#475)', (t) => {
+    // The reproduction from the issue: flags supply every value, stdin is
+    // /dev/null, and no --non-interactive. This used to exit 13 at the Step 1
+    // "Press Enter" wait.
+    const { status, output, home } = runSlack([
+        'setup', '--skip-validate', '--no-notify',
+        '--bot-token', 'xoxb-1-testbot', '--app-token', SAMPLE_APP_TOKEN, '--team-id', 'T123',
+    ]);
+    t.after(() => rmSync(home, { recursive: true, force: true }));
+
+    assert.equal(status, 0, output);
+    assert.doesNotMatch(output, /unsettled top-level await/i);
+    // The point of the bug: the run "finished" without persisting anything.
+    const s = readSettings(home);
+    assert.equal(s.slack.enabled, true);
+    assert.equal(s.slack.botToken, 'xoxb-1-testbot');
+    assert.equal(s.slack.appToken, SAMPLE_APP_TOKEN);
+    assert.equal(s.slack.teamId, 'T123');
+});
+
+test('setup skips the Press-Enter wait and browser launch without a TTY (#475)', (t) => {
+    // The old guard read the flag only, so a plain piped run still shelled out
+    // to `open` and threw a browser window at whoever ran it — on a host with
+    // no terminal attached, that is pure noise nobody asked for. PATH is
+    // shadowed with recording stubs so the assertion is about the syscall, not
+    // about a log line that might merely have been silenced.
+    const stubs = mkdtempSync(join(homedir(), '.cljaw-stub-'));
+    t.after(() => rmSync(stubs, { recursive: true, force: true }));
+    const stubLog = join(stubs, 'calls.log');
+    writeLauncherStubs(stubs, stubLog);
+    const { status, output, home } = runSlack(
+        ['setup', '--skip-validate', '--no-notify', '--bot-token', 'xoxb-1-testbot'],
+        undefined,
+        { PATH: `${stubs}:${process.env.PATH ?? ''}` },
+    );
+    t.after(() => rmSync(home, { recursive: true, force: true }));
+
+    assert.equal(status, 0, output);
+    // The manifest and URL still print — they are the actionable output. Only
+    // the prompt and the macOS conveniences are suppressed.
+    assert.match(output, /api\.slack\.com\/apps/);
+    assert.doesNotMatch(output, /Press Enter once the app is created/);
+    assert.doesNotMatch(output, /copied to clipboard/i);
+    assert.equal(existsSync(stubLog), false, 'a headless run must not launch a browser or touch the clipboard');
+});
+
+test('a non-TTY run still refuses to write without a bot token (#475)', (t) => {
+    // Not prompting must not mean accepting less. The crash is gone; the
+    // requirement is not relaxed, and the message names the flag to use.
+    const { status, output, home } = runSlack(['setup', '--skip-validate', '--no-notify']);
+    t.after(() => rmSync(home, { recursive: true, force: true }));
+
+    assert.equal(status, 1);
+    assert.match(output, /A bot token is required/);
+    assert.match(output, /--bot-token/);
+    const persisted = JSON.parse(readFileSync(join(home, 'settings.json'), 'utf8')) as Record<string, any>;
+    assert.notEqual(persisted.slack?.enabled, true);
+});
+
+test('setup still prompts when stdin is a TTY (#475)', async (t) => {
+    // The other half of the guard: a terminal must still get the wizard.
+    //
+    // isTTY is forced by a preload rather than opened with `script`, whose
+    // flags differ between BSD and GNU. Two consequences shape this setup.
+    // readline in terminal mode drains the pipe on its first read, so only
+    // the FIRST prompt can be answered — every other value is passed as a
+    // flag, including an explicit empty --app-token, which leaves exactly one
+    // question. And the answer must be written only AFTER that question is on
+    // screen, since anything sent earlier is swallowed by the manifest
+    // printing above it.
+    //
+    // PATH is shadowed so the macOS conveniences hit recording stubs: a test
+    // must never actually open a browser. The `open` stub must not read stdin
+    // either — execFile leaves that pipe open, and a stub that blocks on it
+    // keeps the child alive after the wizard is done.
+    const home = mkdtempSync(join(homedir(), '.cljaw-test-'));
+    t.after(() => rmSync(home, { recursive: true, force: true }));
+    const stubs = mkdtempSync(join(homedir(), '.cljaw-stub-'));
+    t.after(() => rmSync(stubs, { recursive: true, force: true }));
+    const stubLog = join(stubs, 'calls.log');
+    writeLauncherStubs(stubs, stubLog);
+
+    const env = { ...process.env };
+    for (const key of ['SLACK_BOT_TOKEN', 'SLACK_APP_TOKEN', 'SLACK_TEAM_ID', 'SLACK_CHANNEL_IDS']) delete env[key];
+    env.CLI_JAW_HOME = home;
+    env.PATH = `${stubs}:${process.env.PATH ?? ''}`;
+
+    const forceTty = pathToFileURL(join(repoRoot, 'tests', 'fixtures', 'force-tty-stdin.mts')).href;
+    const child = spawn(
+        process.execPath,
+        ['--import', 'tsx', '--import', forceTty, cliEntry,
+            'slack', 'setup', '--skip-validate', '--no-notify',
+            '--bot-token', 'xoxb-1-ttybot', '--app-token', '', '--channel-ids', 'C9'],
+        { env, stdio: ['pipe', 'pipe', 'pipe'] },
+    );
+    let output = '';
+    let answered = false;
+    child.stdout.setEncoding('utf8');
+    child.stderr.setEncoding('utf8');
+    child.stderr.on('data', (chunk: string) => { output += chunk; });
+    child.stdout.on('data', (chunk: string) => {
+        output += chunk;
+        if (!answered && /Press Enter once the app is created/.test(output)) {
+            answered = true;
+            child.stdin.end('\n');
+        }
+    });
+    const status = await new Promise<number>((resolve, reject) => {
+        const timer = setTimeout(() => {
+            child.kill('SIGKILL');
+            reject(new Error(`setup never exited on a TTY; output so far:\n${output}`));
+        }, 45_000);
+        child.on('error', reject);
+        child.on('close', (code) => { clearTimeout(timer); resolve(code ?? 1); });
+    });
+
+    assert.equal(status, 0, output);
+    // The prompt the non-TTY path skips is present here.
+    assert.match(output, /Press Enter once the app is created/);
+    assert.doesNotMatch(output, /unsettled top-level await/i);
+    assert.equal(readSettings(home).slack.botToken, 'xoxb-1-ttybot');
+    // ...and on a terminal the macOS conveniences DO fire, which is the
+    // behaviour the non-TTY test asserts is suppressed.
+    if (process.platform === 'darwin') {
+        assert.match(readFileSync(stubLog, 'utf8'), /open/);
+    }
+});