fix: capture output from quick-completing commands in screen isolation (issue #96)#97
Merged
fix: capture output from quick-completing commands in screen isolation (issue #96)#97
Conversation
Adding .gitkeep for PR creation (default mode). This file will be removed when the task is complete. Issue: #96
…n (issue #96) Add `logfile flush 0` via a temporary screenrc to force GNU Screen to flush its log buffer immediately after every write, preventing output loss for commands like `agent --version` that complete faster than screen's default 10-second log flush interval. Also add a 50ms retry for the tee fallback path (screen < 4.5.1) to handle the TOCTOU race where the log file may be empty when first read immediately after session completion. Changes: - js/src/lib/isolation.js: create temp screenrc with `logfile flush 0` for native logging path; add readAndDisplayOutput() with retry logic; clean up both log file and screenrc on completion - js/test/isolation.test.js: add regression test for version-flag commands (issue #96) - rust/src/lib/isolation.rs: equivalent fix for Rust implementation - rust/src/lib/isolation_tests.rs: add Rust regression test for version-flag commands (issue #96) - docs/case-studies/issue-96/: deep-dive case study with timeline, root cause analysis, and solutions evaluated - js/.changeset/fix-screen-log-flush-quick-commands.md: patch changeset - experiments/: scripts used to diagnose and verify the fix Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
agent --version does not work in screen isolationThis reverts commit 9adb05b.
Member
Author
🤖 Solution Draft LogThis log file contains the complete execution trace of the AI solution draft process. 💰 Cost estimation:
Now working session is ended, feel free to review and add any feedback on the solution draft. |
Member
Author
🔄 Auto-restart triggered (attempt 1)Reason: CI failures detected Starting new session to address the issues. Auto-restart-until-mergeable mode is active. Will continue until PR becomes mergeable. |
…-line limit Both isolation.js (1016 lines) and isolation.rs (1048 lines) exceeded the 1000-line file size limit enforced by CI. - Extract screen version detection and log capture logic from isolation.js into new js/src/lib/screen-isolation.js module - Extract screen-related functions from isolation.rs into new rust/src/lib/isolation_screen.rs module - Add rust/changelog.d/97.md changelog fragment for issue #96 fix After refactoring: - isolation.js: 758 lines (was 1016) - isolation.rs: 851 lines (was 1048) - screen-isolation.js: 309 lines (new) - isolation_screen.rs: 215 lines (new) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Member
Author
🔄 Auto-restart-until-mergeable Log (iteration 1)This log file contains the complete execution trace of the AI solution draft process. 💰 Cost estimation:
Now working session is ended, feel free to review and add any feedback on the solution draft. |
Member
Author
✅ Ready to mergeThis pull request is now ready to be merged:
Monitored by hive-mind with --auto-restart-until-mergeable flag |
7 tasks
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
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.
Summary
Fixes #96:
agent --version(and other quick-completing commands) run through screen isolation produced no output, even though the command succeeded with exit code 0.Root Cause
GNU Screen's
-Llog capture uses an internalFILE*buffer with a 10-second default flush interval (log_flush = 10). For commands that complete in milliseconds (likeagent --version), the buffer is never flushed to the log file before the session terminates — so the output is silently lost.From GNU Screen source (
src/ansi.c):Fix
Native logging path (screen ≥ 4.5.1): Create a temporary screenrc with
logfile flush 0and pass it viascreen -c. This setslog_flush = 0, forcing screen to flush after every write — eliminating the 10-second delay.Tee fallback path (screen < 4.5.1, e.g. macOS bundled 4.0.3): Add a 50ms retry when the log file appears empty right after session completion, handling the TOCTOU race where
teemay not have flushed to disk beforescreen -lsshows the session as gone.Before (output missing)
After (output captured)
Changes
js/src/lib/isolation.jslogfile flush 0for native logging; add retry for empty log; clean up both temp filesjs/test/isolation.test.jsnode --versionoutput must be captured in screen isolation (issue #96)rust/src/lib/isolation.rsrust/src/lib/isolation_tests.rsdocs/case-studies/issue-96/js/.changeset/fix-screen-log-flush-quick-commands.mdexperiments/Test Plan
67 pass, 0 fail)should capture output from version-flag commands (issue #96)passes — capturesv20.20.1fromnode --versioncargo check,cargo test,cargo fmt --check,cargo clippy)node js/src/bin/cli.js --isolated screen -- agent --versionshows version output🤖 Generated with Claude Code