Skip to content

terminal ui state detection - #228

Merged
agent-era-ai merged 3 commits into
mainfrom
terminal-ui-state-detection
Apr 26, 2026
Merged

terminal ui state detection#228
agent-era-ai merged 3 commits into
mainfrom
terminal-ui-state-detection

Conversation

@agent-era-ai

Copy link
Copy Markdown
Collaborator
  • tracker: seed item files for terminal-ui-state-detection
  • fix(ai-status): drive detector from real terminal captures

promptium-ai and others added 2 commits April 20, 2026 22:12
Replaces inline synthetic fixtures with a fixture-driven test loader and
patches three detector bugs that real captures surfaced:

- Claude `… (` substring matched transcript lines like `Reading 1 file…
  (ctrl+o to expand)`, false-positiving idle/waiting frames as working.
  Tightened to require the `(\d+s` duration only the live spinner shows.
- Codex `▌ && !⏎ send` heuristic was dead on modern v0.125+ (uses `›` for
  the prompt cursor) and conflated working with waiting on older builds
  (the streaming spinner also lacks the send hint). Replaced with the
  canonical "Press enter to confirm" / "Would you like to run" markers.
- Waiting now takes precedence over working: a permission picker is the
  user-actionable state and the spinner can render alongside it.

Adds a `.claude/skills/capture-ai-states/` skill that drives each
(tool × state) cell automatically (3 tools × 3 states), writes plain-text
captures to `tests/fixtures/ai-states/<tool>/<state>.txt`, and a fixture
loader test that asserts each fixture classifies to its directory name.

Gemini's per-project oauth picker can't be auto-dismissed; the script
errors out clearly and the gemini fixtures are realistic curated
snapshots — see SKILL.md.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Apr 26, 2026

Copy link
Copy Markdown

Code Review — PR #228: terminal-ui-state-detection

Overview

This PR ships three things together: an automated capture skill that scripts each AI CLI into each state, a set of real fixture files replacing the old inline strings, and targeted detector fixes validated against those fixtures. The approach is sound — fixture-driven tests against real captures are strictly better than hand-written strings, and the bugs found (Claude working false-positive, Codex waiting heuristic on modern UI) prove the value.


Detector fixes — src/services/AIToolService.ts

Good

  • The order swap (waiting before working) is correct and the comment explaining why is exactly the right level of detail. The Claude case — permission picker showing above a spinner — is a real edge case that the inline tests never caught.
  • CLAUDE_WORKING_RE = /…\s*\(\d+s/ is a clear improvement over the bare … ( substring. The comment in the module explains the tradeoff concisely.
  • Codex waiting updated to explicit language patterns (press enter to confirm / would you like to run) is cleaner than the cursor+send-hint inference, which was dead code on modern Codex.

Concerns

  1. default: return false in isWorking is silently wrong for new tools. When a fourth AI tool is added, the switch falls to false with no warning — the kanban will show the tool as permanently idle and nobody will know why. At minimum, a comment; ideally an assertion or a log.

    default:
      // If a new tool is added, extend this switch — it will silently return idle otherwise.
      return false;
  2. Dead constant. AI_TOOLS.claude.statusPatterns.working ('… (') is now unused — isWorking takes the regex path for Claude, and this constant is never read. The implementation notes flag it as cleanup, but shipping it as unused is a small debt. Worth removing here while the context is fresh.

  3. CLAUDE_WORKING_RE minute-scale brittleness. If Claude's spinner ever shows (2m 30s for long operations, the regex /…\s*\(\d+s/ won't match. Low probability, but worth a note in the comment.


Tests — tests/unit/ai-tool-detection.test.ts

Good

  • The fixture-loader pattern is much cleaner than 330 lines of inline strings with mocked TmuxService.
  • test.skip for missing fixtures degrades gracefully on a fresh checkout.
  • The trust-folder invariant test is a good explicit regression guard for a previously subtle false-positive.

Concerns

  1. existsSync at module load time. Fixture existence is evaluated at import, before any test runs. A missing fixture silently produces a .skip — CI stays green and nobody notices the gap until the skip count is audited. Consider logging a warning or collecting skip counts so CI at least surfaces them:

    if (fixture === null) {
      console.warn(`[fixture missing] ${tool}/${state}.txt — run capture-ai-states skill`);
      test.skip(...)
    }
  2. No test for the mixed state (working + waiting simultaneously). The main motivation for the order swap is that both markers can appear in the same capture. There's no test that exercises this case directly — a synthetic fixture with both a … (2s line and a ❯ 1. Yes picker would pin the behaviour explicitly.

  3. Gemini fixtures are curated, not live. The implementation notes are honest about this. The limitation should also be noted in tests/fixtures/ai-states/gemini/ (e.g. a README or a comment at the top of each curated file) so a future developer regenerating fixtures knows the gemini ones need manual attention.


Fixture files

The claude and codex captures look genuine and representative. The codex idle fixture correctly includes "Press enter to continue" (trust dialog) which is distinct from "Press enter to confirm" (permission picker) — the patterns don't collide. Good.

The gemini fixtures are accurate snapshots of the real states. The working fixture (⠏ Analyzing test files... esc to cancel) and waiting fixture (⠏ Waiting for user confirmation...) will classify correctly against statusPatterns.working for gemini.


Summary

Area Status
Detector logic ✅ correct, well-justified
Test coverage ✅ good, two gaps noted
Dead code (statusPatterns.working for claude) ⚠️ minor, worth cleaning up
default: return false in isWorking ⚠️ needs a comment
Gemini fixture provenance ⚠️ should be noted in the files

The core work is solid. The two ⚠️ items in the detector are small — a comment and a constant removal — and don't block merging, but the mixed-state test case is worth adding before this lands since it directly documents the motivating bug.

- Drop dead `AI_TOOLS.claude.statusPatterns.working` constant; claude
  detection lives in AIToolService.isWorking via CLAUDE_WORKING_RE.
- Comment the `default` arm of isWorking's switch so adding a new tool
  surfaces the gap loudly.
- Note minute-scale brittleness on CLAUDE_WORKING_RE.
- Add explicit mixed-state test: working spinner + permission picker
  visible together must classify as waiting (motivating bug for the
  waiting-before-working order swap).
- Add README in tests/fixtures/ai-states/gemini/ explaining the
  curated-not-live provenance.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Apr 26, 2026

Copy link
Copy Markdown

Code Review — PR #228: terminal ui state detection

Overview

This PR ships three things together: a capture skill to snapshot real AI terminal states, a fixture tree (tests/fixtures/ai-states/) replacing hand-written inline test strings, and targeted detector fixes in AIToolService.ts to match observed real-world output. The scope is well-defined and the motivation is solid — the old inline fixtures were synthetic and couldn't catch regressions from upstream CLI changes.


What's Good

  • Fixture-driven test architecture is a meaningful improvement over inline strings. The readFileSync-based loader in ai-tool-detection.test.ts automatically skips missing fixtures cleanly and will catch real regressions going forward.
  • Order flip (waiting before working) is the right call. The real waiting.txt capture for Claude shows Reading 1 file… (ctrl+o to expand) above the permission picker — without this order change, that frame would mis-classify as working. The invariant test at the bottom of ai-tool-detection.test.ts proves the fix directly.
  • CLAUDE_WORKING_RE = /…\s*\(\d+s/ cleanly fixes the false positive. The … ( substring matches transcript lines at idle/waiting; anchoring on \d+s selects only the live spinner duration.
  • Codex waiting heuristic was correctly identified as dead code. ▌ && !⏎ send never fires on modern Codex (v0.125+), which uses not . The replacement patterns (press enter to confirm, would you like to run) match the actual captured pane output.
  • The gemini/README.md is good documentation for the known OAuth limitation — future contributors will know why those fixtures are curated and how to regenerate them.

Issues / Concerns

1. CLAUDE_WORKING_RE won't match minute-scale operations

const CLAUDE_WORKING_RE = /\s*\(\d+s/;

The comment itself flags this: if Claude ever shows (2m 30s) for long-running operations, detection silently degrades to idle. This is a known edge case, but since it's a regression path rather than a new gap, consider a slightly broader pattern now rather than a follow-up:

const CLAUDE_WORKING_RE = /\s*\(\d+(\s*m\s*\d+s|s)/;

That matches (2s, (2m 30s, and (12s while still excluding (ctrl+o to expand).

2. isWorking default case is silent

private isWorking(text: string, tool: AITool): boolean {
  switch (tool) {
    case 'claude': return CLAUDE_WORKING_RE.test(text);
    case 'codex':
    case 'gemini': return text.toLowerCase().includes(...);
    default:
      // comment: "falling through reports permanent idle with no warning"
      return false;
  }
}

The comment is accurate but only visible in code review. Since AITool is a string union (not an enum), TypeScript won't flag an unhandled case at compile time. Consider a never-assertion to make exhaustiveness a build error:

default: {
  const _exhaustive: never = tool;  // compile error when a new tool is added
  return false;
}

This would have caught the gap in PR #176 (Auggie) before it merged.

3. statusPatterns.working for Claude is now unused dead config

The diff removes working: '… (' from AI_TOOLS.claude.statusPatterns, which is correct — Claude now uses the regex path. However, the implementation notes say "left as-is for now." The diff itself does remove it, so this is just a note that the notes are slightly inconsistent with the code. Confirm the field is intentionally removed (the diff says yes).

4. idle_prompt patterns are declared but never consumed

From the discovery notes:

idle_prompt hints (the latter are declared but not consumed by detection)

AI_TOOLS.claude.statusPatterns.idle_prompt = ['│ >', '│'] is still present. Either remove it or consume it in detection. Dead config causes confusion when someone tries to add a new tool and follows the claude shape.

5. Gemini curated fixtures don't cover the idle classification path fully

The curated gemini/idle.txt contains the Gemini banner and the > Type your message prompt box. The working Gemini detection relies on waiting for user substring match. It's worth verifying the curated idle fixture does not contain that string (it doesn't appear to) but a brief comment in the README confirming this would make it clearer.


Test Coverage

The fixture-driven suite covers 9 cells (3 tools × 3 states) plus two invariant tests. Missing cases worth adding:

  • Codex idle vs. working disambiguation: the old ⏎ send heuristic was wrong; the new working detection uses ◦ Working (0s • esc to interrupt) (visible in codex/working.txt). A synthetic unit test asserting the idle fixture classifies as idle (not working) would protect against future Codex UI changes.
  • Scrollback false positive for Claude: the discovery notes call out "a transcript with a stale ❯ 1. … still visible after the tool went idle." There is no test for this. CLAUDE_WAITING_RE requires ❯\s+\d+\.\s+\w+ on the same line, which should prevent the false positive — but a test case would confirm it.

Summary

Solid PR overall. The detection fixes are backed by real evidence, the test architecture is clearly better, and the key edge cases (trust-folder dialog, mixed working+waiting frame) are explicitly tested. The main follow-up items are:

  1. Broaden CLAUDE_WORKING_RE to handle minute-scale durations (minor — current gap, not a regression).
  2. Add a never-assertion in isWorking default to make new-tool omissions a compile error.
  3. Remove or start consuming idle_prompt patterns to avoid dead config.

@agent-era-ai
agent-era-ai merged commit 33c4a71 into main Apr 26, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants