Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/fixed-372.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- `pluk attach` now validates the CLI command and arguments before creating the tmux session, so a bad `--cli-args` (e.g. an unterminated quote) is reported as a usage error (exit 2) instead of leaving an orphaned session with pipe-pane attached.
10 changes: 5 additions & 5 deletions src/attach.ts
Original file line number Diff line number Diff line change
Expand Up @@ -337,11 +337,6 @@ export function attach(opts: AttachOptions): void {
log(`tmux session "${session}" exists: ${sessionExists}`);

if (!sessionExists) {
console.error(`Creating tmux session: ${session}`);
log(`execFile: tmux new-session -d -s ${shellQuote(session)} -c ${shellQuote(workDir)}`);
tmuxNewSession(session, workDir);
wirePipePane();

let fullCmd = buildCliCommand(cliCmd, opts.cliArgs);
if (opts.dangerouslySkipPermissions) {
const dangerFlag = resolveDangerousFlag(cli);
Expand All @@ -355,6 +350,11 @@ export function attach(opts: AttachOptions): void {
console.error(`Warning: --dangerous has no known auto-approve flag for cli=${cli}; starting without it`);
}
}
console.error(`Creating tmux session: ${session}`);
log(`execFile: tmux new-session -d -s ${shellQuote(session)} -c ${shellQuote(workDir)}`);
tmuxNewSession(session, workDir);
wirePipePane();

console.error(`Starting ${cli}: ${fullCmd}`);
const sendArgs = ['send-keys', '-t', exactTarget(session), '--', fullCmd, 'Enter'];
log(`execFile: tmux ${sendArgs.map(shellQuote).join(' ')}`);
Expand Down
13 changes: 12 additions & 1 deletion src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { PLUK_EVENT_TYPES, type PlukEventType } from './event.js';
import { listAvailableCLIs, bundledPatternsDir, getPatterns, hasAnyPattern, BUILTIN_PATTERNS } from './patterns.js';
import { discoverSessions } from './sessions.js';
import { resolveRunDir, rotateLogFileIfNeeded, validateSessionName } from './run-dir.js';
import { attach } from './attach.js';
import { attach, splitShellWords } from './attach.js';
import { send } from './send.js';
import { ANSI_RED, ANSI_GREEN, ANSI_CYAN, ANSI_DIM, ANSI_BOLD, ANSI_RESET, paintStderr, paintStdout } from './ansi.js';
import { packageVersion } from './version.js';
Expand Down Expand Up @@ -242,6 +242,17 @@ function cmdAttach(args: string[]): void {
process.exit(2);
}

for (const flag of ['command', 'cli-args']) {
const value = flags[flag];
if (value === undefined) continue;
try {
splitShellWords(value);
} catch (err) {
console.error(`${paintStderr(ANSI_RED, 'Error:')} --${flag}: ${(err as Error).message}`);
process.exit(2);
}
}

if (flags['rationguard'] !== 'true') {
for (const flag of ['rebuttal', 'rationguard-bin']) {
if (flags[flag] !== undefined) {
Expand Down
21 changes: 21 additions & 0 deletions test/attach-branch-gaps.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -301,3 +301,24 @@ test('--dir sets the tmux new-session working directory', () => {
assert.equal(code, 0);
assert.match(stubs.log('tmux'), new RegExp(`new-session -d -s agent-wd -c ${workDir}`));
});

test('attach() builds the CLI command before new-session so a bad --cli-args leaves no tmux session (#372)', () => {
// cli.js pre-validates --cli-args, so the CLI-level test cannot tell whether
// attach() itself still orders buildCliCommand() before tmux new-session.
// Call attach() directly to pin that ordering: has-session is the only tmux
// call allowed before the throw, and no new-session/pipe-pane may follow.
const stubs = makeStubs({ which: ['tmux', 'pluk'] });
const runDir = makeRunDir();
const script = `import { attach } from ${JSON.stringify(ATTACH)};
attach({ session: 'agent-badargs', runDir: ${JSON.stringify(runDir)}, noOpen: true, cliArgs: "--model 'unterminated" });`;
const res = spawnSync(process.execPath, ['--input-type=module', '-e', script], {
encoding: 'utf-8',
timeout: 15000,
env: { ...process.env, PATH: `${stubs.dir}:${process.env.PATH}` },
});
assert.notEqual(res.status, 0, 'attach() must throw on an unterminated quote');
assert.match(res.stderr, /Unterminated quote in command arguments/);
const tmuxCalls = stubs.log('tmux').trim().split('\n').filter(Boolean);
assert.deepEqual(tmuxCalls, ['has-session -t =agent-badargs:'],
'only the existence probe may run before the command is validated');
});
11 changes: 11 additions & 0 deletions test/attach-orchestration.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,17 @@ test('fresh session wires pipe-pane before send-keys so start-up output is captu
assert.ok(pipePane < startCli, 'pipe-pane must precede the send-keys that starts the CLI');
});

test('attach rejects unterminated quote in --cli-args before creating tmux session', () => {
const stubs = makeStubs({ which: ['tmux', 'pluk'] });
const { code, stderr } = runAttach([
'bad-args', `--run-dir=${makeRunDir()}`, '--no-open',
"--cli-args=--model 'unterminated",
], stubs);
assert.equal(code, 2);
assert.match(stderr, /Unterminated quote/);
assert.equal(stubs.log('tmux'), '', 'validation must precede all tmux calls');
});

// --- existing-session path ---------------------------------------------------------

test('attach to an existing session skips create/send-keys and warns about --dangerous', () => {
Expand Down
Loading