Handle Claude 2.1.169 TUI completion without assistant transcript events#1
Open
simonrowland wants to merge 1 commit into
Open
Handle Claude 2.1.169 TUI completion without assistant transcript events#1simonrowland wants to merge 1 commit into
simonrowland wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThis PR updates PTY turn handling to use screen snapshots and idle detection for both prompt submission retries and response completion. It also adds marker-based assistant text parsing for final ChangesScreen-idle synchronized turn flow
Sequence DiagramsequenceDiagram
participant Client
participant ManagedSession
participant ClaudePtySession
participant PTYScreen
participant Recognizer
participant Parser
Client->>ManagedSession: prompt_with_permission_handler(prompt)
ManagedSession->>ClaudePtySession: submit_prompt(prompt)
ClaudePtySession->>PTYScreen: write prompt
loop retry while idle echo condition persists
ClaudePtySession->>PTYScreen: screen_snapshot
PTYScreen->>Recognizer: provide screen text
ClaudePtySession->>ClaudePtySession: evaluate echo needle & Idle
alt echo present and Idle
ClaudePtySession->>PTYScreen: write "\r"
else stop retry
ClaudePtySession->>ClaudePtySession: stop retry loop
end
end
loop polling for completion
ManagedSession->>PTYScreen: screen_snapshot
PTYScreen->>Recognizer: recognize_idle
ManagedSession->>ManagedSession: check assistant transcript events & screen_is_idle
alt assistant event and idle
ManagedSession->>ManagedSession: complete polling
else idle marker text without assistant event
ManagedSession->>ManagedSession: clear accumulated transcript events
end
end
ManagedSession->>Parser: assistant_text_from_screen(snapshot)
Parser->>ManagedSession: extracted text or none
ManagedSession->>Client: TurnOutput with extracted or raw screen_text
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/session/manager.rs (2)
416-430: 💤 Low valueExtraction only captures single-line response and may truncate at
(.The function takes text only from the first marker line and strips everything after
(. If the assistant response spans multiple lines or legitimately contains(, the extracted text will be incomplete.Given the PR scope (fallback for short completions like "OK." in Claude 2.1.169), this is acceptable, but consider adding a brief inline comment documenting these constraints for future maintainers.
📝 Suggested documentation
fn assistant_text_from_screen(text: &str) -> Option<String> { + // Extracts short assistant text from TUI screen for fallback when transcript + // events are unavailable. Only captures first marker line; strips " (" suffix + // (e.g., token counts). Not suitable for multi-line responses. let marker = '⏺'; let line = text.lines().find(|line| line.contains(marker))?;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/session/manager.rs` around lines 416 - 430, The helper function assistant_text_from_screen currently only reads the first line containing the marker '⏺' and truncates at the first " (" which can drop multi-line assistant responses or legitimate " (" sequences; add a concise inline comment above assistant_text_from_screen documenting these exact constraints (that it returns the first marker line only and cuts at " (" intentionally for short-fallback behavior), so future maintainers understand this limitation and the rationale within the PR scope (fallback for short completions like "OK.").
319-322: 💤 Low valueRaw screen fallback may include TUI artifacts when marker extraction fails.
When
assistant_text_from_screenreturnsNone, the full screen snapshot becomesscreen_text. If this fallback is triggered (e.g., missing marker due to TUI changes), the client may receive prompt echoes, border characters, or other noise as the assistant response.Consider logging a warning when falling back to raw text to aid debugging TUI compatibility issues.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/session/manager.rs` around lines 319 - 322, The current fallback to raw text sets screen_text to the full snapshot when assistant_text_from_screen(&text) returns None, which can introduce TUI artifacts; update the code around pty.screen_snapshot() so you first call assistant_text_from_screen(&text) and, if it returns None, emit a warning log indicating you fell back to raw screen text (include identifying info like a short prefix or length of text) and then use the raw text for screen_text. Target the expression using pty.screen_snapshot(), assistant_text_from_screen, and the screen_text binding and add the warning log in the None branch to aid debugging of TUI marker extraction failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/session/manager.rs`:
- Around line 416-430: The helper function assistant_text_from_screen currently
only reads the first line containing the marker '⏺' and truncates at the first "
(" which can drop multi-line assistant responses or legitimate " (" sequences;
add a concise inline comment above assistant_text_from_screen documenting these
exact constraints (that it returns the first marker line only and cuts at " ("
intentionally for short-fallback behavior), so future maintainers understand
this limitation and the rationale within the PR scope (fallback for short
completions like "OK.").
- Around line 319-322: The current fallback to raw text sets screen_text to the
full snapshot when assistant_text_from_screen(&text) returns None, which can
introduce TUI artifacts; update the code around pty.screen_snapshot() so you
first call assistant_text_from_screen(&text) and, if it returns None, emit a
warning log indicating you fell back to raw screen text (include identifying
info like a short prefix or length of text) and then use the raw text for
screen_text. Target the expression using pty.screen_snapshot(),
assistant_text_from_screen, and the screen_text binding and add the warning log
in the None branch to aid debugging of TUI marker extraction failures.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9284ee26-9c6b-4ca7-86c0-ee364a6d9deb
📒 Files selected for processing (2)
src/pty/session.rssrc/session/manager.rs
simonrowland
added a commit
to simonrowland/goal-flight
that referenced
this pull request
Jun 9, 2026
… PR #1) — folded into v1.0.4 Vendored upstream TUI-submit patch + opt-in apply script + doctor probe so an installed claude-code-cli-acp@0.1.1 works with Claude Code 2.1.169 until upstream moabualruz/claude-code-cli-acp#1 ships. patches/claude-code-cli-acp-2.1.169-tui-submit.patch (+README) pinned to upstream base c93f4f4. scripts/install_claude_acp_patch.sh: clone pinned base -> git apply -> cargo build -> swap + xattr -c + ad-hoc codesign the platform binary; run-scoped backup + atomic restore on any cp/chmod/xattr/codesign failure; persistent .orig as first-ever backup; idempotent sha skip; version-gated (skips when installed > 0.1.1); cargo-required (clean skip if absent). goalflight_doctor.py claude_acp_stopgap probe warns only for definitively <=0.1.1 unpatched + points to the script (never auto-applies). Tests: installer gating/idempotency/rollback + doctor probe. Two review passes converged (rollback stale-.orig, half-swap restore, doctor version-parse, revert-path). Not auto-run by install.sh. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…t events Claude Code 2.1.169 can visibly complete an interactive TUI turn while the ACP session transcript has no assistant event. The adapter previously required an assistant transcript event before returning, so it timed out (~120s) even after the TUI showed the answer; it also submitted the composer before the newer startup was ready. Changes: submit_prompt gives the composer a startup settle window, waits before Enter, and re-sends Enter (spaced) if the prompt is still echoed on an idle screen; the completion loop keeps transcript-first behavior but, when no assistant transcript event exists and the idle TUI screen shows an assistant marker line, returns the extracted assistant text via the existing screen fallback. The submit settle/retry delays are fixed and conservative; happy to switch to a readiness poll if preferred. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
simonrowland
force-pushed
the
fix-claude-2-1-169-submit
branch
from
136b97e to
35cb5cf
Compare
simonrowland
added a commit
to simonrowland/goal-flight
that referenced
this pull request
Jun 9, 2026
…s, restricted scrape, unit tests) — P3 audit Re-vendors the hardened upstream patch (PR moabualruz/claude-code-cli-acp#1, branch fix-claude-2-1-169-submit): named the Claude Code 2.1.169 TUI timing constants, restricted assistant_text_from_screen to the assistant-line shape (marker at line start + space), added Rust unit tests for prompt_echo_needle + assistant-line extraction. cargo fmt/test/build green; live claude-acp turn re-validated state=complete; public diff codename-clean. From the 2026-06-09 package audit (P3). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/session/manager.rs`:
- Around line 418-436: The fallback parser can return stale or truncated
assistant text; change assistant_text_from_screen to search bottom-up (iterate
lines().rev()) so it finds the most recent CLAUDE_ASSISTANT_LINE_MARKER, and
simplify assistant_text_from_line to not blindly split on " (" — return the full
trimmed text after CLAUDE_ASSISTANT_LINE_MARKER, but if you must remove a
parenthetical do so only when it appears at the very end (e.g., match a trailing
" (…)" and strip that whole suffix), otherwise preserve the entire parenthetical
content; keep using CLAUDE_ASSISTANT_LINE_MARKER to locate the assistant line.
- Around line 257-263: The loop currently treats a failed PTY snapshot
(screen_snapshot().ok()) as non-idle which prevents returning a completed
assistant turn when an assistant terminal event arrives; change the logic to
preserve the snapshot Result (let screen_text = pty.screen_snapshot()) and
compute screen_is_idle only if the Result is Ok (use
screen_text.as_deref().ok().is_some_and(recognizers::recognize_idle)), then
update the break condition to: if has_assistant_event && (screen_is_idle ||
screen_text.is_err()) { break } so that an assistant transcript completion
(has_assistant_event / is_assistant_terminal_event) is not blocked by snapshot
read failures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 184522e9-aba6-4f2e-9293-87eb031a4b39
📒 Files selected for processing (2)
src/pty/session.rssrc/session/manager.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pty/session.rs
Comment on lines
+257
to
+263
| let screen_text = pty.screen_snapshot().ok(); | ||
| let screen_is_idle = screen_text | ||
| .as_deref() | ||
| .is_some_and(recognizers::recognize_idle); | ||
| let has_assistant_event = events.iter().any(is_assistant_terminal_event); | ||
| if has_assistant_event && screen_is_idle { | ||
| break; |
There was a problem hiding this comment.
Keep transcript completion independent of snapshot read failures.
screen_snapshot().ok() turns a fallible PTY read into screen_is_idle = false. If the assistant transcript event has already landed, a snapshot failure now makes this loop wait until timeout instead of returning the completed turn.
Suggested fix
- let screen_text = pty.screen_snapshot().ok();
- let screen_is_idle = screen_text
- .as_deref()
- .is_some_and(recognizers::recognize_idle);
+ let screen_text = pty.screen_snapshot();
+ let screen_is_idle = screen_text
+ .as_deref()
+ .ok()
+ .is_some_and(recognizers::recognize_idle);
let has_assistant_event = events.iter().any(is_assistant_terminal_event);
- if has_assistant_event && screen_is_idle {
+ if has_assistant_event && (screen_is_idle || screen_text.is_err()) {
break;
}
if !has_assistant_event
&& screen_is_idle
&& screen_text
- .as_deref()
+ .as_deref()
+ .ok()
.and_then(assistant_text_from_screen)
.is_some()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let screen_text = pty.screen_snapshot().ok(); | |
| let screen_is_idle = screen_text | |
| .as_deref() | |
| .is_some_and(recognizers::recognize_idle); | |
| let has_assistant_event = events.iter().any(is_assistant_terminal_event); | |
| if has_assistant_event && screen_is_idle { | |
| break; | |
| let screen_text = pty.screen_snapshot(); | |
| let screen_is_idle = screen_text | |
| .as_deref() | |
| .ok() | |
| .is_some_and(recognizers::recognize_idle); | |
| let has_assistant_event = events.iter().any(is_assistant_terminal_event); | |
| if has_assistant_event && (screen_is_idle || screen_text.is_err()) { | |
| break; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/session/manager.rs` around lines 257 - 263, The loop currently treats a
failed PTY snapshot (screen_snapshot().ok()) as non-idle which prevents
returning a completed assistant turn when an assistant terminal event arrives;
change the logic to preserve the snapshot Result (let screen_text =
pty.screen_snapshot()) and compute screen_is_idle only if the Result is Ok (use
screen_text.as_deref().ok().is_some_and(recognizers::recognize_idle)), then
update the break condition to: if has_assistant_event && (screen_is_idle ||
screen_text.is_err()) { break } so that an assistant transcript completion
(has_assistant_event / is_assistant_terminal_event) is not blocked by snapshot
read failures.
Comment on lines
+418
to
+436
| fn assistant_text_from_screen(text: &str) -> Option<String> { | ||
| text.lines().find_map(assistant_text_from_line) | ||
| } | ||
|
|
||
| fn assistant_text_from_line(line: &str) -> Option<String> { | ||
| let after_marker = line | ||
| .trim_start() | ||
| .strip_prefix(CLAUDE_ASSISTANT_LINE_MARKER)? | ||
| .strip_prefix(' ')?; | ||
| let text = after_marker | ||
| .split(" (") | ||
| .next() | ||
| .unwrap_or(after_marker) | ||
| .trim(); | ||
| if text.is_empty() { | ||
| None | ||
| } else { | ||
| Some(text.to_string()) | ||
| } |
There was a problem hiding this comment.
The screen fallback can return stale or truncated assistant text.
This parser scans top-down, so a reused PTY can return an older visible ⏺ line from a previous turn. It also splits on the first " (", which drops legitimate parenthetical content from the current reply.
Suggested fix
fn assistant_text_from_screen(text: &str) -> Option<String> {
- text.lines().find_map(assistant_text_from_line)
+ text.lines().rev().find_map(assistant_text_from_line)
}
fn assistant_text_from_line(line: &str) -> Option<String> {
let after_marker = line
.trim_start()
.strip_prefix(CLAUDE_ASSISTANT_LINE_MARKER)?
.strip_prefix(' ')?;
- let text = after_marker
- .split(" (")
- .next()
- .unwrap_or(after_marker)
- .trim();
+ let text = after_marker.trim();
+ let text = text
+ .strip_suffix(')')
+ .and_then(|text| text.rsplit_once(" ("))
+ .filter(|(_, suffix)| {
+ suffix.ends_with('s')
+ && suffix[..suffix.len() - 1]
+ .chars()
+ .all(|c| c.is_ascii_digit() || c == '.')
+ })
+ .map(|(prefix, _)| prefix)
+ .unwrap_or(text)
+ .trim();
if text.is_empty() {
None
} else {
Some(text.to_string())
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/session/manager.rs` around lines 418 - 436, The fallback parser can
return stale or truncated assistant text; change assistant_text_from_screen to
search bottom-up (iterate lines().rev()) so it finds the most recent
CLAUDE_ASSISTANT_LINE_MARKER, and simplify assistant_text_from_line to not
blindly split on " (" — return the full trimmed text after
CLAUDE_ASSISTANT_LINE_MARKER, but if you must remove a parenthetical do so only
when it appears at the very end (e.g., match a trailing " (…)" and strip that
whole suffix), otherwise preserve the entire parenthetical content; keep using
CLAUDE_ASSISTANT_LINE_MARKER to locate the assistant line.
simonrowland
added a commit
to simonrowland/goal-flight
that referenced
this pull request
Jun 10, 2026
… PR #1) — folded into v1.0.4 Vendored upstream TUI-submit patch + opt-in apply script + doctor probe so an installed claude-code-cli-acp@0.1.1 works with Claude Code 2.1.169 until upstream moabualruz/claude-code-cli-acp#1 ships. patches/claude-code-cli-acp-2.1.169-tui-submit.patch (+README) pinned to upstream base c93f4f4. scripts/install_claude_acp_patch.sh: clone pinned base -> git apply -> cargo build -> swap + xattr -c + ad-hoc codesign the platform binary; run-scoped backup + atomic restore on any cp/chmod/xattr/codesign failure; persistent .orig as first-ever backup; idempotent sha skip; version-gated (skips when installed > 0.1.1); cargo-required (clean skip if absent). goalflight_doctor.py claude_acp_stopgap probe warns only for definitively <=0.1.1 unpatched + points to the script (never auto-applies). Tests: installer gating/idempotency/rollback + doctor probe. Two review passes converged (rollback stale-.orig, half-swap restore, doctor version-parse, revert-path). Not auto-run by install.sh. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
simonrowland
added a commit
to simonrowland/goal-flight
that referenced
this pull request
Jun 10, 2026
…s, restricted scrape, unit tests) — P3 audit Re-vendors the hardened upstream patch (PR moabualruz/claude-code-cli-acp#1, branch fix-claude-2-1-169-submit): named the Claude Code 2.1.169 TUI timing constants, restricted assistant_text_from_screen to the assistant-line shape (marker at line start + space), added Rust unit tests for prompt_echo_needle + assistant-line extraction. cargo fmt/test/build green; live claude-acp turn re-validated state=complete; public diff codename-clean. From the 2026-06-09 package audit (P3). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
Summary
Why
Claude Code 2.1.169 can visibly complete an interactive TUI turn while the ACP session transcript has no assistant event. The adapter previously required an assistant transcript event before returning, so
session/prompttimed out (~120s, JSON-RPC-32603) even after the TUI showed the answer.Reproduction
Environment: macOS,
claude-code-cli-acp@0.1.1, Claude Code v2.1.169 (the version this regressed against; earlier Claude Code builds submitted/completed fine).Drive the adapter over ACP with any client (no extra tooling required):
claude-code-cli-acpwith no argv flags (it only speaks ACP on stdio when launched with no args).initializeauthenticatewithmethodId: \"claude-code-login\"session/new(local cwd, no MCP servers)session/promptwith textReply with: okObserved before this PR:
> OK.renders on screen), then a fresh idle prompt appears.session/promptnever returns -- it fails with-32603 Internal errorafter ~120s:timed out waiting for Claude transcript completion ....~/.claude/projects/.../<session>.jsonlfor that session.Two contributing factors: (a) on 2.1.169 the composer needs a longer startup settle before a carriage return submits the typed prompt; (b) 2.1.169 can finish a turn without writing an assistant transcript event under the ACP session id, so the transcript-only completion check waited until the timeout.
After this PR the same
session/promptreturnsstate=complete,result_text=\"ok\",stop_reason=end_turnin ~15-20s.Changes
src/pty/session.rs--submit_prompt: composer startup settle, wait before Enter, and a spaced Enter retry (1.0/1.5/2.0s) if the prompt is still echoed on an idle screen.src/session/manager.rs-- the completion loop keeps transcript-first behavior, but when no assistant transcript event exists and the idle TUI screen contains an assistant marker line, it returns the extracted assistant text via the existing screen fallback.Notes / open to feedback
The submit settle (2s) and spaced Enter retries are fixed, conservative delays chosen empirically against 2.1.169 -- happy to switch to a readiness poll over fixed sleeps if you prefer. The screen fallback parses the assistant-marker line, which is specific to the current TUI rendering; a more structured completion signal would be more robust if one is available.
Test Plan
cargo fmt --checkcargo teststate=complete,result_text=\"ok\",stop_reason=end_turnSummary by CodeRabbit
Release Notes
Improvements
Tests