fix(ding): classify a used Claude pane instead of deferring forever - #27
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e1e64b837a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Observed delivery to a Claude pane never left `Ambiguous` once the pane had been used at all, so every DING deferred indefinitely and messages only ever reached the inbox. Three independent signals in the classifier are read as "unproven" when they are really just "not a freshly started pane": 1. The harness was detected via the `Claude Code v…` startup banner, but `pty peek` returns the viewport only, so that banner scrolls away after the first screenful. Detect the ruled `❯` composer instead, which is durable, and keep an unlocatable Claude pane `Ambiguous` rather than letting it fall through to the Codex heuristics. 2. The composer row of an empty composer is `❯` + U+00A0 and nothing else. U+00A0 is whitespace, so trimming before stripping the prompt erased the prompt and the row failed to parse. Strip the prompt first, then trim. An empty composer is also positive-empty proof in its own right: a human draft cannot be empty, which makes it a stronger signal than the rotating `Try "…"` placeholder that only an unused pane shows. 3. The idle footer required `shift+tab to cycle`. That hint is derived from a keybinding lookup and rendered under two further runtime conditions, so it is absent on real idle panes and changes text when the binding is remapped. The permission-mode indicator next to it ships unconditionally with the composer chrome, so gate on that and let the hint corroborate. Fail-closed behaviour is unchanged: a human draft is still `Changed`, an active turn or modal is still `ExactBlocked` via `interaction_blocked`, and an unrecognised screen is still `Ambiguous`. Only wrong negatives are removed. Extends `maintained_composer_classifiers_require_exact_idle_state`, the test INVARIANTS.md names for this guarantee, with a used-pane screen covering all three signals plus the two fail-closed cases on that same shape. Fixes #26
schickling-assistant
force-pushed
the
fix/claude-composer-idle-detection
branch
from
July 28, 2026 13:22
e1e64b8 to
3ec59fa
Compare
Routing preferred any Claude-shaped text over the ANSI-detected Codex composer, so a Codex pane whose scrollback holds a captured Claude screen was classified from the capture. Capturing and pasting pane text is routine, so that shape is not exotic. This is a wrong positive rather than a wrong negative, which makes it worse than the deferral bugs around it: the paste and the bare Return always go to the pane's real bottom composer whichever text was read. A transcript row that looks empty lets the paste land in a human's live Codex draft, and one holding the exact notice can satisfy both adjacent observations and submit that draft. A pane has exactly one live composer and it is the lowest one on screen, so compare positions instead of preferring a harness. Scrollback is above the live composer by construction, which makes this one comparison rather than a per-pair special case. The Codex locator reports a byte offset into the raw ANSI screen while the Claude locator works in stripped line indices; every Codex marker starts at an `\x1b[` boundary, so stripping the prefix is faithful and its newline count is that composer's row. Detection also stops consulting the `Claude Code v…` startup banner entirely. An unlocatable composer now reaches the same `Ambiguous` it reached before via the banner branch, so that check no longer carries any routing weight. Reported by automated review on #27. agent-session-id: e5217740-eef6-48eb-a794-3e5e11939e4d agent-tool: Claude Code agent-tool-version: 2.1.220 agent-model: claude-opus-5 agent-runtime-profile: /nix/store/i8y8b542cyqi385ywcjw5fvsq24f75v4-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/i81qxhzlrzcxrrdwpp6i8hagka2gby8y-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty
Claude 2.1.220 ships no interrupt hint with its working status line, so none of
the strings `interaction_blocked` matches appear while a turn is running. On a
170-column capture the line renders as `✻ Frolicking… (3m 35s · ↓ 6.9k tokens)`
in 38 cells, so this is not width truncation — the hint is simply not there.
Until now that was harmless: an active pane also has an empty composer, and an
empty composer failed to parse, so delivery returned `Ambiguous` and deferred.
Fixing that parse removes the accidental guard and leaves a working pane
reading as positively idle, which would paste a notice and press Return in the
middle of someone else's turn.
Gate on the status line instead. Two observations shape the predicate, both
sampled from real panes rather than assumed:
- The leading glyph animates — `·`, `✶`, `✻`, and `✽` were all observed — so
it cannot be matched literally.
- The elapsed timer is absent from some active frames (`✽ Schlepping…`), so
requiring it would fail open on exactly the frames that matter.
What separates the states is the ellipsis directly after a single gerund. An
active turn is `<glyph> <Gerund>…`; a finished one is `<glyph> <Past> for <n>s`.
That distinction is load-bearing in the other direction too: the finished line
sits above every genuinely idle composer, so matching it would stop delivery
altogether rather than merely being conservative. The added test pins both
directions across four real active frames and four real finished ones.
The predicate lives in the shared `interaction_blocked` for now, so a Codex
pane whose scrollback shows a Claude spinner also blocks. That is the
fail-closed direction, and the per-harness split moves it to the Claude adapter.
agent-session-id: e5217740-eef6-48eb-a794-3e5e11939e4d
agent-tool: Claude Code
agent-tool-version: 2.1.220
agent-model: claude-opus-5
agent-runtime-profile: /nix/store/i8y8b542cyqi385ywcjw5fvsq24f75v4-coding-agent-runtime-profile/share/coding-agents/profile.json
agent-skills-manifest: /nix/store/i81qxhzlrzcxrrdwpp6i8hagka2gby8y-agent-skills-corpus/share/agent-skills/manifest.json
tooling-profile: dotfiles@unknown-dirty
Pure restructure: no behaviour change, no test renamed, no assertion altered.
`ding.rs` had grown to hold transport, delivery state, and both harnesses'
screen grammars in one file, with routing expressed as an ordered preference
between them. The preceding commit replaced that preference with a positional
rule — classify the lowest composer, because scrollback is above the live one
by construction. This gives that rule a structural home.
src/ding/
mod.rs transport, pending notices, DingConfig, run_ding, serve
composer.rs ComposerState, the router, strip_ansi, soft-wrap candidates
harness/
mod.rs trait Harness + Located + the registry
claude.rs locate / classify / idle footer / blocked
codex.rs locate / classify / idle footer / blocked
The router is now `max_by_key(row)` over every registered harness, so the
positional rule is one comparison rather than a special case per harness pair,
and a third harness cannot reintroduce an ordering. Claude is registered last
so an exact row tie still resolves to Claude, as the explicit comparison did.
Active-turn and modal detection moves to the harnesses, since those shapes are
harness-specific TUI chrome rather than a shared contract. The shared predicate
was already Claude-flavoured — `Create a plan?`, `Our systems are thinking a
bit more`, and `Retry with a faster model` were applied to Codex panes too. It
is duplicated verbatim into both adapters rather than narrowed, so this commit
changes nothing; narrowing the Codex side is separate work. Only the
choice-menu heuristic, which is genuinely cross-harness, stays in `composer`.
INVARIANTS.md names tests by file path, so its seventeen `src/ding.rs::`
references are repointed to `src/ding/mod.rs::`. Every test stayed in that
module: the screen fixtures are shared between the composer assertions and the
transport ones, so splitting them would have meant duplicating fixtures or
exporting them across modules, and neither is worth it for a move.
Verified rather than assumed:
- `cargo test --lib -- --list` is byte-identical before and after (142 tests).
- All 27 `src/<file>.rs::<test>` references in INVARIANTS.md resolve, checked
both by grepping the named path for `fn <name>` and by running each one
with `cargo test <full::path> -- --exact` (27 resolved, 27 passed).
- The two real captures behind the preceding commits classify identically.
agent-session-id: e5217740-eef6-48eb-a794-3e5e11939e4d
agent-tool: Claude Code
agent-tool-version: 2.1.220
agent-model: claude-opus-5
agent-runtime-profile: /nix/store/i8y8b542cyqi385ywcjw5fvsq24f75v4-coding-agent-runtime-profile/share/coding-agents/profile.json
agent-skills-manifest: /nix/store/i81qxhzlrzcxrrdwpp6i8hagka2gby8y-agent-skills-corpus/share/agent-skills/manifest.json
tooling-profile: dotfiles@unknown-dirty
The extraction removed whole items but left their leading blank separators behind, so the previous commit's diff carries a fifteen-line whitespace gap in `mod.rs` and smaller ones in the new modules. `cargo fmt` is deliberately not gated in this repo — and the tree is not rustfmt-clean, so running it broadly would reformat hand-written code — hence collapsing only the runs the split introduced, between top-level items. Whitespace only. The test list stays byte-identical at 142 and every INVARIANTS.md reference still resolves. agent-session-id: e5217740-eef6-48eb-a794-3e5e11939e4d agent-tool: Claude Code agent-tool-version: 2.1.220 agent-model: claude-opus-5 agent-runtime-profile: /nix/store/i8y8b542cyqi385ywcjw5fvsq24f75v4-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/i81qxhzlrzcxrrdwpp6i8hagka2gby8y-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty
The two locators do not natively work in the same units. Codex is matched with `rfind` over the raw screen and reports a byte offset, inflated by every escape sequence above it; Claude is matched over stripped lines and reports a row. Comparing those directly picks Codex almost always, since an offset dwarfs a row — including when the live composer is Claude's and the Codex match is stale scrollback, which is the dangerous direction. The dispatch already normalizes, so this changes no production code. It pins the property, which was previously only argued in a comment. On the added screen, measured: the Codex composer sits at byte offset 560 but row 10, while the live Claude composer is row 20. Reverting the locator to the raw offset turns the assertion from `EmptySafe` into `Changed` — classifying a pasted draft instead of the real composer. The normalization counts newlines in the *stripped* prefix, which is only faithful if stripping preserves them. Verified rather than assumed, and it does not hold universally: the CSI scanner runs until a byte in `0x40..=0x7e` and `\n` is `0x0a`, so an unterminated CSI eats newlines, and an unterminated OSC consumes to end of input. Well-formed sequences preserve the count exactly. Both directions are now asserted, so a future change to `strip_ansi` cannot silently shift every row. agent-session-id: e5217740-eef6-48eb-a794-3e5e11939e4d agent-tool: Claude Code agent-tool-version: 2.1.220 agent-model: claude-opus-5 agent-runtime-profile: /nix/store/i8y8b542cyqi385ywcjw5fvsq24f75v4-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/i81qxhzlrzcxrrdwpp6i8hagka2gby8y-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty
The split left the Codex predicate calling into the Claude module for the progress-line shape, which is the coupling the harness boundary exists to prevent: one harness's rendering reaching directly into another's safety gate, and a narrowing edit to one silently changing the other. Duplicate the shape into the Codex adapter instead, so narrowing it later is a local edit. That duplication is the accepted cost recorded in the dispatch decision, and `claude_turn_in_flight` goes back to private. Behaviour is unchanged: the predicate is the same shape over the same input, and the two harnesses still block on the same set they did before the split. agent-session-id: e5217740-eef6-48eb-a794-3e5e11939e4d agent-tool: Claude Code agent-tool-version: 2.1.220 agent-model: claude-opus-5 agent-runtime-profile: /nix/store/i8y8b542cyqi385ywcjw5fvsq24f75v4-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/i81qxhzlrzcxrrdwpp6i8hagka2gby8y-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty
…ness specs Adds `docs/vrs/01-ding/` — subsystem requirements, the shared mechanism spec, and one spec per maintained harness — plus the decision record behind positional dispatch and harness-owned vocabulary. The requirements decompose the ratified `R05` floor into the obligations DING actually has to meet, written as preconditions for pressing Return rather than as a delivery guarantee, since this is the one subsystem that writes into a surface a human may also be typing into. Harnesses get spec only: the fail-closed guarantees are one contract owned at the DING level, and a harness adapter is mechanism rather than constitution. Two things here are new conventions for this repo and are flagged for the owner rather than assumed: a `.decisions/` directory, which did not previously exist, and a subsystem-level `requirements.md`. `docs/vrs/spec.md` reserves changes to vision and requirements for the owner's explicit approval; the reading taken here is that this covers the ratified root documents rather than forbidding a subsystem decomposition beneath them, but that is the owner's call to make and both are cheap to move or drop. Written against the code as it stands. Where the prose and the landed code disagreed, the prose was corrected — those corrections are listed in the pull request rather than left silent, because a doc quietly reshaped to match code stops being independent evidence about it. agent-session-id: e5217740-eef6-48eb-a794-3e5e11939e4d agent-tool: Claude Code agent-tool-version: 2.1.220 agent-model: claude-opus-5 agent-runtime-profile: /nix/store/i8y8b542cyqi385ywcjw5fvsq24f75v4-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/i81qxhzlrzcxrrdwpp6i8hagka2gby8y-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty
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.
Fixes #26.
Delivery to a Claude pane never left
Ambiguous, so every DING deferred indefinitely and messages only ever reached the inbox. Fixing that turned out to expose two further wrong-positives in the same routing path, which are fixed here rather than shipped.Five commits: three behaviour, then a pure restructure and its whitespace cleanup.
3ec59fa37c446704b600cd3f5fd72cd06351. Harness detection depended on a banner that is often absent
classify_composerrouted to the Claude branch onplain.contains("Claude Code v").pty peekreturns the viewport only — there is an explicit--fullflag for scrollback — so the check is unreliable, and when it fails the function returnsAmbiguouswithout ever looking at the composer.It is worse than "the banner scrolls away". On a detached 80x24 pane the string
Claude Code vdoes not appear at all: the boxed welcome panel renders the wordmark and a truncated model line instead. Measured on a live capture at boot,plain.contains("Claude Code v")wasfalse. On a 170-column capture of a different pane the same string is present. Either way it cannot gate detection, so detection now uses the ruled❯composer, which is durable, and the banner check is gone entirely.2. An empty composer row failed to parse
The composer row of an empty composer is
❯+ U+00A0 and nothing else. U+00A0 is Rust-whitespace, sorows.first()?.trim_end()erased the prompt and bothstrip_prefixcalls failed, returningNone. Strip the prompt, then trim.placeholderalso needed the empty case: Claude shows the rotatingTry "…"placeholder only on an unused pane, and an empty composer is the stronger positive-empty proof, since a human draft cannot be empty.3. Transcript text could outrank the live composer
Raised by automated review on this PR. Routing preferred any Claude-shaped text over the ANSI-detected Codex composer, so a Codex pane whose scrollback held a captured Claude screen was classified from the capture. Capturing and pasting pane text is routine.
This is a wrong positive, which makes it worse than the deferral bugs: the paste and the bare Return go to the pane's real bottom composer whichever text was read. A transcript row that looks empty lets the paste land in a human's live Codex draft; one holding the exact notice can satisfy both adjacent observations and submit that draft.
A pane has exactly one live composer and it is the lowest on screen, so compare positions instead of preferring a harness — scrollback is above the live composer by construction. The Codex locator reports a raw ANSI byte offset while the Claude locator works in stripped line indices; every Codex marker starts at an
\x1b[boundary, so stripping the prefix is faithful and its newline count is that composer's row.4. An in-flight turn read as idle
Claude 2.1.220 ships no interrupt hint with its working status line. None of the nine strings
interaction_blockedmatched appear during a turn — across roughly seventy real captures spanning several live turns, every one scored zero. It is not width truncation: on a 170-column pane the line✻ Frolicking… (3m 35s · ↓ 6.9k tokens)renders in 38 cells.Until now that was harmless, because an active pane also has an empty composer and an empty composer failed to parse, so delivery deferred. Fixing defect 2 removes that accidental guard and leaves a working pane reading as positively idle — DING would paste and press Return in the middle of someone else's turn.
The predicate is derived from captures rather than assumed, and two observations shaped it:
·,✶,✻,✽all observed — so it cannot be matched literally.✽ Schlepping…), so requiring it would fail open on exactly the frames that matter.What separates the states is the ellipsis directly after a single gerund. Active is
<glyph> <Gerund>…; finished is<glyph> <Past> for <n>s. That distinction is load-bearing in both directions: the finished line sits above every idle composer, so matching it too would have stopped delivery altogether rather than being merely conservative.Restructure
d3f5fd7is a pure move — no behaviour change, no test renamed, no assertion altered.The router is now
max_by_key(row)over every registered harness, so the positional rule is one comparison rather than a special case per harness pair. Claude is registered last so an exact row tie still resolves to Claude, as the explicit comparison did.Active-turn and modal detection moves to the harnesses, since those shapes are harness-specific chrome. The shared predicate was already Claude-flavoured —
Create a plan?,Our systems are thinking a bit more, andRetry with a faster modelwere applied to Codex panes too — so it is duplicated verbatim into both adapters rather than narrowed, keeping the move behaviour-free. Narrowing the Codex side is separate work. Only the choice-menu heuristic, genuinely cross-harness, stays incomposer.INVARIANTS.md names tests by file path, so its seventeen
src/ding.rs::references are repointed tosrc/ding/mod.rs::. All tests stayed in that module: the screen fixtures are shared between the composer assertions and the transport ones.Fail-closed
A human draft is still
Changed, an active turn or modal stillExactBlockedvia the harness's blocked predicate, an unrecognised screen stillAmbiguous. Transcript text never outranks the live composer, and an in-flight turn is now genuinely blocked rather than accidentally unparseable.One known limitation, pinned by a test rather than left implicit:
permissions onmatches only the bypass footer, so accept-edits and auto panes are never positively idle. With the empty-composer case added they returnChangedwheremainreturnedAmbiguous; at theobserved_pokeentry point both areDeferred, so entry behaviour is unchanged.Testing
cargo test --lib --bins: 142 passed, 0 failed.nix flake check --print-build-logs: exit 0.EmptySafe; without the spinner predicate an active turn yieldsEmptySafe.cargo test --lib -- --listis byte-identical before and after (142 tests); all 27src/<file>.rs::<test>references in INVARIANTS.md resolve, checked both by grepping the named path forfn <name>and by running each withcargo test <full::path> -- --exact.bypassPermissions, under a throwaway catalog and pty root: a[DING] new st2 message: …notice was typed into the composer and submitted, and the agent read, replied to, and archived the message. Re-run at2cd0635— the pushed head, after both the routing change and the module split — by polling the pane until the❯ [DING] …line was caught in the composer, since the harness split rewrote the router that decides delivery.origin/mainlocated_bottom_claude_composerreturnsNoneandclassify_composerreturnsAmbiguous; on this branch,SomeandEmptySafe. A mid-turn capture returnsChangedhere andAmbiguousonmain— for the wrong reason, via the parse bug.bypassPermissionsonly.No captured pane text is committed; all fixtures are hand-written synthetic screens.
Posted on behalf of @schickling
agent_nameagent_session_idagent_toolagent_tool_versionagent_runtimeagent_modelruntime_profileskills_manifestworktreemachinetooling_profile