Skip to content

Commit 4d5372a

Browse files
committed
fix(components/execd): keep bash session newlines to support heredoc scripts
1 parent 2c9af4c commit 4d5372a

File tree

2 files changed

+106
-2
lines changed

2 files changed

+106
-2
lines changed

components/execd/pkg/runtime/bash_session.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,7 @@ func (s *bashSession) readStdout(r io.Reader) {
184184
}
185185
}
186186

187+
//nolint:gocognit
187188
func (s *bashSession) run(command string, timeout time.Duration, hooks *ExecuteResultHook) error {
188189
s.mu.Lock()
189190
defer s.mu.Unlock()
@@ -206,11 +207,14 @@ func (s *bashSession) run(command string, timeout time.Duration, hooks *ExecuteR
206207
wait = 24 * 3600 * time.Second // default to 24 hours
207208
}
208209

209-
cleanCmd := strings.ReplaceAll(command, "\n", " ; ")
210+
cmdBody := command
211+
if !strings.HasSuffix(cmdBody, "\n") {
212+
cmdBody += "\n"
213+
}
210214

211215
// send command + marker, preserving the user's last exit code
212216
// use a subshell at the end to restore $? to the original exit code
213-
cmdText := fmt.Sprintf("%s\n__c=$?\nprintf \"%s${__c}%s\\n\"\n(exit ${__c})\n", cleanCmd, exitCodePrefix, exitCodeSuffix)
217+
cmdText := fmt.Sprintf("%s__c=$?\nprintf \"%s${__c}%s\\n\"\n(exit ${__c})\n", cmdBody, exitCodePrefix, exitCodeSuffix)
214218
if _, err := fmt.Fprint(s.stdin, cmdText); err != nil {
215219
if errors.Is(err, io.ErrClosedPipe) || strings.Contains(err.Error(), "broken pipe") {
216220
s.terminated.Store(true)

components/execd/pkg/runtime/bash_session_test.go

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
package runtime
1919

2020
import (
21+
"fmt"
22+
"os"
2123
"strings"
2224
"testing"
2325
"time"
@@ -173,6 +175,104 @@ func TestBashSessionEnvLargeOutputChained(t *testing.T) {
173175
}
174176
}
175177

178+
func TestBashSession_heredoc(t *testing.T) {
179+
rewardDir := t.TempDir()
180+
controller := NewController("", "")
181+
182+
hooks := ExecuteResultHook{
183+
OnExecuteStdout: func(line string) {
184+
fmt.Printf("[stdout] %s\n", line)
185+
},
186+
OnExecuteComplete: func(d time.Duration) {
187+
fmt.Printf("[complete] %s\n", d)
188+
},
189+
}
190+
191+
// First run: heredoc + reward file write.
192+
script := fmt.Sprintf(`
193+
set -x
194+
reward_dir=%q
195+
mkdir -p "$reward_dir"
196+
197+
cat > /tmp/repro_script.sh <<'SHEOF'
198+
#!/usr/bin/env sh
199+
echo "hello heredoc"
200+
SHEOF
201+
202+
chmod +x /tmp/repro_script.sh
203+
/tmp/repro_script.sh
204+
echo "after heredoc"
205+
echo 1 > "$reward_dir/reward.txt"
206+
cat "$reward_dir/reward.txt"
207+
`, rewardDir)
208+
209+
if err := controller.Execute(&ExecuteCodeRequest{
210+
Language: Bash,
211+
Timeout: 10 * time.Second,
212+
Code: script,
213+
Hooks: hooks,
214+
}); err != nil {
215+
fmt.Fprintf(os.Stderr, "first Execute failed: %v\n", err)
216+
os.Exit(1)
217+
}
218+
219+
// Second run: ensure the session keeps working.
220+
if err := controller.Execute(&ExecuteCodeRequest{
221+
Language: Bash,
222+
Timeout: 5 * time.Second,
223+
Code: "echo 'second command works'",
224+
Hooks: hooks,
225+
}); err != nil {
226+
fmt.Fprintf(os.Stderr, "second Execute failed: %v\n", err)
227+
os.Exit(1)
228+
}
229+
}
230+
231+
func TestBashSession_execReplacesShell(t *testing.T) {
232+
session := newBashSession(nil)
233+
t.Cleanup(func() { _ = session.close() })
234+
235+
if err := session.start(); err != nil {
236+
t.Fatalf("Start() error = %v", err)
237+
}
238+
239+
var stdoutLines []string
240+
hooks := ExecuteResultHook{
241+
OnExecuteStdout: func(line string) {
242+
stdoutLines = append(stdoutLines, line)
243+
},
244+
}
245+
246+
script := `
247+
cat > /tmp/exec_child.sh <<'EOF'
248+
echo "child says hi"
249+
EOF
250+
chmod +x /tmp/exec_child.sh
251+
exec /tmp/exec_child.sh
252+
`
253+
254+
err := session.run(script, 5*time.Second, &hooks)
255+
if err == nil {
256+
t.Fatalf("expected error because exec replaces the shell, got nil")
257+
}
258+
if !strings.Contains(err.Error(), "stdout closed") && !strings.Contains(err.Error(), "terminated") {
259+
t.Fatalf("unexpected error for exec: %v", err)
260+
}
261+
if !containsLine(stdoutLines, "child says hi") {
262+
t.Fatalf("expected child output, got %v", stdoutLines)
263+
}
264+
if !session.terminated.Load() {
265+
t.Fatalf("expected session to be marked terminated after exec")
266+
}
267+
268+
// Subsequent run should fail immediately because the shell was replaced.
269+
if err := session.run("echo still-alive", 2*time.Second, &hooks); err == nil {
270+
t.Fatalf("expected run to fail after exec replaced the shell")
271+
} else if !strings.Contains(err.Error(), "terminated") {
272+
t.Fatalf("expected terminated error, got %v", err)
273+
}
274+
}
275+
176276
func containsLine(lines []string, target string) bool {
177277
for _, l := range lines {
178278
if strings.TrimSpace(l) == target {

0 commit comments

Comments
 (0)