[PR #1522] [MERGED] fix: prevent terminal execute echo from colliding with output sentinels #28689

New Issue

GiteaMirror · 2026-07-13T14:31:26-05:00

GiteaMirror commented

2026-07-13 14:31:26 -05:00

📋 Pull Request Information

Original PR: https://github.com/moghtech/komodo/pull/1522
Author: @mvanhorn
Created: 7/13/2026
Status: ✅ Merged
Merged: 7/14/2026
Merged by: @mbecker20

Base: 2.3.0 ← Head: fix/terminal-echo-sentinel-collision

📝 Commits (1)

e306f34 fix terminal execute echo colliding with output sentinels

📊 Changes

1 file changed (+8 additions, -1 deletions)

View changed files

📝 bin/periphery/src/api/terminal.rs (+8 -1)

📄 Description

Summary

Fixes the intermittent wrong-output / wrong-exit-code failures of the terminal
execute methods (execute_container_terminal, execute_server_terminal,
execute_stack_exec) reported in #1289, where a run returns

'; date; rc=$?; printf '
Command finished with exit code %d

instead of the real command output, even though the command ran correctly on the
server. As agreed in the issue thread ("a parsing issue of the terminal output,
and the command is executed correctly"), the command itself is fine and this is a
parsing collision, not an execution bug.

The execute command is written to a PTY-backed shell whose stdout is also the
stream the parser reads for its sentinel markers, and local echo is on (needed for
interactive terminals). In bin/periphery/src/api/terminal.rs the built command
used real newline bytes inside the single-quoted printf format strings, so the
command spanned multiple physical lines:

printf '
__KOMODO_START_OF_OUTPUT__

'; date; rc=$?; printf '
__KOMODO_EXIT_CODE:%d
__KOMODO_END_OF_OUTPUT__
' "$rc"

The PTY echoes that multi-line input back before the real output, putting bare
lines equal to __KOMODO_START_OF_OUTPUT__, __KOMODO_EXIT_CODE:%d, and
__KOMODO_END_OF_OUTPUT__ on the stream. The setup loop
(line == START_OF_OUTPUT) breaks on the echoed start marker, then forwards the
echoed '; date; rc=$?; printf ' as output and reads the echoed
__KOMODO_EXIT_CODE:%d (hence exit code "%d") and echoed END_OF_OUTPUT as the
terminator - exactly the reported symptom. This also matches why sh fails
deterministically (verbatim line-discipline echo of the bare marker lines) while
interactive bash/readline usually re-renders continuation lines and only fails
intermittently under arm64/load timing.

The fix makes the command a single physical line by using literal \n escapes
(Rust \\n) inside the printf format strings instead of embedded newline bytes.
printf expands those escapes, so the emitted output is byte-identical, but the
echoed command can no longer contain a bare sentinel line.

Why this matters

These terminal execute methods back Actions and Stack exec, and the failure is
silent and non-deterministic: the command really runs, but the caller sees garbage
output and a %d (or otherwise wrong) exit code. On sh it fails every time; on
bash under arm64/load it fails an unpredictable fraction of runs, which makes it
hard to trust or automate around.

Change

bin/periphery/src/api/terminal.rs, in setup_execute_command_on_terminal:

let full_command = format!(
  "printf '\\n{START_OF_OUTPUT}\\n\\n'; {command}; rc=$?; printf '\\n{KOMODO_EXIT_CODE}%d\\n{END_OF_OUTPUT}\\n' \"$rc\"\n"
);

The inner newlines (inside the single-quoted printf formats) become \\n; the
final trailing \n that submits the command to the shell is left as a real
newline.

Byte-identity proof

Reconstructing the OLD and NEW command strings and running both under sh
({command} = echo hi):

OLD source (7 physical lines, as above) vs NEW source (1 physical line):

printf '\n__KOMODO_START_OF_OUTPUT__\n\n'; echo hi; rc=$?; printf '\n__KOMODO_EXIT_CODE:%d\n__KOMODO_END_OF_OUTPUT__\n' "$rc"

cmp of the two stdouts reports byte-identical output:

$ sh -c "$OLD" > old.txt 2>&1
$ sh -c "$NEW" > new.txt 2>&1
$ cmp old.txt new.txt && echo BYTE_IDENTICAL_OUTPUT
BYTE_IDENTICAL_OUTPUT
$ cat old.txt

__KOMODO_START_OF_OUTPUT__

hi

__KOMODO_EXIT_CODE:0
__KOMODO_END_OF_OUTPUT__

So the change only affects how the command echoes, not what it emits. Only the
START sentinel needs de-colliding for correctness (once the real START marker is
matched, the whole echo has already been consumed), but all three markers are
de-collided together since it is free.

cargo check -p komodo_periphery compiles.

Verification note

This was verified by the byte-identity proof above and cargo check, but not
reproduced end-to-end (that needs core + periphery + Docker with a real PTY). The
change is tiny and output-preserving, so regression risk is low - but it would be
great if you could confirm it under your own repro, especially the arm64/load
bash case, in case you would prefer a different approach.

Closes #1289

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/moghtech/komodo/pull/1522 **Author:** [@mvanhorn](https://github.com/mvanhorn) **Created:** 7/13/2026 **Status:** ✅ Merged **Merged:** 7/14/2026 **Merged by:** [@mbecker20](https://github.com/mbecker20) **Base:** `2.3.0` ← **Head:** `fix/terminal-echo-sentinel-collision` --- ### 📝 Commits (1) - [`e306f34`](https://github.com/moghtech/komodo/commit/e306f343df75134f4058f13933d9893c0fc9c6df) fix terminal execute echo colliding with output sentinels ### 📊 Changes **1 file changed** (+8 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `bin/periphery/src/api/terminal.rs` (+8 -1) </details> ### 📄 Description ## Summary Fixes the intermittent wrong-output / wrong-exit-code failures of the terminal `execute` methods (`execute_container_terminal`, `execute_server_terminal`, `execute_stack_exec`) reported in #1289, where a run returns ``` '; date; rc=$?; printf ' Command finished with exit code %d ``` instead of the real command output, even though the command ran correctly on the server. As agreed in the issue thread ("a parsing issue of the terminal output, and the command is executed correctly"), the command itself is fine and this is a parsing collision, not an execution bug. The `execute` command is written to a PTY-backed shell whose stdout is also the stream the parser reads for its sentinel markers, and local echo is on (needed for interactive terminals). In `bin/periphery/src/api/terminal.rs` the built command used real newline bytes inside the single-quoted `printf` format strings, so the command spanned multiple physical lines: ``` printf ' __KOMODO_START_OF_OUTPUT__ '; date; rc=$?; printf ' __KOMODO_EXIT_CODE:%d __KOMODO_END_OF_OUTPUT__ ' "$rc" ``` The PTY echoes that multi-line input back before the real output, putting bare lines equal to `__KOMODO_START_OF_OUTPUT__`, `__KOMODO_EXIT_CODE:%d`, and `__KOMODO_END_OF_OUTPUT__` on the stream. The setup loop (`line == START_OF_OUTPUT`) breaks on the *echoed* start marker, then forwards the echoed `'; date; rc=$?; printf '` as output and reads the echoed `__KOMODO_EXIT_CODE:%d` (hence exit code `"%d"`) and echoed `END_OF_OUTPUT` as the terminator - exactly the reported symptom. This also matches why `sh` fails deterministically (verbatim line-discipline echo of the bare marker lines) while interactive `bash`/readline usually re-renders continuation lines and only fails intermittently under arm64/load timing. The fix makes the command a single physical line by using literal `\n` escapes (Rust `\\n`) inside the `printf` format strings instead of embedded newline bytes. `printf` expands those escapes, so the emitted output is byte-identical, but the echoed command can no longer contain a bare sentinel line. ## Why this matters These terminal `execute` methods back Actions and Stack exec, and the failure is silent and non-deterministic: the command really runs, but the caller sees garbage output and a `%d` (or otherwise wrong) exit code. On `sh` it fails every time; on `bash` under arm64/load it fails an unpredictable fraction of runs, which makes it hard to trust or automate around. ### Change `bin/periphery/src/api/terminal.rs`, in `setup_execute_command_on_terminal`: ```rust let full_command = format!( "printf '\\n{START_OF_OUTPUT}\\n\\n'; {command}; rc=$?; printf '\\n{KOMODO_EXIT_CODE}%d\\n{END_OF_OUTPUT}\\n' \"$rc\"\n" ); ``` The inner newlines (inside the single-quoted `printf` formats) become `\\n`; the final trailing `\n` that submits the command to the shell is left as a real newline. ### Byte-identity proof Reconstructing the OLD and NEW command strings and running both under `sh` (`{command}` = `echo hi`): OLD source (7 physical lines, as above) vs NEW source (1 physical line): ``` printf '\n__KOMODO_START_OF_OUTPUT__\n\n'; echo hi; rc=$?; printf '\n__KOMODO_EXIT_CODE:%d\n__KOMODO_END_OF_OUTPUT__\n' "$rc" ``` `cmp` of the two stdouts reports **byte-identical** output: ``` $ sh -c "$OLD" > old.txt 2>&1 $ sh -c "$NEW" > new.txt 2>&1 $ cmp old.txt new.txt && echo BYTE_IDENTICAL_OUTPUT BYTE_IDENTICAL_OUTPUT $ cat old.txt __KOMODO_START_OF_OUTPUT__ hi __KOMODO_EXIT_CODE:0 __KOMODO_END_OF_OUTPUT__ ``` So the change only affects how the command *echoes*, not what it emits. Only the START sentinel needs de-colliding for correctness (once the real START marker is matched, the whole echo has already been consumed), but all three markers are de-collided together since it is free. `cargo check -p komodo_periphery` compiles. ### Verification note This was verified by the byte-identity proof above and `cargo check`, but not reproduced end-to-end (that needs core + periphery + Docker with a real PTY). The change is tiny and output-preserving, so regression risk is low - but it would be great if you could confirm it under your own repro, especially the arm64/load `bash` case, in case you would prefer a different approach. Closes #1289 --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

GiteaMirror added the pull-request label 2026-07-13 14:31:26 -05:00

GiteaMirror changed title from ~~[PR #1522] fix: prevent terminal execute echo from colliding with output sentinels~~ to [PR #1522] [MERGED] fix: prevent terminal execute echo from colliding with output sentinels

2026-07-14 20:48:55 -05:00

GiteaMirror closed this issue

2026-07-14 20:48:55 -05:00

Sign in to join this conversation.

Branches Tags

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/komodo#28689