Skip to content

test(openclaw): protect post-idle TUI interactions - #7523

Closed
senthilr-nv wants to merge 4 commits into
mainfrom
codex/fix-tui-idle-order
Closed

test(openclaw): protect post-idle TUI interactions#7523
senthilr-nv wants to merge 4 commits into
mainfrom
codex/fix-tui-idle-order

Conversation

@senthilr-nv

@senthilr-nv senthilr-nv commented Jul 25, 2026

Copy link
Copy Markdown
Collaborator

Summary

The OpenClaw TUI can render its idle status before it paints the correlated reply or slash-command output. The #6194 live regression guard now accepts either order while requiring both signals inside one bounded wait.

Related Issue

Refs #6194

Changes

  • Wait for reply and idle signals as an unordered pair after a chat request.
  • Wait for status output and idle signals as an unordered pair after /nemoclaw status.
  • Keep both signals inside one deadline with exp_continue -continue_timer.
  • Preserve separate timeout and EOF exit codes for whichever signal is missing.
  • Execute contract tests for both signal orders, all four failure exits, and repeated redraws.

The expect_pair_or_exit procedure serves the accepted #6194 chat and status journey because terminal redraws do not guarantee text order. Sequential waits discard a signal that renders before the preceding match. A direct ordered wait cannot distinguish a working interaction from a missing signal.

This PR does not change OpenClaw product code or close #6194. It protects the accepted user-facing journey through the existing live E2E boundary.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification: This change strengthens internal E2E coverage and does not change supported product behavior.
  • Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
  • Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification:
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Documentation Writer Review

DGX Station Hardware Evidence

  • Tested on DGX Station
  • Tested commit:
  • Station profile/scenario:
  • Result:
  • Supporting evidence:

Verification

  • PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: /opt/homebrew/bin/node node_modules/vitest/vitest.mjs run --project e2e-support test/e2e/support/issue-6194-tui-post-idle-contract.test.ts passed 18 tests with real Expect.
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result:
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Senthil Ravichandran senthilr@nvidia.com

Summary by CodeRabbit

  • Bug Fixes

    • Improved terminal interaction handling when chat responses and status updates appear in either order.
    • Preserved distinct timeout and end-of-stream outcomes when expected terminal signals are missing.
  • Tests

    • Added coverage for paired terminal signals, including reversed ordering, repeated redraws, timeouts, and early termination.

Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
@senthilr-nv senthilr-nv self-assigned this Jul 25, 2026
@coderabbitai

coderabbitai Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: bfdb4b25-379e-4992-8329-8b575e9f7ad1

📥 Commits

Reviewing files that changed from the base of the PR and between 6a47eb5 and f4e1dc7.

📒 Files selected for processing (1)
  • test/e2e/support/issue-6194-tui-post-idle-contract.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/e2e/support/issue-6194-tui-post-idle-contract.test.ts

📝 Walkthrough

Walkthrough

The TUI Expect script adds paired matching for chat and status output followed by connected-idle states. Support tests validate the generated helper, control flow, exit codes, signal ordering, and redraw timeout behavior.

Changes

TUI Expect Flow

Layer / File(s) Summary
Paired Expect helper
test/e2e/live/issue-6194-tui-expect.ts
Adds expect_pair_or_exit to track two regex matches and handle timeout or EOF with pattern-specific exits.
Paired transitions and validation
test/e2e/live/issue-6194-tui-expect.ts, test/e2e/support/issue-6194-tui-post-idle-contract.test.ts
Pairs chat and status output with connected-idle matches and tests generated structure, signal ordering, exit codes, and redraw handling.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly reflects the main change: protecting post-idle TUI interactions in OpenClaw tests.
Linked Issues check ✅ Passed The new paired Expect flow handles post-idle chat and /nemoclaw status ordering with bounded waits and distinct timeout/EOF exits.
Out of Scope Changes check ✅ Passed The changes stay within the E2E Expect harness and contract tests, with no unrelated product or tooling edits.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-tui-idle-order

Comment @coderabbitai help to get the list of available commands.

@github-code-quality

github-code-quality Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 01d38fd in the codex/fix-tui-idle-o... branch remains at 96%, unchanged from commit 6e8d21a in the main branch.

TypeScript / code-coverage/cli

The overall coverage in commit 01d38fd in the codex/fix-tui-idle-o... branch remains at 80%, unchanged from commit 6e8d21a in the main branch.

Show a code coverage summary of the most impacted files.
File main 6e8d21a codex/fix-tui-idle-o... 01d38fd +/-
src/lib/onboard...ndbox-create.ts 83% 33% -50%
src/lib/onboard...-create-plan.ts 88% 75% -13%
src/lib/onboard...ndbox-create.ts 91% 83% -8%
src/lib/onboard...der-metadata.ts 100% 100% 0%
src/lib/sandbox...rce-identity.ts 87% 87% 0%
src/lib/shields/index.ts 71% 72% +1%

Updated July 25, 2026 14:11 UTC

@senthilr-nv senthilr-nv added chore Build, CI, dependency, or tooling maintenance area: e2e End-to-end tests, nightly failures, or validation infrastructure integration: openclaw OpenClaw integration behavior labels Jul 25, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@test/e2e/live/issue-6194-tui-expect.ts`:
- Around line 58-82: Update expect_pair_or_exit in
test/e2e/live/issue-6194-tui-expect.ts:58-82 to enforce one overall deadline
across both signal orders, while preserving the distinct timeout and EOF exits
for whichever pattern remains unseen. In
test/e2e/support/issue-6194-tui-post-idle-contract.test.ts:66-79, replace exact
source-shape assertions with behavioral checks covering both arrival orders and
both timeout/EOF exit paths.

In `@test/e2e/support/issue-6194-tui-post-idle-contract.test.ts`:
- Around line 66-79: Replace the source-text assertions in the TUI contract test
with behavioral coverage of the expect script: verify both reply-then-idle and
idle-then-reply arrival orders, and verify distinct exit behavior when either
signal is missing, including timeout and EOF paths. Keep assertions focused on
observable outcomes rather than the internal while-loop or variable
implementation.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 9f84b6c9-7c26-4935-bbed-a8cc54e40f56

📥 Commits

Reviewing files that changed from the base of the PR and between 64df93a and 58bd931.

📒 Files selected for processing (2)
  • test/e2e/live/issue-6194-tui-expect.ts
  • test/e2e/support/issue-6194-tui-post-idle-contract.test.ts

Comment thread test/e2e/live/issue-6194-tui-expect.ts Outdated
Comment thread test/e2e/support/issue-6194-tui-post-idle-contract.test.ts
@github-actions

github-actions Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Informational

Advisor assessment: Informational / low confidence
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions
Status: PR review advisor failed: PR review advisor SDK execution failed: session: omitted required tool result(s): pr_review_correctness_state_context; correctness-state-analysis omitted required analysis; turn: correctness-state-analysis: omitted required tool result(s): pr_review_correctness_state_context; correctness-state-analysis omitted required analysis

Model lanes

  • GPT-5.6 Terra (primary): Failed
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: cloud-onboard, credential-sanitization, security-posture

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
@senthilr-nv senthilr-nv changed the title test(e2e): accept reordered TUI idle output test(openclaw): protect post-idle TUI interactions Jul 25, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@test/e2e/support/issue-6194-tui-post-idle-contract.test.ts`:
- Around line 101-116: Add killSignal: "SIGKILL" to both spawnSync calls in the
paired Expect test, including the version probe and the expect -c execution.
Give the version probe a positive timeout, while preserving the existing timeout
on the expect script and ensuring both calls use the hard-kill behavior.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 268278ca-1458-4fee-833a-5899880e2d64

📥 Commits

Reviewing files that changed from the base of the PR and between 58bd931 and 6a47eb5.

📒 Files selected for processing (2)
  • test/e2e/live/issue-6194-tui-expect.ts
  • test/e2e/support/issue-6194-tui-post-idle-contract.test.ts

Comment thread test/e2e/support/issue-6194-tui-post-idle-contract.test.ts Outdated
@copy-pr-bot

copy-pr-bot Bot commented Jul 25, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@senthilr-nv

Copy link
Copy Markdown
Collaborator Author

Superseded by #7526. The replacement is based on current main and contains the same reviewed change as one GitHub-verified commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: e2e End-to-end tests, nightly failures, or validation infrastructure chore Build, CI, dependency, or tooling maintenance integration: openclaw OpenClaw integration behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[All Platforms][Agent&Skills] openclaw tui interactions timeout after connected idle on v0.0.72

1 participant