chore(biome): cover and format all TypeScript files#5020
Merged
Conversation
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Contributor
|
Caution Review failedPull request was closed or merged during review 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 (300)
💤 Files with no reviewable changes (3)
📝 WalkthroughWalkthroughThis PR mainly reformats TypeScript and command metadata across the repository, broadens Biome and pre-commit TypeScript coverage, adds sandbox destroy domain helpers, changes uninstall stdin reading, adds log-merge tests, updates a public argv translation test, and adjusts a few targeted tests and user-facing messages. ChangesRepository-wide formatting and targeted behavior adjustments
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Contributor
E2E Advisor RecommendationRequired E2E: None Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Contributor
E2E Scenario Advisor RecommendationRequired scenario E2E: Dispatch required scenario E2E:
Full scenario advisor summaryE2E Scenario AdvisorBase: Required scenario E2E
Optional scenario E2E
Relevant changed files
|
Contributor
PR Review AdvisorFindings: 1 needs attention, 6 worth checking, 0 nice ideas Review findings🛠️ Needs attention
🔎 Worth checking
🌱 Nice ideas
Consider writing more tests for
Since last review detailsCurrent findings:
This is an automated advisory review. A human maintainer must make the final merge decision. |
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Collaborator
Author
Codebase Growth Guardrail ExemptionThe This PR intentionally separates the tooling change and the mechanical formatting change:
|
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Collaborator
Author
Feedback UpdateAddressed the PR Review Advisor test-size budget finding in What changed:
Validation on the updated branch:
I also dispatched the required scenario E2E from the advisor on the latest PR head: https://github.com/NVIDIA/NemoClaw/actions/runs/27188638108 The remaining |
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Collaborator
Author
Latest Sync / Static Checks UpdateSynced the branch with current What happened:
Validation on the latest head (
I also re-dispatched the required scenario E2E on the latest head: https://github.com/NVIDIA/NemoClaw/actions/runs/27189152047 The remaining known failure is still |
Signed-off-by: Carlos Villela <cvillela@nvidia.com> # Conflicts: # src/lib/core/wait.ts # src/lib/onboard/sandbox-provider-cleanup.ts # test/wait.test.ts # test/whatsapp-qr-compact.test.ts
Signed-off-by: Carlos Villela <cvillela@nvidia.com> # Conflicts: # test/e2e-scenario/framework-tests/e2e-negative-matcher.test.ts # test/e2e-scenario/framework/clients/index.ts # test/e2e-scenario/framework/phases/environment.ts # test/e2e-scenario/scenarios/assertions/registry.ts
Signed-off-by: Carlos Villela <cvillela@nvidia.com> # Conflicts: # test/e2e-scenario/framework-tests/e2e-assertion-modules.test.ts # test/e2e-scenario/framework-tests/e2e-convention-lint.test.ts # test/e2e-scenario/framework-tests/e2e-lib-helpers.test.ts # test/e2e-scenario/framework-tests/e2e-manifests.test.ts # test/e2e-scenario/framework-tests/e2e-scenario-registry.test.ts
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com> # Conflicts: # test/pr-workflow-contract.test.ts
jyaunches
added a commit
that referenced
this pull request
Jun 9, 2026
After merging main (which brought #5020 'cover and format all TypeScript files' into the branch), biome flagged 5 PR files for organize-imports and one wrap. Auto-applied via: npx biome check --write \ test/e2e-scenario/framework-tests/e2e-live-skip-name-contract.test.ts \ test/e2e-scenario/framework-tests/e2e-scenario-matrix.test.ts \ test/e2e-scenario/framework-tests/e2e-scenarios-workflow.test.ts \ test/e2e-scenario/live/registry-scenarios.test.ts \ test/e2e-scenario/scenarios/run.ts Verified post-format: - 24/24 e2e-scenario-framework tests still green (skip-name contract, live-registry-discovery, scenario-matrix, workflow boundary). - Workflow filter sim still works: SCENARIO_ID=ubuntu-repo-cloud-hermes + -t "^ubuntu-repo-cloud-hermes$" exits 0 with structured [not wired] reason in stderr. Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
12 tasks
2 tasks
cv
pushed a commit
that referenced
this pull request
Jun 12, 2026
<!-- markdownlint-disable MD041 --> ## Summary Interactive `nemoclaw uninstall` printed `Proceed? [y/N]` and then immediately `Aborted.` before the user could type — on every Linux TTY. Regression from #5020, which replaced the child-shell stdin read with an in-process `fs.readSync(0)` loop: `buildRuntime` touches `process.stdin` to detect a TTY, which flips fd 0 to non-blocking (libuv side effect), so the empty read throws `EAGAIN` and the catch treated it as EOF. This PR detects the TTY without side effects, retries transient read errors, and clarifies the abort message. ## Related Issue <!-- Fixes #NNN or Closes #NNN. Remove this section if none. --> Fixes #5188 ## Changes - Detect TTY via `tty.isatty(0)` in `buildRuntime` instead of `process.stdin.isTTY`, which instantiates the stdin stream and switches fd 0 to non-blocking mode for the rest of the process. - Retry `EAGAIN`/`EWOULDBLOCK` (25 ms sleep) and `EINTR` in `defaultReadLine` instead of conflating them with EOF; hard errors and real EOF keep prior behavior (return buffered bytes or null). - Print a `re-run with --yes` hint when stdin yields no input at the confirm prompt instead of a bare `Aborted.`. - Export `defaultReadLine` with injectable `readSync`/`sleep` deps; add 7 unit tests (EAGAIN retry, EINTR, EOF, CRLF, partial-line, hard errors) and 1 integration test for the no-input message. Manually verified under a pty against the compiled CLI: with fd 0 poisoned non-blocking, the prompt now waits for input (read `"y"` after ~1 s) instead of aborting in ~1 ms; full `internal uninstall run-plan` waits and aborts only after an actual `n`. ## 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) ## Verification <!-- Check each item you ran and confirmed. Leave unchecked items you skipped. Doc-only changes do not require npm test unless you ran it. --> - [ ] `npx prek run --all-files` passes - [ ] `npm test` passes - [ ] Tests added or updated for new or changed behavior - [ ] No secrets, API keys, or credentials committed - [ ] Docs updated for user-facing behavior changes - [ ] `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) --- <!-- DCO sign-off required by CI. Run: git config user.name && git config user.email --> Signed-off-by: Hung Le <hple@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved uninstall confirmation behavior when stdin is closed/non-interactive; now logs a clear message and guides rerunning with --yes. * Better terminal detection to avoid unwanted side effects and ensure interactive prompts behave correctly. * **Tests** * Added robust tests covering no-stdin, TTY vs non-TTY, retry/error cases, and PTY-driven prompt interactions to prevent regressions. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
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
3 participants
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
Expand Biome coverage so every tracked TypeScript file is included in repo-wide formatting and linting, then apply the resulting formatting pass. The formatting commit also updates the test file size budget to the post-format line counts.
Changes
biome.jsonTypeScript includes to cover root, nested, test, agent, and config.tsfiles..tsfiles trigger the Biome hooks.src/**/*.tsformatter opt-out and format the newly covered TypeScript files.ci/test-file-size-budget.jsonfor deterministic post-format line counts.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesnpm run docsbuilds without warnings (doc changes only)Signed-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit