fix(e2e): preserve Deep Agents TUI outcome - #5912
Conversation
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a ChangesTUI Startup Check: Sanitized Capture Excerpt on Failure
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in the Show a code coverage summary of the most covered files.
TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most covered files.
Updated |
PR Review Advisor (Nemotron Ultra) — No blocking findingsMerge posture: No blocking advisor findings Action checklist
Findings index
Review findings by urgency: 0 required fixes, 0 items to resolve/justify, 2 in-scope improvements
|
E2E Advisor RecommendationRequired E2E: None Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
|
Vitest E2E Scenario RecommendationRequired Vitest E2E scenarios: Dispatch required Vitest E2E scenarios:
Full Vitest E2E advisor summaryVitest E2E Scenario AdvisorBase: Required Vitest E2E scenarios
Optional Vitest E2E scenarios
Relevant changed files
|
|
CodeRabbit’s only pre-merge warning is the generic docstring-coverage heuristic. This PR adds a Bash E2E helper, where Python/TypeScript-style docstrings do not apply; its behavior and safety boundary are explicit in the function name, control flow, PR description, and behavioral tests. Treating this warning as non-blocking. CodeRabbit generated no actionable inline findings, and the PR Review Advisor reports 0 required items and 0 warnings. |
PR Review Advisor — No blocking findingsMerge posture: No blocking advisor findings This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision. |
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
|
Addressed
|
<!-- markdownlint-disable MD041 --> ## Summary Preserves the real exit status from the Deep Agents Code Expect harness and emits a bounded, secret-screened sanitized capture excerpt when the harness fails. This removes a shell declaration-order bug that made timeout and EOF outcomes appear successful, while keeping the strict prompt and exit assertions intact. ## Changes - Declare the Expect status variable before invoking the harness so `local` cannot reset `$?`. - Print the last 20,000 bytes of the sanitized capture after failures only when no secret-shaped value remains. - Add behavioral and image-contract coverage for nonzero Expect outcomes and safe diagnostics. ## Type of Change - [x] 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 - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: release-gate harness diagnostics only; no user-facing behavior changed. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: self-review confirms diagnostic output is read only from the already-sanitized artifact, is suppressed if secret-shaped data remains, and raw/combined captures are still deleted. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Git hooks passed during commit and push, or `npx prek run --from-ref main --to-ref HEAD` passes - [x] Targeted tests pass for changed behavior - [ ] Full `npm test` passes (broad runtime changes only) - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) Targeted verification: ```bash bash -n test/e2e/e2e-cloud-experimental/checks/10-deepagents-code-tui-startup.sh npm test -- --run test/deepagents-code-tui-startup-check.test.ts test/langchain-deepagents-code-image.test.ts npm run typecheck:cli ``` --- Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Expanded TUI startup-check coverage to explicitly preserve lifecycle markers and ensure the embedded harness exits cleanly after marker writes. * Added a new failure-case test that verifies the helper returns the correct non-zero status and emits only the sanitized capture excerpt (including the EOF marker) for diagnostics. * Updated end-to-end checks to confirm the generated startup-check script includes and positions `expect_rc` correctly and prints the sanitized capture excerpt on non-zero runs. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Summary
Preserves the real exit status from the Deep Agents Code Expect harness and emits a bounded, secret-screened sanitized capture excerpt when the harness fails. This removes a shell declaration-order bug that made timeout and EOF outcomes appear successful, while keeping the strict prompt and exit assertions intact.
Changes
localcannot reset$?.Type of Change
Quality Gates
Verification
Verifiedin GitHubnpx prek run --from-ref main --to-ref HEADpassesnpm testpasses (broad runtime changes only)npm run docsbuilds without warnings (doc changes only)Targeted verification:
bash -n test/e2e/e2e-cloud-experimental/checks/10-deepagents-code-tui-startup.sh npm test -- --run test/deepagents-code-tui-startup-check.test.ts test/langchain-deepagents-code-image.test.ts npm run typecheck:cliSigned-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit
expect_rccorrectly and prints the sanitized capture excerpt on non-zero runs.