Superseeded: Run scripts under a PTY so terminal-aware tools work - #286
Closed
gorandodig wants to merge 84 commits into
Closed
Superseeded: Run scripts under a PTY so terminal-aware tools work#286gorandodig wants to merge 84 commits into
gorandodig wants to merge 84 commits into
Conversation
The harness reconstructs the topology from ADR-001 - a child spawned into a new session whose fd 0 still points at the terminal of the session it left - using only synthetic processes, so it documents the defect independently of any fix and never validates one. The Node probe is a manual, non-gating diagnostic whose outcome varies by host.
Locks in today's exit-code passthrough, merged stderr, timeout result, cancellation, large-output drain, and output sanitizing, so the PTY backend rewrite can be proven equivalent.
A rendered script inherited fd 0 pointing at the renderer's terminal, and a child in a new session bypasses SIGTTIN for a terminal it does not own, so reads succeeded and consumed the user's keystrokes. Pointing fd 0 at /dev/null ends that and gives an immediate EOF instead; interim until the PTY backend lands.
The macOS bug this branch addresses had no macOS runner, and the unit suite had no Windows runner at all. Per-platform mypy plus per-module strict error codes protect the platform-split modules that follow. The removed test step sourced .env.dev.example, which is not in the repo; coverage still uploads from Ubuntu.
The macOS bug this branch addresses had no macOS runner, and the unit suite had no Windows runner at all. Per-platform mypy plus per-module strict error codes protect the platform-split modules that follow. The removed test step sourced .env.dev.example, which is not in the repo; coverage still uploads from Ubuntu. The tests job runs one command per step so a failing command fails the job on Windows, where PowerShell otherwise continues past a failed native command. Tilde-expansion tests set USERPROFILE alongside HOME, since expanduser reads USERPROFILE on Windows. � Conflicts: � .github/workflows/lint-and-test.yml
The launcher attaches a PTY slave to fds 0, 1 and 2, verifies the terminal invariants before exec, and reports progress as typed length-framed records. An acknowledgment barrier holds the target until the parent has recorded its process group, and -I -S keeps anything else from running before that point.
One pseudoterminal backs the target's three standard descriptors. The spawn sequence completes a strict framed handshake and an acknowledgment barrier that records the process group before the target can run. A single reader thread owns the master descriptor and services a bounded, byte-accounted input queue.
Adds the fault-injection and lifecycle suite: reader shutdown, ack-barrier cancellation on both sides, descriptor ownership, escalation and reap ordering, VEOF delivery, input-queue receipts, and the bounded final drain. Two fixes fell out: the reader's select unpacking and joining a reader that never started.
Renders the terminal stream into scrollback plus final screen instead of stripping escape bytes, so repaints and progress rewrites collapse the way a terminal collapses them, and caps the transcript head and tail. Fixtures are real recordings of npm, pytest, a spinner, a full-screen repainter, and a detached run.
The parser now runs in the reader through its byte-feed hook, so a target that emits a device-status, cursor-position or attributes query gets an answer instead of hanging to the timeout. Each reply is one non-blocking admission into the ordered input queue, and its obligation is tracked to completion.
A failure part-way through _open_channels() leaked the descriptors already opened and escaped as a raw OSError instead of the environment-error channel. The final drain appended straight to the raw buffer, so output still in flight at close() never reached read_output(). close_and_fail_all() dropped the in-flight item without closing its transaction, leaving termios unrestored when an EAGAIN'd VEOF met a close. submit() copied before validating the size and admitted unlimited zero-length items; the queue is now bounded in items as well as bytes. Two tests were not testing what they claimed: the daemon-thread target outlived nothing, and the pre-ack cancellation raced a timer instead of the delayed-ack hook.
Escape sequences are framed and capped before pyte sees them, so an unterminated OSC string or CSI parameter no longer grows the reader's memory, an oversized sequence is dropped rather than parsed, and a parse failure costs one sequence instead of the rest of an OS-sized read. One cell can no longer accumulate combining marks without bound. The normalizer gained an idempotent finalize(), called at reader shutdown beside the decoded-stream flush, so a trailing partial UTF-8 sequence renders as U+FFFD instead of vanishing. Terminal-reply failures are counted in full but retained as a bounded, deduplicated sample, and failure_detail() names that sample plus how many failures it omits. Fixture tests assert a committed golden rendering per recording and compute ratios byte to byte; new tests cover the caps, boundary-invariant recovery, a query flood, and two-thread races at the callback and quiesce boundary.
Wraps today's Popen path — merged streams, the drain thread, the Linux pipe widening — behind the TerminalProcess interface, keeping stdin at DEVNULL so neither the escape hatch nor the Windows interim can steal keystrokes again. The backend has no input channel, so its query responder starts quiesced and output is fed through the normalizer.
Scripts now run on the PTY backend on POSIX and on the legacy pipe backend on Windows, an interim until the ConPTY backend lands. One arbiter ranks the conditions that can race — infrastructure failure, cancellation, the deadline, an undelivered terminal reply, the target's exit — so launch and reader failures reach the environment-error channel instead of the patcher, and the timeout message names the absent input driver. The temp file now holds the rendered transcript, with the raw bytes kept beside it.
Setting CODEPLAIN_NO_PTY=1 in Codeplain's own environment runs scripts on the legacy pipe backend, read once per spawn at the single construction site and warned about on every use. It is never selected automatically — a failed openpty() stays an environment error — and the variable is stripped from the child's environment so a rendered script cannot branch on it.
Add a validation suite that drives the terminal contract through the real path: session and foreground-group invariants, Node and shell compatibility, lifecycle bounds, the documented process-tree limits, PTY exhaustion, terminal isolation on both backends, the child environment, and a detached run.
The new job builds and installs this checkout's wheel, then runs Node through the installed execute_script(), so it needs neither Docker nor the API key. Both existing jobs now set up Python 3.11, matching the project's pin.
Each poll now records every observable fact — target exit, expired deadline, set cancellation, reader failure — before the rank table picks one, so racing conditions are arbitrated instead of published in discovery order. Any backend exception, not only TerminalProcessError, is classified as an environment failure with its detail, and the launch failure is recorded before teardown so a cleanup diagnostic follows it rather than replacing it. Both backends publish a reader that outlives its join bound, the pipe reader publishes read failures that happen while it is active, and the PTY drain suppresses only PTY EOF and a closing backend's EBADF. The raw transcript is written as a sibling of the published output file instead of an orphaned temp file, and a cancelled run leaves no artifact at all.
Add the missing case for terminating a process group that was stopped mid-run, and make the detached case launch its parent through the real nohup binary. Survivors are now registered by pidfile the moment a process starts and swept with bounded waits and escalation, so cleanup no longer depends on where a case failed. Pin the backend read size to one byte in the fragmented-stream case, and judge the renderer-death case on the signal that killed it and on a stable-quiet heartbeat window.
The repr highlighter interleaved ANSI codes inside logged text on color-capable terminals, breaking the exactly-as-logged contract that markup=False already promised. Disable highlighting on the same path.
Redirecting stdin to DEVNULL does not stop a Windows child from opening CONIN$ on the console it inherited, so children are now created with CREATE_NO_WINDOW and get a console of their own. The legacy backend also honours a stop event that is already set before it launches anything.
The wrapper joined the render thread for 0.7 seconds, less than the SIGTERM grace a script teardown runs to its end, so the CLI could exit mid-escalation and leave a descendant alive. The wait is now the sum of the teardown budgets the backend can spend, and a thread still running past it is reported.
The characterization harness documents the topology execute_script() produced before this branch, not the one it produces now. available_backends() has no callers and only leaked internal backend names into the public surface.
The characterization harness is a session leader whose controlling terminal is the PTY it opens; Linux delivers SIGHUP when the master closes, killing it before the report is written. Ignore the hangup — it is teardown noise. test_terminal_process imported termios at module level, breaking collection on Windows; move it behind the existing platform guard.
Parametrize lists reference pty_exec at import time, so the skipif mark cannot save collection; skip the module before anything evaluates.
The arbiter fixture name must end in .ps1 on Windows or execute_script rejects it before the injected backend is reached. The git-plumbing suites skip on native Windows: files land on disk with CRLF and GitPython keeps repository handles open, breaking diff assertions and temp-dir teardown. The drain-bound test now waits for the escapee's flood before closing, so the escapee is provably still writing while close() drains.
Every Repo in git_utils leaked its persistent git cat-file children; they are only reaped by close(). On Windows a live child keeps the repository directory undeletable, which is what broke temp-dir teardown — and on every platform the children accumulate for the lifetime of a render. All repos now close before returning; a closed Repo re-acquires resources lazily, so the returned handles stay usable. Tests write files with explicit LF (text mode writes CRLF on Windows, which the diff assertions would see) and the fixtures collect test-held repos before the temp directory is removed. The win32 skips are gone.
Marshaling for CreateProcessW (list2cmdline quoting, the double-NUL environment block, and rejection of embedded NULs, '=' in a name and an empty name), the bounded input queue with its reserved partition and control lane, and the writer protocol that owns every write to a synchronous input pipe. Split from the backend module, which binds kernel32 at import time, so these rules run in every platform's test suite instead of only on a Windows runner.
One pseudoconsole behind the script's standard handles and one Job Object holding its process tree, both attached through the same proc-thread attribute list so the child is created inside the job or not at all. Ownership is incremental from before the first allocation, and the ordered teardown runs above the rollback stack so a session that outlives its bound can be handed to a daemon finalizer. Windows selects it whenever CODEPLAIN_NO_PTY is not set; the escape hatch keeps selecting the legacy pipe backend on both platforms. Builds below 17763 report an environment error rather than downgrading to pipes.
…lain-tty Phase C) execute_script() gains an explicit platform_test_runtime option — never a process-global switch. A runtime execution gets a per-execution broker, the helper prepended to PATH from the broker's own bin directory, and a scoped environment with caller-supplied CODEPLAIN_TTY_* values stripped before the broker's own are added; a broker that cannot start is an environment error (exit 69) and the script never runs. Unit-test and environment-preparation executions are unchanged. Only the conformance action asks for the runtime (acceptance tests extend and execute the same suite), and only when the cached preflight passed: one real round trip that starts a broker, installs the helper, and completes a wait-for through the socket exactly the way a generated test will. The same preflight gates the capability advertisement on the three conformance/ acceptance API calls, so the API is told about a runtime only after this machine has proven it can provide it. The acceptance-gate reproduction now passes through the real execution path: a conformance-style script drives a getpass child (whose TCSAFLUSH discards the spawn-time VEOF) through codeplain-tty instead of hanging to the 120-second timeout.
…y Phase E, client side) The last net behind the API's response validation: at module completion, before the build is copied anywhere, the build folder is walked and any reference to codeplain-tty, the codeplain_tty module, or the CODEPLAIN_TTY_ environment prefix fails the render with a clear message. Vendor and build directories are skipped; internal conformance/acceptance tests live outside the build folder, so any hit is a violation by definition.
The transcript is rendered per line with trailing whitespace stripped, so a needle quoting a prompt verbatim — 'Master password: ' — could never match as written; every such wait burned its full timeout and stacked past the script budget. Deterministically reproduced from the cli-password-manager failure in the pty-with-codeplain-tty benchmark run (17 of the render's conformance failures were 120-second timeouts): the generated tests drove the CLI through codeplain-tty exactly as instructed, and hung only on the trailing space in 'wait-for "Master password: "'. The broker now strips end-of-line whitespace from wait-for and wait-until-absent needles before matching; a needle that is empty after the normalization is a usage error.
The server's internal conformance-fix loop can spend its whole attempt budget on one functionality; that outcome now arrives as a structured 400 (error_code ConformanceTestsFixExhausted) instead of a raw 500. The client maps it to a typed exception carrying the server's message, so the render fails with 'Could not fix conformance tests issue ... Please review and rewrite the specification' rather than an opaque HTTPError. Unknown future codes still fall through unchanged. Found via the bookshelf-api failure in pty-with-codeplain-tty-retry2: the customers module's fix loop exhausted 10 server-side attempts, Flask 500'd, and the client died with no explanation in codeplain.log.
A wait-for matched anywhere in the cumulative transcript, so the second of two sequential interactive targets in one test file matched the first target's stale prompt instantly, typed into a terminal nobody was reading yet, had those bytes discarded by the new target's TCSAFLUSH, and hung the whole script to its 120-second timeout. Deterministically reproduced with two getpass children with realistic (Argon2id-like) delays between prompts — exactly the cli-password-manager failure in pty-with-codeplain-tty-retry2 (FRID 2 exhausted; the trailing-space fix had removed the earlier failure mode, leaving this one). Each successful wait-for now consumes the transcript through its match, and later waits (including wait-until-absent) look only beyond that cursor; the transcript re-renders as the screen changes, so the cursor is clamped rather than trusted exactly.
A/B arm ab-no-analyze-plus-tty-no-prompts gave up on python_tui at FRID 1
inside the unit-test fix loop and told the user only:
ERROR codeplain: None
ExitWithError printed whatever payload the failing action handed over, and
actions reach that state by three routes: an encoded RenderError dict, a plain
string, or nothing at all. The last route printed "None"; the first printed a
raw dict repr, which is what the conformance-fix-exhausted path has been
showing all along:
ERROR codeplain: {'error': {'message': "The renderer was unable to ...",
'type': None, 'details': None}}
Unwraps the reason at the log site and falls back to last_error_message, the
same message the returned payload already carried — so the line a user reads
is never less informative than the error the renderer returns.
…toring one The conformance-fix loop reports exhaustion as a typed, actionable error (105b215 server-side, d6777d1 client-side). The two client-side loops that can also give up did not. RenderFunctionalRequirement gives up when a functionality's unit tests still fail after MAX_CODE_GENERATION_RETRIES re-renders. It set last_error_message but returned no payload, which is the `ERROR codeplain: None` seen when ab-no-analyze-plus-tty-no-prompts abandoned python_tui at FRID 1. It now returns an encoded RenderError typed UNIT_TESTS_FIX_EXHAUSTED, carrying the frid and a message that mirrors the conformance wording: what could not be produced, for which functionality, and that the specification needs review. RefactorCode already returned a payload, but built its message without an f-string prefix, so the user was shown literal `{MAX_REFACTORING_ITERATIONS}` and `{render_context.frid_context.frid}`. No API change: unlike conformance, this loop is entirely client-side, so a REST error code would be dead code.
Every loss in the retry5-era benchmarks was a fix loop spending its whole
budget re-patching one file against one unchanging failure: cli-password-manager
20 conformance attempts on vault_cli FRID 2, loglens the same on loglens_cli
FRID 2, python_tui 16+ unit-test attempts on FRID 1. The loop could not tell it
was stuck, and the only externally visible outcome was "did the render abort" —
a rare binary event, too coarse to compare configurations against.
Adds per-(module, FRID, loop) accounting for both loops. Each script run is
recorded with a fingerprint of its failure output, normalised past the tokens
that differ between two runs of the same failure (renderer temp paths,
durations, addresses) while leaving genuinely different failures apart.
Consecutive identical fingerprints are counted, and three in a row are reported
as they happen rather than at exhaustion.
Counts are emitted as a greppable line per FRID:
[fix-loop] module=vault_cli frid=2 conformance=20 conformance_failed=20 max_repeat=20
on FRID completion, and for the whole render on both the completed and the
failed path — the FRID that exhausted its budget never reaches
FinishFunctionalRequirement, and its numbers are the ones worth having.
This is measurement, not behaviour: nothing here changes what the renderer
produces, so results stay comparable across the change. It makes
iterations-to-convergence observable per FRID, which is what a rare binary
outcome could not give: a near-continuous measure that says something after a
single run.
Switching strategy on detection (instrument/diagnose/delete rather than
re-patch) is deliberately not included — that would change renderer output and
break comparability with the runs in flight.
The exit summary reaches the terminal through Rich's print, which never touches
logging, so codeplain.log simply stopped at whatever happened to be logged last.
An artifact that ends mid-render is indistinguishable from a process that died
silently — originally noted for hycu's ConflictingRequirements abort, and it
just blocked a real diagnosis: cli-password-manager in
codeplain-tty-capability-run2 rendered to completion (22 functionalities,
48m54s) yet delivered a build with no CLI entry point, and the captured log
ended at 01:29:57 mid-render with no record of how it finished.
Adds a trailer written through the codeplain logger — so it lands in the log
file — on every exit path, since print_exit_summary is called from a finally:
[render-trailer] outcome=completed render_id=... functionalities=22
render_time_s=2934 generated_code=- spec=vault_cli...plain
[render-trailer] error=<reason> (failed renders only)
Handlers are flushed explicitly; a process exiting immediately after would
otherwise defeat the point of writing it.
The trailer doubles as a probe: because it is written unconditionally, a
captured log *without* one proves the file was truncated rather than merely
uninformative — which is the open question behind the missing entry point.
…ests The final audit walked the whole build folder and treated any codeplain-tty reference as a violation, on the stated assumption that internal tests live outside it. They do not: a module's build folder carries a conformance_tests/ subtree, and those tests drive the helper because that is what it is for. So CreateDist raised PlatformBoundaryViolation *after* the render succeeded. The build was never copied, run_state.render_generated_code_path stayed empty, and the harness fell back to whatever tree it could find. Run against the cli-password-manager artifact from codeplain-tty-capability-run2, the audit flags four files, all of them conformance tests — which is why that render reported 22 functionalities in 48m54s and delivered a build with no CLI entry point, and why code_retrieval_example scored 0/10 with generated_code=- in the wave2 re-baseline. Two changes: - Internal test trees (conformance_tests, acceptance_tests, dist_conformance_tests) are exempt from the audit. Delivered code beside them is still audited — a test asserted for that explicitly. - The exit summary and the render trailer now report an error whenever there is one, not only when the render failed. A render that completes its functionalities and then raises on the way out printed a success banner, logged no reason at all, and exited 1 — which is exactly how this went unnoticed across a whole benchmark run.
gorandodig
force-pushed
the
node-subprocess-handling
branch
from
August 18, 2026 17:31
edf07e0 to
3127595
Compare
CI caught what my local check missed: I ran black with --quiet piped into tail, which suppresses the "would reformat" message and discards the exit code, so the gate looked clean when it was not.
Second false-positive class, found on loglens in the wave2 re-baseline: the only file the audit flagged was codeplain.log, which records broker activity and module names and so contains codeplain-tty by construction. The directory exemption added in 3127595 does not cover it — it is a file, not a tree. Against the real artifacts the audit now reports nothing for either loglens or cli-password-manager, where it previously flagged one and four files.
Matches the plain2code* naming the rest of the client entry points use. The console script stays `codeplain-tty` — only the module providing it moves, so generated tests and the broker's argv are unaffected. Deliberately unchanged: the `codeplain_tty` key in the platform_test_runtime capability descriptor. That is the wire contract the server parses, not a module name, and renaming it would break capability negotiation against every deployed API. The portability audit gains `plain2code_tty` as a marker, since that is now the importable name a generated build could reach for. The old module name is kept alongside it so a build that copied an older reference is still caught.
codeplain-tty is new on this branch, so no build can carry a reference to the old module name — the marker guarded against nothing and would have flagged the capability descriptor key if it ever appeared in a file.
b4d8d7d moved the module but committed only the rename — the entry point and the broker's argv path stayed on the old name, so the pushed tree declared a console script for a module that no longer existed. Local runs passed because they ran against the working tree, not the commit.
A headless render suppresses Rich output and attaches no TUI handler, so its only sink was the log file: the process said nothing on stdout for its entire run, and the benchmark harness surfaced that file only after the render ended. A cli-password-manager render wedged for four hours and a healthy one are indistinguishable from outside — the wedged one emitted 858 lines, all in its first two minutes. Headless is the mode CI and benchmarks use, so the diagnostics added for exactly this problem could only ever be read once the budget was spent. Attaching a second handler exposed a latent hazard: one record is passed to every handler in turn, and both formatters rewrote record.msg in place, so each would re-indent the other's continuation lines. They now format a copy. frid_summary reported one max_repeat as the max across both loops, which reads as whichever loop the reader has in mind. The counts sat right beside it: run4's cli-password-manager showed max_repeat=7 next to conformance=42, and the log showed conformance had in fact repeated three times while the unit loop was the one stuck at seven. Each loop now reports its own streak. The aggregate stays -- the benchmark series already collected is indexed on it, and dropping it mid experiment would strand those runs.
The conformance fix loop's failure mode is repetition, not slowness: it re-sends the same fix request, gets back a patch that changes nothing the test can see, and does it again. A wedged cli-password-manager render failed conformance 20 times on one functionality with a streak of 8 -- while its unit loop never failed once -- spent 5h20m, and still scored 1/16. The same signature appears on bookshelf-api, so it is not one spec's quirk. Until now nothing in the loop could tell that state from ordinary slow convergence, so it patched until the attempt limit. Regenerating the conformance test is a genuinely different move: it discards the test the loop cannot satisfy rather than editing code against it again. That path already existed for the attempt limit, so this reaches it as soon as there is evidence instead of after twenty blind patches, and it draws on the same re-render budget -- once spent, the loop patches to the limit and stops, exactly as before. The threshold is the one the warning already uses, and three is validated in both directions by a single run: a healthy functionality repeated a failure twice and then converged, so two would abandon tests that were about to pass, while the wedged one went from three to eight and never recovered. Reading the streak needed an accessor. record() returns it to whoever is recording, but the fix action runs after the test action and has to ask again from its own call site.
…ecution Closing a socket interrupts a blocked accept() on macOS and BSD but not on Linux, where the FD is released and the parked syscall is not woken. The broker closed its listener from close() and joined the server thread with a five second bound, which on Linux always expired: the thread stayed in accept() on a socket whose path close() had just unlinked, so nothing would ever connect to it again and it never returned. A render runs one broker per test-script execution, so that is one thread and one socket held for the life of the render, every time. A benchmark render logged the give-up message on 275 of 275 conformance runs and was still going; the count tracks executions exactly and is uncorrelated with anything going wrong -- a run with zero script timeouts logged it 49 times out of 48. Against a typical thousand-FD limit a long render could plausibly exhaust it, and the failure would surface as the broker refusing to start rather than as anything resembling the cause. The message deserved attention for a second reason: firing on every close, it could no longer distinguish a genuinely stuck thread from ordinary operation. Giving accept() a poll bound lets the loop look at the closing flag on its own schedule rather than depending on a client arriving. socket.timeout subclasses OSError, so it has to be caught ahead of the handler that treats an error as shutdown. The three leak tests would only fail on Linux, where the platform is unkind, so they cannot fail on a developer laptop. The decisive test sets the closing flag and leaves the listener open, which pins the loop's own behaviour everywhere.
…loop The conformance switch shipped in ff1f2fc fired on three identical failures, which generalised from the one wedge that had been captured in full. Two runs since then show the generalisation was too narrow in both directions. A loop can fail every single time without ever repeating itself. A task-manager render failed one functionality's conformance tests 40 times out of 40 with a longest identical run of two, exhausted its whole budget, and reached regeneration only through the attempt limit -- exactly as it would have without the switch. No streak threshold can see that: at three it never fires, at two it fires on renders that are converging. So repetition joins a second arm that counts consecutive failures however they look, set at six because the highest failure count on a functionality that then recovered is four. The unit loop wedges too, which the earlier reasoning had ruled out from a single render where it never failed at all. A cli-password-manager render wedged in both loops on the same functionality: unit repeated 17 times, conformance 3. The unit loop already gives up at its attempt limit and restarts the functionality, so it reaches the same decision on the same evidence, eleven minutes sooner in that render. Restarting is destructive enough to want a careful threshold, and the data makes it easy: across three renders every healthy functionality finished with unit_max_repeat=1, and nothing was observed between that and the 17. Both loops now ask one question, so a future change to what counts as stuck cannot drift between them. Neither switch turns a wedge into a pass, and the wall-clock case is weaker than it first looked: the run that exercised the conformance switch finished in 2h35m against prior wedged runs of 2h43m, 4h09m, 4h47m and 5h20m -- the fastest, but only just below a range already 2h37m wide, and a single run cannot separate the switch from that spread. What they bound is the waste.
A driverless execution gets one end-of-file at spawn. `getpass` calls `tcsetattr(..., TCSAFLUSH, ...)` before reading, and TCSAFLUSH means discard pending input -- so the end-of-file is gone by the time the read happens and the target waits for input nobody will send. It costs the script its whole timeout, and because the timeout is handed to the fix loop as an ordinary failure, the loop reads it as a defect in the generated code and patches against it. One benchmark render did that seventeen times in a row on one functionality while its conformance loop never failed at all; the control arm of the current A/B logged the same diagnostic 69 times across five renders. The broker exists for exactly this, but unit tests never get one: they are part of the delivered codebase and must not depend on Codeplain's test tooling, which the build audit enforces. So the driverless path has to answer for itself. Quiet is the only evidence available from outside -- the parent cannot see the child's tcsetattr -- so a target that is alive and has stopped producing output gets the end-of-file again. That describes a program blocked on a read, and also one that has simply finished talking; with no driver attached both want the same answer, which is what makes repeating it safe. Bounded at three, because a target still quiet after that is not waiting on the terminal and a stuck script should not also be a noisy one. The flag has to be threaded through rather than read from INPUT_DRIVER, which is a module default nothing assigns: the driver is chosen per execution, and pushing an end-of-file into a terminal the broker is driving would answer a prompt the generated test meant to answer itself. The end-to-end case spawns a real getpass target and asserts it exits; without the re-delivery it reproduces the production diagnostic verbatim and hangs to timeout.
The switch had two rungs: patch, then discard the conformance test. It skipped the one in between -- asking the same question in a way that says the previous answers achieved nothing. Every fix request was identical apart from the failure text, so a stuck loop re-sent the same question and got the same patch, and the only escalation was a temperature nudge that cannot tell stuck from slow. Now a stalled loop first sends one request carrying the reason it is stalled, which the API turns into a section asking which kind of failure this actually is. That matters here specifically: the failures these loops kept patching were often script timeouts and missing entry points rather than wrong answers, and no amount of editing implementation code resolves those. Only if that request changes nothing does the loop discard the test, and only then does it give up. Once spent the rung does not repeat, so a loop that stays stuck still reaches regeneration and then the attempt limit on the same budget as before. An ordinary request omits the field entirely and is byte-identical to what it was. The fixture had to set asked_with_stall_context explicitly: a MagicMock answers truthily, which silently skipped the new rung and made two existing tests pass for the wrong reason.
The consecutive-failure arm was applied to both loops on one threshold, and the threshold was calibrated on conformance data because no unit-loop equivalent existed: the highest failure count on a functionality that then recovered was four, so six looked safe. It was not. On the unit side it fired on loops that were working. Two renders showed the same signature -- unit=7 unit_failed=7 unit_max_repeat=1, seven failures with none of them alike -- and in both the arm restarted the functionality and the render scored 0/10. One of them burned its whole restart budget and gave up in under five minutes. Every render without a unit restart scored 2 or 3. Seven different failures is a loop working through issues one at a time; restarting throws all of it away, which is a far worse trade than the one the conformance side makes by discarding a single test file. In both harmful cases unit_max_repeat was 1, so the streak arm would never have fired. That arm stays: across three renders every healthy functionality finished with unit_max_repeat=1, and nothing was observed between that and the 17 reached by the one that wedged, so a streak of three is a state healthy renders do not enter. The conformance loop keeps both arms. There the consecutive arm is what catches a loop failing 40 times out of 40 with a longest identical run of two, which no streak threshold reaches, and its remedy costs one regenerated test rather than a functionality's implementation. Removing it does not make such a unit loop run forever; it stops where it always did, at the attempt limit.
…once The regeneration budget was one per functionality, and that one was the difference between a render that finished and a render that did not. In a ten-task benchmark run, every render that failed to publish died at the conformance attempt limit -- a path only reachable once this budget is spent: one regeneration, then twenty fixes that changed nothing, then abandonment. Both examples failed this way. The single render that completed spent exactly one regeneration on each of three separate functionalities and cleared the bar with nothing to spare; it scored 9/10 where every wedged render scored its example's floor. Regeneration discards a test the loop has already proven it cannot satisfy. Stopping after the first one abandons the render at the point the move is still working. Three rather than more: each regeneration resets the attempt counter, so the worst case for a genuinely unfixable functionality is four rounds of patching instead of two, and that cost falls on renders that were going to fail anyway. The accompanying test names the count 1 literally rather than deriving a range from the constant -- a derived bound is vacuous at exactly the value it is meant to rule out, and an earlier draft passed against a budget of 1 for that reason. Claude-Session: https://claude.ai/code/session_01RwffYaMySLGNKhGyXzDLGG
Raising the regeneration budget to three exposed a latent bug rather than delivering the benefit it was meant to. Regeneration reset the attempt counter but left the stall counters alone, and those counters had been measuring a test that no longer existed. So the replacement test's very first failure landed on a consecutive-failure count already past its threshold, the loop concluded it was stuck again, and it regenerated once more. Four benchmark renders spent their entire budget that way -- consecutive_failures 7, 8, 9, three tests discarded after one attempt each, all inside half a minute -- and then failed exactly as they had before. The stall evidence is now cleared when the loop is handed a genuinely new problem. Cumulative attempt and failure counts survive, because they answer a different question and the benchmark series is indexed on them. At the old budget of one this was invisible: a single regeneration, then the budget was gone and the loop had no choice but to patch. Claude-Session: https://claude.ai/code/session_01RwffYaMySLGNKhGyXzDLGG
Attaching the broker suppresses the spawn-time VEOF, on the promise that something will answer the target's terminal reads. The broker is attached to every conformance execution, but only a test written against codeplain-tty ever makes that promise good -- and almost none are. Those targets got a terminal whose input never ended: anything reading stdin blocked until the 120-second timeout, the conformance loop read the timeout as a defect in the generated code, and patched against it until the render was abandoned. cli-password-manager is the clearest casualty. It scores 15/16 and 16/16 on main across five consecutive stdbmark runs; on this branch it scored 1/16 in fourteen consecutive renders, every one of them dying in the vault_cli module, whose target prompts for a master password. The missing dist/vault.py that all fifteen failing tests cite is downstream of that -- the render never got far enough to publish it. The broker now reports whether it has served a command. Until it has, nothing is driving the terminal and a quiet target is sent end-of-file exactly as a driverless one is; once a test has spoken, deliveries stop for the rest of the execution so an unsolicited EOF cannot land in the middle of its dialogue. The decision is re-made each poll, since a test may drive only late. Claude-Session: https://claude.ai/code/session_01RwffYaMySLGNKhGyXzDLGG
The helper let a generated conformance or acceptance test drive its target through the controlling terminal. Attaching it to an execution suppressed the end-of-file the terminal is otherwise given at spawn, on the basis that something would answer the target's reads instead. That trade was applied to every conformance and acceptance execution, while only a test written against the helper ever made it good. Tests that never used it -- nearly all of them -- ran against a terminal whose input simply never ended, so any target reading standard input blocked until the execution timeout. The fix loop then read that timeout as a defect in the generated code and patched against it until the render was abandoned. An interactive command-line application is the worst case: its first prompt is unanswerable, so no functionality after it can pass. Weighed against that, the capability was used once in roughly thirty renders. Removing it deletes the broker, its wire protocol, the helper executable, the capability descriptor, and the portability audit that existed only to keep the helper out of delivered code -- along with the outstanding Windows transport work none of it can ship without. The pseudoterminal backend stays. Giving the target a real terminal is what makes isatty() true and what interactive programs need to start at all; that is independent of anything driving the terminal from outside. The spawn-time end-of-file becomes unconditional, matching the behaviour the pipe backend has always had, and quiet-target re-delivery now applies uniformly rather than only where no driver was attached. Tests that need to drive an interactive program can do so with an established pseudoterminal library, which also leaves the generated suite runnable outside this renderer. Claude-Session: https://claude.ai/code/session_01RwffYaMySLGNKhGyXzDLGG
Member
Author
|
Breaking this into multiple reviewable PRs. Closing. |
gorandodig
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces pipe-based script execution with terminal-backed subprocess backends (ADR-001, Option A): scripts run with fds 0/1/2 on one terminal in their own contained process tree, output is VT-rendered to clean transcripts, live terminal queries (DSR/CPR/DA) get answered, and teardown covers the whole tree with bounded escalation. Fixes the Node.js startup failure on macOS.
render_machine/_conpty.py), validated by a dedicated windows-2022 lifecycle job plus the windows-latest suite.execute_script().