Skip to content

fix: reduce auto-pair timeout and add skip/configure env vars#538

Closed
brianwtaylor wants to merge 2 commits intoNVIDIA:mainfrom
brianwtaylor:fix/auto-pair-timeout-hardening
Closed

fix: reduce auto-pair timeout and add skip/configure env vars#538
brianwtaylor wants to merge 2 commits intoNVIDIA:mainfrom
brianwtaylor:fix/auto-pair-timeout-hardening

Conversation

@brianwtaylor
Copy link
Copy Markdown
Contributor

@brianwtaylor brianwtaylor commented Mar 20, 2026

Summary

  • Reduce default auto-pair watcher timeout from 5 minutes to 2 minutes
  • Add NEMOCLAW_AUTOPAIR_TIMEOUT env var to configure timeout
  • Add NEMOCLAW_AUTOPAIR_SKIP env var to skip auto-pair entirely
  • Add subprocess timeout to watcher run() helper to prevent indefinite blocking
  • Echo timeout value before nohup redirect for observability

Continuation of closed #145.

Related to #430 — the indefinite blocking during sandbox startup is partly caused by the auto-pair watcher having no timeout; this adds configurable timeouts and a skip option

Test plan

Automated Tests

npm test -- --grep "auto-pair"

Summary by CodeRabbit

  • New Features

    • Background auto-pair watcher can be disabled via an environment variable.
    • Auto-pair timeout duration is now configurable via an environment variable (default: 120s).
  • Improvements

    • Watcher now enforces a global deadline so background jobs cannot exceed the configured timeout.
    • Subprocess invocations respect the remaining deadline and return explicit timeout results when exceeded.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 21, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

start_auto_pair() in scripts/nemoclaw-start.sh keeps the skip/timeout env handling and now enforces the watcher's global deadline in the embedded Python watcher: each subprocess call is bounded by the remaining time, the watcher returns exit code 124 with a clear message when the deadline has passed, and TimeoutExpired is handled to return code 124 with a timeout message.

Changes

Cohort / File(s) Summary
Auto-pair watcher
scripts/nemoclaw-start.sh
Kept NEMOCLAW_SKIP_AUTO_PAIR early-skip and NEMOCLAW_AUTO_PAIR_TIMEOUT read/validate/echo behavior. In the embedded Python watcher, compute DEADLINE = time.time() + timeout; the run(*args) helper now: if remaining = DEADLINE - time.time() <= 0, return (124, '', 'watcher deadline exceeded'); otherwise call subprocess.run(..., timeout=remaining) and on subprocess.TimeoutExpired return (124, '', f'subprocess timed out after {remaining:.0f}s').

Sequence Diagram(s)

sequenceDiagram
    participant Shell as Shell (scripts/nemoclaw-start.sh)
    participant Watcher as Python Watcher (nohup python3 -)
    participant CLI as External CLI (openclaw)
    participant Devices as Devices / JSON

    Shell->>Shell: Check NEMOCLAW_SKIP_AUTO_PAIR\nIf set -> return
    Shell->>Shell: Read & validate NEMOCLAW_AUTO_PAIR_TIMEOUT\nExport & echo TIMEOUT
    Shell->>Watcher: Launch watcher with TIMEOUT env
    Watcher->>Watcher: Compute DEADLINE = now + TIMEOUT\nPrint startup message
    loop until DEADLINE or approvals complete
        Watcher->>Watcher: remaining = DEADLINE - time.time()
        alt remaining <= 0
            Watcher->>Watcher: return (124, '', 'watcher deadline exceeded')
        else
            Watcher->>CLI: run("openclaw devices", timeout=remaining)
            CLI-->>Watcher: devices JSON or (TimeoutExpired)
            alt TimeoutExpired
                Watcher->>Watcher: return (124, '', 'subprocess timed out after Xs')
            else
                Watcher->>Watcher: parse devices JSON
                alt pending approvals found
                    Watcher->>CLI: run("openclaw approve ...", timeout=remaining)
                    CLI-->>Watcher: approve result or (TimeoutExpired)
                end
            end
        end
        Watcher->>Watcher: sleep then retry
    end
    Watcher-->>Shell: exit when DEADLINE reached or all approvals done
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I tightened the timer, kept skips in sight,
Subprocesses bounded, no endless night.
If time runs out, I tap with a paw,
Code 124 — tidy, neat, no thaw.
Hop, hop, the watcher nibbles the log light.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The changeset implements the core requirement of adding subprocess timeout handling [#145], but does not include environment variable support for NEMOCLAW_AUTO_PAIR_TIMEOUT or NEMOCLAW_SKIP_AUTO_PAIR, nor the timeout reduction from 600s to 120s. Implement NEMOCLAW_AUTO_PAIR_TIMEOUT and NEMOCLAW_SKIP_AUTO_PAIR environment variable support, reduce default timeout from 600s to 120s, and add observability logging for the configured timeout.
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly matches the PR objectives and linked issue #145 requirements: reducing auto-pair timeout and adding environment variable configuration.
Out of Scope Changes check ✅ Passed The changes are limited to adding subprocess timeout handling within the internal run() helper, which is directly related to the linked issue #145 requirement for per-subprocess timeouts.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

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 the current code and only fix it if needed.

Inline comments:
In `@scripts/nemoclaw-start.sh`:
- Around line 123-137: The echo currently prints pair_timeout from the shell
which can differ from the Python-normalized timeout (the Python code coerces
invalid/<=0 values to 120 using timeout and DEADLINE); update the script so the
logged timeout reflects the effective value — either validate/coerce
NEMOCLAW_AUTO_PAIR_TIMEOUT in the shell before setting pair_timeout or move the
echo after the Python normalization and have the Python block print the
effective timeout (use the same environment key NEMOCLAW_AUTO_PAIR_TIMEOUT and
the Python variable timeout/DEADLINE to produce the logged value) so the message
shows the actual watcher timeout.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 087d654f-9b57-41ec-acee-f885a765801f

📥 Commits

Reviewing files that changed from the base of the PR and between 867f27d and 0e79d1a.

📒 Files selected for processing (1)
  • scripts/nemoclaw-start.sh

@brianwtaylor brianwtaylor force-pushed the fix/auto-pair-timeout-hardening branch from b671da2 to 19111ce Compare March 21, 2026 05:57
@wscurran wscurran added bug Something isn't working NemoClaw CLI Use this label to identify issues with the NemoClaw command-line interface (CLI). labels Mar 23, 2026
@wscurran
Copy link
Copy Markdown
Contributor

Thanks for the proposed fix to reduce the auto-pair timeout and add configuration options, which could help improve the security and usability of our CLI.

@brianwtaylor brianwtaylor force-pushed the fix/auto-pair-timeout-hardening branch from 19111ce to ba72b64 Compare March 23, 2026 21:58
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

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 the current code and only fix it if needed.

Inline comments:
In `@scripts/nemoclaw-start.sh`:
- Around line 103-110: The run function currently computes remaining = max(1,
int(DEADLINE - time.time())) which becomes 1 after the deadline and still
launches subprocesses; change run to compute remaining = DEADLINE - time.time()
and if remaining <= 0 immediately return (124, '', f'subprocess timed out after
0s') instead of calling subprocess.run, otherwise call subprocess.run with
timeout=remaining (or int(max(1, remaining)) if an int is required). Update the
TimeoutExpired handling to preserve the existing return format; references:
function run, variable DEADLINE, subprocess.run, and subprocess.TimeoutExpired.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9cb34af1-182e-4ec2-8e1f-f977d26ae541

📥 Commits

Reviewing files that changed from the base of the PR and between 19111ce and ba72b64.

📒 Files selected for processing (1)
  • scripts/nemoclaw-start.sh

@brianwtaylor brianwtaylor force-pushed the fix/auto-pair-timeout-hardening branch from ba72b64 to 144450d Compare March 23, 2026 22:50
Replace max(1, ...) clamp with early return when deadline has
passed. Prevents launching subprocesses after the configured
timeout. Use raw float for more precise timeout bound.

Signed-off-by: Brian Taylor <brian.taylor818@gmail.com>
@brianwtaylor brianwtaylor force-pushed the fix/auto-pair-timeout-hardening branch from 144450d to a6f80a3 Compare March 23, 2026 22:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working NemoClaw CLI Use this label to identify issues with the NemoClaw command-line interface (CLI).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants