Skip to content

Stabilize PTY example command sync#103

Draft
scotttrinh wants to merge 2 commits intomainfrom
fix/flaky-sandbox-pty-example
Draft

Stabilize PTY example command sync#103
scotttrinh wants to merge 2 commits intomainfrom
fix/flaky-sandbox-pty-example

Conversation

@scotttrinh
Copy link
Copy Markdown
Collaborator

The sandbox PTY example could observe a prompt before the shell was ready to reliably echo command output, which made the example flaky in CI.

Add an explicit round-trip readiness check and wait for a completion sentinel so the example verifies full PTY command delivery before exiting.

The sandbox PTY example could observe a prompt before the shell was
ready to reliably echo command output, which made the example flaky
in CI.

Add an explicit round-trip readiness check and wait for a completion
sentinel so the example verifies full PTY command delivery before
exiting.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to make the sandbox_14_pty_test.py PTY example less flaky by adding an explicit readiness “round-trip” check and a completion sentinel so the example can deterministically confirm command delivery/execution before exiting.

Changes:

  • Add READY_OUTPUT and DONE_OUTPUT sentinel markers for PTY synchronization.
  • Introduce run_and_collect() helper to send a command and read output until a marker is observed.
  • Update the PTY flow to (1) verify readiness via a round-trip and (2) wait for a completion marker before validating output and exiting.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

try:
ready_output = await run_and_collect(
session,
f"printf '{READY_OUTPUT}\\n'\n",
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

collect_output_until(session, READY_OUTPUT) can return as soon as the shell echoes the typed command, because the input line includes the literal text PTY_READY inside quotes. That means this “round-trip” check may succeed without waiting for the printf output to be produced/executed, defeating the goal of ensuring PTY readiness. Consider matching the marker as a full output line (e.g., require newline boundaries around it) or emitting a sentinel that does not appear verbatim in the echoed command (e.g., decode/construct the marker at runtime) before searching for it.

Suggested change
f"printf '{READY_OUTPUT}\\n'\n",
"printf '%s\\n' 'PTY_''READY'\n",

Copilot uses AI. Check for mistakes.
Comment on lines +122 to +126
output = await run_and_collect(
session,
f"printf '{EXPECTED_OUTPUT}\\n'; pwd; printf '{DONE_OUTPUT}\\n'; exit\n",
DONE_OUTPUT,
)
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Waiting for DONE_OUTPUT via substring search has the same false-positive risk as the READY check: the PTY will typically echo the command line, which contains PTY_DONE inside quotes, so output collection may stop before the command finishes running (and before exit completes). To make this a reliable completion sentinel, detect the marker as an actual output line (newline-delimited) or print a marker that isn’t present in the echoed input (e.g., construct it via decoding) and then wait for that.

Copilot uses AI. Check for mistakes.
The readiness and completion sentinels in the PTY example could
match the shell's echoed command line before the command actually
executed.

Require sentinel markers to appear as complete output lines so the
example waits for real PTY output before continuing or exiting.
@vercel
Copy link
Copy Markdown

vercel bot commented Apr 15, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
vercel-py Ready Ready Preview Apr 15, 2026 8:55pm

Request Review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants