fix(terminal): separate prompt after unterminated command output#987
Conversation
Add a display-layer prompt line break handler so recognized shell prompts move to the next visual line when the final command output line is not newline terminated. Also add a terminal setting to toggle the behavior, sync support, i18n copy, and focused tests for prompt insertion.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2173cd3db2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c13aae5037
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!state.pendingCommand || state.lastPromptText) { | ||
| state.lastPromptText = prompt.promptText; |
There was a problem hiding this comment.
When pendingCommand is true and lastPromptText is still empty (for example, an auto-run startup command is sent before any initial prompt has been cached), this guard skips updating lastPromptText. That leaves the cache empty, so subsequent commands keep entering the same state and prompt line-break insertion never activates for the rest of the session. The prompt cache should be allowed to initialize from the first valid prompt even if it arrives while a command is pending.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab57b6142a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const cursorXBeforeWrite = getCursorX(term); | ||
| const nextData = insertPromptLineBreakBeforePrompt( | ||
| data, | ||
| state.lastPromptText, | ||
| cursorXBeforeWrite, |
There was a problem hiding this comment.
This computes cursorXBeforeWrite from term.buffer.active before xterm has necessarily applied earlier queued chunks, but Terminal.write is asynchronous and the buffer is only current after write callbacks. If command output and the following prompt arrive in separate rapid chunks, the prompt chunk can observe a stale cursor position (often 0) and skip insertion, so the new "Prompt on a new line" behavior becomes nondeterministic based on chunk timing. Consider serializing this decision with the write callback chain (or another post-parse signal) so cursor state reflects prior output.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b0e8906e32
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const cleanupData = clearPasteResidualAfterTerminalWrite(term); | ||
| if (cleanupData) { | ||
| ctx.onTerminalLogData?.(cleanupData); | ||
| enqueueTerminalWrite(term, (done) => { |
There was a problem hiding this comment.
Queueing onSessionData writes behind enqueueTerminalWrite while still emitting exit/status text via writeTerminalLine can reorder output when a session ends before queued chunks flush. In that case, [session closed] (or error text) can be written before still-pending data chunks, so users see stale command output after the close message. This is new with the callback queue and should be fixed by routing terminal line writes through the same queue (or draining/canceling pending writes on exit) so all terminal writes stay strictly ordered.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2878a6b5a5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const cmd = ctx.commandBufferRef.current.trim(); | ||
| if (cmd) ctx.onCommandExecuted(cmd, ctx.host.id, ctx.host.label, ctx.sessionId); | ||
| ctx.commandBufferRef.current = ""; | ||
| markPromptLineBreakCommandPending(ctx.promptLineBreakStateRef); |
There was a problem hiding this comment.
The new prompt-line-break feature is only armed inside if (ctx.statusRef.current === "connected" && ctx.onCommandExecuted), so pressing Enter will not set pendingCommand when onCommandExecuted is undefined even though onCommandExecuted is an optional prop in CreateXTermRuntimeContext. In that valid integration mode, the new setting silently does nothing for normal typed commands because markPromptLineBreakCommandPending is never reached. Move command-buffer/prompt-line-break bookkeeping outside the ctx.onCommandExecuted guard and keep only the history callback invocation conditional.
Useful? React with 👍 / 👎.
| label={t("settings.terminal.behavior.forcePromptNewLine")} | ||
| description={t("settings.terminal.behavior.forcePromptNewLine.desc")} |
There was a problem hiding this comment.
This UI now renders settings.terminal.behavior.forcePromptNewLine and .desc, but only en and zh-CN were updated in this commit; application/i18n/locales/ru.ts still lacks both keys. In Russian locale, users will see raw translation keys instead of readable labels/descriptions for this new toggle, which is a user-facing regression introduced by adding these lookups without completing locale coverage.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d860bae4c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ctx.promptLineBreakStateRef?.current, | ||
| forcePromptNewLine, | ||
| ); | ||
| ctx.onTerminalLogData?.(displayData); |
There was a problem hiding this comment.
writeSessionData now logs displayData after prepareTerminalDataForPromptLineBreak runs, so when forcePromptNewLine is enabled the synthetic \r\n inserted for rendering is also persisted via onTerminalLogData. That makes captured/replayed session logs diverge from the actual backend byte stream (contradicting the new forcePromptNewLine contract in TerminalSettings), which can corrupt transcript fidelity for export/replay workflows whenever a command ends without a trailing newline.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e160e00ac
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const mapped = mapVisibleText(data); | ||
| if (!mapped.text.endsWith(promptText)) return data; | ||
|
|
There was a problem hiding this comment.
The insertion logic treats any chunk whose visible text ends with the cached prompt text as an actual prompt (endsWith(promptText)), but PTY output chunk boundaries are arbitrary. After a command is marked pending, a normal output chunk that happens to end with "$ "/"# " (or another cached prompt) will get an injected \r\n even when the shell has not emitted a prompt yet, which corrupts terminal display for streaming outputs. This should require a stronger prompt confirmation than a raw suffix match on a single chunk.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
2 participants
Add a display-layer prompt line break handler so recognized shell prompts move to the next visual line when the final command output line is not newline terminated.
Also add a terminal setting to toggle the behavior, sync support, i18n copy, and focused tests for prompt insertion.