-
Notifications
You must be signed in to change notification settings - Fork 0
OutputContext.fileWrite() shares #partialLine buffer with write() and duplicates its logic #4
Description
Summary
OutputContext.fileWrite() was introduced to mirror prompt text (questions and user answers) to the log file without terminal output. It works correctly for its current use case, but has two related issues:
1. Shared #partialLine buffer creates a latent interleaving risk
fileWrite() and write() both append to the same this.#partialLine buffer and flush complete lines to disk on newline boundaries. If write() has buffered a partial line (e.g. a spinner, progress indicator, or any text not yet terminated with \n) and fileWrite() is then called before that partial line is flushed, the two contents will be concatenated in the same buffer and flushed together as one garbled line.
Example scenario:
write(" 🔍 Validating... ") // no newline — " 🔍 Validating... " sits in #partialLine
fileWrite("Proceed? [Y/n]: ") // appends to same buffer → " 🔍 Validating... Proceed? [Y/n]: "
// When a newline eventually arrives, the flushed line is garbled
Why this isn't a problem today: fileWrite() is only called during interactive prompts (promptYesNo via the mirrorOutput callback), and during prompts no other output is being written. But if fileWrite() is ever called in a context where write() has pending buffered content, the two streams will intermix.
Suggested fix: Give fileWrite() its own private partial-line buffer (e.g. #filePartialLine), so the two write paths are independent.
2. Duplicated buffering logic
The body of fileWrite() (lines 356–367) is identical to the file-writing portion of write() (lines 332–345):
// In both write() and fileWrite():
this.#partialLine += text;
const lastNewline = this.#partialLine.lastIndexOf('\n');
if (lastNewline !== -1)
{
const completeLines = this.#partialLine.slice(0, lastNewline + 1);
this.#partialLine = this.#partialLine.slice(lastNewline + 1);
this.#queueWrite(completeLines);
}This could be extracted into a shared private method like #bufferAndFlush(text: string) to reduce duplication. If finding #1 is fixed by introducing a separate buffer, the shared method could accept the buffer as a parameter or use a different approach — but either way the core logic should live in one place.
Context
These findings came from a code review of the Bug 3 fix (prompts not captured in log file) from BUGS.md. The fix itself is correct and well-tested — these are improvement suggestions for robustness and maintainability.
(filed by Claude Opus 4.6 [VS Code Extension] per verbal request of @masonmark)