fix: revert broken keychain stdin, fix login prompt duplication

[ryanmcmillan]

Problem

v1.1.1 changed macOS keychain writes to pipe via stdin. macOS security does NOT support this — it opens /dev/tty and prompts interactively ('password data for new item'). This broke delega login completely.

v1.1.2 fixed the Node 24 crash but introduced a PassThrough stream that echoed the prompt multiple times.

Fix

Keychain: Revert to execFileSync with -w apiKey in argv. This is the only reliable approach without native Keychain bindings. The argv exposure window is <100ms while security runs. Documented the limitation in comments.

Login prompt: Replace PassThrough stream with simple terminal: false readline. Print prompt ourselves, read input without echo. Works on Node 24, no duplicate prompts.

Testing

• npm run build passes
• Tested on Node 24

[greptile-apps]

Greptile Summary

This PR fixes two regressions introduced in v1.1.1 and v1.1.2: a broken macOS Keychain write (where security -w was incorrectly fed via stdin, causing it to hang on /dev/tty) and a duplicate-prompt bug caused by a PassThrough stream workaround for Node 24 compatibility.

Key changes:

• src/secret-store.ts: Reverts writeMacosKeychain to pass the key directly as an argv to execFileSync. The trade-off (brief argv exposure in ps) is real but correctly documented and is the only viable approach without native bindings. This part of the fix is correct.
• src/commands/login.ts: Replaces the PassThrough-based output stream with a terminal: false readline. This resolves the duplicate-prompt issue on Node 24, but the echo-suppression logic is broken: calling process.stdin.setRawMode?.(false) keeps the terminal in cooked mode (echo on, the default), so the API key will be echoed in plain text as the user types. The comment "Disable raw echo at the TTY level" is misleading — setRawMode(false) does nothing to suppress echo. The previous implementation's intent (hiding secret input) is not preserved.
• Additionally, the rl.on('SIGINT', ...) handler won't fire with terminal: false, making it dead code.

Confidence Score: 2/5

• Not safe to merge — the primary fix (secret prompt) has a security regression: the API key is echoed in plain text as the user types.
• The keychain fix in secret-store.ts is correct and well-reasoned. However, the login prompt fix in login.ts fails to suppress echo: setRawMode(false) does not disable TTY echo (it's a no-op in cooked mode), and terminal: false cedes all TTY control to the OS, which echoes by default. The result is that delega login now prints the API key in plain text to the terminal as it's typed — a direct security regression from the intended behavior of promptSecret.
• src/commands/login.ts — the echo-suppression logic needs to be corrected before merge.

Important Files Changed

Filename	Overview
src/commands/login.ts	Replaces broken PassThrough stream with a terminal:false readline, fixing prompt duplication on Node 24 — but the echo-suppression logic is wrong: setRawMode(false) doesn't hide input, so the API key will be visible as typed. SIGINT handler is dead code with terminal:false.
src/secret-store.ts	Correctly reverts macOS keychain write to execFileSync with -w apiKey in argv, with a clear comment documenting the brief argv exposure trade-off. The fix is accurate — security(1) does not accept -w from stdin and opens /dev/tty instead.

Sequence Diagram

sequenceDiagram
    participant User
    participant CLI as delega login
    participant Prompt as promptSecret()
    participant RL as readline terminal:false
    participant Out as process.stdout
    participant Store as key-store
    participant Sec as macOS security(1)

    User->>CLI: delega login
    CLI->>Prompt: call with question string
    Prompt->>Out: write prompt text
    Prompt->>RL: createInterface
    Note over RL: TTY stays in cooked mode<br/>keystrokes echoed by OS
    User->>RL: types API key (visible on screen)
    RL->>Prompt: line event fired
    Prompt->>Out: write newline
    Prompt-->>CLI: answer resolved
    CLI->>CLI: validate format and call API
    CLI->>Store: persistApiKey
    Store->>Sec: execFileSync with key in argv
    Note over Sec: brief argv exposure in ps
    Sec-->>Store: done
    Store-->>CLI: storage label
    CLI->>Out: Logged in confirmation

Loading

_{Reviews (1): Last reviewed commit: "fix: revert keychain stdin approach, fix..." | Re-trigger Greptile}

[greptile-apps]

Echo is not suppressed — API key visible as typed

The comment says "Disable raw echo at the TTY level", but process.stdin.setRawMode?.(false) keeps the terminal in cooked mode (the default), where the kernel TTY line discipline echoes keystrokes automatically. Calling setRawMode(false) is a no-op when the terminal is already in cooked mode, so it does nothing to hide input.

With terminal: false, readline cedes all TTY control back to the OS. The result is that every character of the API key will be echoed to the screen as the user types, defeating the purpose of promptSecret.

To actually suppress echo you need either:

1. setRawMode(true) — enters raw mode so the OS doesn't echo, but then you must collect characters manually until \n.
2. Override _writeToOutput on a terminal: true interface — a common Node.js idiom for password prompts:

const rl = node_readline.createInterface({
  input: process.stdin,
  output: process.stdout,
  terminal: true,
});
(rl as any)._writeToOutput = () => { /* suppress echo */ };
process.stdout.write(question);

The current code regresses from the previous (broken) implementation's intent: users will see their key echoed in plain text.

[greptile-apps]

SIGINT listener is dead code with terminal: false

Node's readline only intercepts Ctrl+C and emits the 'SIGINT' event when terminal: true. With terminal: false, readline does not install a SIGINT interceptor, so this handler will never fire. Ctrl+C will just invoke the default process SIGINT handler and terminate the process without printing the newline.

The handler is harmless to leave in, but if graceful cancellation is desired, a process-level listener would be needed instead:

process.once("SIGINT", () => {
  rl.close();
  process.stdout.write("\n");
  reject(new Error("Cancelled."));
});

[greptile-apps]

close handler can race with line handler

When the line event fires, rl.close() is called synchronously, which immediately emits close. Both handlers call resolve(...), but since it's the same Promise, the second call (with "") is silently discarded — safe, but potentially confusing.

A more defensive pattern guards the resolve:

let settled = false;
rl.on("line", (answer) => {
  if (settled) return;
  settled = true;
  rl.close();
  process.stdout.write("\n");
  resolve(answer.trim());
});

rl.on("close", () => {
  if (!settled) resolve("");
});

[ryanmcmillan]

/greptile review