Fix Assertion Mechanism

[UdeshyaDhungana]

Note

Medium Risk
Changes the PTY/virtual-terminal IO model to a background relay with new concurrency/locking, which could affect timing, EOF handling, and test flakiness across platforms.

Overview
Refactors the assertion/read mechanism to stop doing timed async reads from the PTY: removes internal/async_reader and internal/condition_reader, adds a dedicated ptyRelay goroutine that continuously drains PTY output into the virtual terminal, and updates ShellExecutable.ReadUntilConditionOrTimeout to poll VT state and detect process exit via the relay (mapping PTY EOF/EIO to ErrProgramExited).

Makes VirtualTerminal thread-safe with an RWMutex around Write and GetScreenState, and adjusts tests to use the new ErrConditionNotMet. Adds a new failing autocompletion scenario + fixtures to validate cases where TAB rings the bell but must not rewrite the input line, and adds a small post-TAB sleep in the autocomplete test case to reduce racey assertions.

^{Reviewed by Cursor Bugbot for commit 09d9e19. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

• ✅ Fixed: Process exit check skips final condition evaluation
  • Reordered the condition() check to occur before processExited() in ReadUntilConditionOrTimeout, ensuring final data that satisfies the condition is detected before the function returns ErrProgramExited.

Or push these changes by commenting:

@cursor push 4295a72fef

Preview (4295a72fef)

diff --git a/internal/shell_executable/shell_executable.go b/internal/shell_executable/shell_executable.go
--- a/internal/shell_executable/shell_executable.go
+++ b/internal/shell_executable/shell_executable.go
@@ -164,6 +164,10 @@
 	deadline := time.Now().Add(timeout)
 
 	for !time.Now().After(deadline) {
+		if condition() {
+			return nil
+		}
+
 		if b.relay.processExited() {
 			if isPtyTerminalError(b.relay.terminalErr) {
 				return ErrProgramExited
@@ -171,10 +175,6 @@
 			return b.relay.terminalErr
 		}
 
-		if condition() {
-			return nil
-		}
-
 		time.Sleep(2 * time.Millisecond)
 	}

_{You can send follow-ups to the cloud agent here.}

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix prepared a fix for the issue found in the latest run.

• ✅ Fixed: Data race: concurrent VT writes and reads across goroutines
  • Added sync.RWMutex to VirtualTerminal wrapper with Lock() in Write() and RLock() in GetScreenState() to synchronize concurrent access from the ptyRelay goroutine and the main goroutine.

Or push these changes by commenting:

@cursor push 24e40f126b

Preview (24e40f126b)

diff --git a/internal/vt/virtual_terminal.go b/internal/vt/virtual_terminal.go
--- a/internal/vt/virtual_terminal.go
+++ b/internal/vt/virtual_terminal.go
@@ -1,11 +1,14 @@
 package virtual_terminal
 
 import (
+	"sync"
+
 	"github.com/charmbracelet/x/vt"
 	"github.com/codecrafters-io/shell-tester/internal/screen_state"
 )
 
 type VirtualTerminal struct {
+	mu       sync.RWMutex
 	vt       *vt.Terminal
 	rows     int
 	cols     int
@@ -50,10 +53,15 @@
 	if len(p) == 0 {
 		return 0, nil
 	}
+	vt.mu.Lock()
+	defer vt.mu.Unlock()
 	return vt.vt.Write(p)
 }
 
 func (vt *VirtualTerminal) GetScreenState() screen_state.ScreenState {
+	vt.mu.RLock()
+	defer vt.mu.RUnlock()
+
 	cellMatrix := make([][]string, vt.rows)
 
 	for i := 0; i < vt.rows; i++ {

_{You can send follow-ups to the cloud agent here.}

^{Reviewed by Cursor Bugbot for commit be2701c. Configure here.}

[rohitpaulk]

Not sure what "pump" refers to here

[UdeshyaDhungana]

ptyRelay.run() was pump() initially, which was refactored later. I've updated this comment.