Skip to content

feature: custom settings for using build endpoints - #2

Closed
ericksoa wants to merge 8 commits into
mainfrom
feat/custom-settings-build-endpoint
Closed

feature: custom settings for using build endpoints#2
ericksoa wants to merge 8 commits into
mainfrom
feat/custom-settings-build-endpoint

Conversation

@ericksoa

Copy link
Copy Markdown
Contributor

Migrated from NVIDIA/openshell-openclaw-plugin#18 by @nv-kasikritc

@cv

cv commented Mar 21, 2026

Copy link
Copy Markdown
Collaborator

Hey @ericksoa — just circling back on this one. I know you've got a few PRs open here (thanks for all the contributions, by the way!). Things have moved pretty quickly in the repo since this was first submitted — new CI pipeline, additional features, etc. Could you rebase this branch against the latest main so we can evaluate it cleanly? Would love to take a fresh look at the custom build endpoint settings. Thanks!

@ericksoa

Copy link
Copy Markdown
Contributor Author

Superseded — inference provider configuration is now handled by the blueprint system in onboard.

@ericksoa ericksoa closed this Mar 22, 2026
jyaunches pushed a commit to jyaunches/NemoClaw that referenced this pull request Apr 14, 2026
- Guard runArgv/runArgvCapture against shell:true to prevent security
  bypass (finding #1) — throws if a caller attempts to re-enable shell
  interpretation. Added 2 tests.
- Document the intentional bash -c exception in getOllamaWarmupCommand
  explaining why it's safe (finding NVIDIA#2).
- Remove dead getOpenshellCommand() from policies.ts (finding NVIDIA#3).
- Remove unused shellQuote import from nim.ts (finding NVIDIA#4).
- Fix brittle indexOf assertion in onboard-readiness test (finding NVIDIA#5).
prekshivyas added a commit to ColinM-sys/NemoClaw that referenced this pull request Apr 16, 2026
…exercised

CodeRabbit correctly flagged that swapping on stat NVIDIA#1 caused
readFileSync to see the live PID and exit via isProcessAlive —
unlinkIfInodeMatches was never called. Move the swap to just before
stat NVIDIA#2 (inside unlinkIfInodeMatches): stat NVIDIA#1 reads the original
stale inode, readFileSync sees the dead PID, isProcessAlive returns
false, stale-cleanup runs, and stat NVIDIA#2 sees the new inode and skips
the unlink.

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
prekshivyas added a commit to ColinM-sys/NemoClaw that referenced this pull request Apr 16, 2026
…exercised

CodeRabbit correctly flagged that swapping on stat NVIDIA#1 caused
readFileSync to see the live PID and exit via isProcessAlive —
unlinkIfInodeMatches was never called. Move the swap to just before
stat NVIDIA#2 (inside unlinkIfInodeMatches): stat NVIDIA#1 reads the original
stale inode, readFileSync sees the dead PID, isProcessAlive returns
false, stale-cleanup runs, and stat NVIDIA#2 sees the new inode and skips
the unlink.

Use write-to-temp + rename instead of unlink + recreate to guarantee
a different inode even on tmpfs/overlayfs which can reuse inodes.

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
jyaunches pushed a commit that referenced this pull request Apr 20, 2026
- Remove unused getForwardList() call from getActiveSandboxSessions —
  only pgrep/ps is needed for SSH session detection (warning #1)
- Consolidate double-prompt in sandboxDestroy into single enriched
  confirmation prompt (warning #2)
- Remove noisy cleanupGatewayAfterLastSandbox forward check that would
  always fire due to dashboard forward (warning #3)
- Use word-boundary regex in parseSshProcesses to prevent false positives
  when sandbox names share prefixes (warning #4)
- Export SessionClassification as named interface (suggestion #1)
- Use cross-platform ps -axo instead of Linux-only pgrep -a for macOS
  compatibility (suggestion #2)
- Add forwardCount to SessionClassification for future consumers
- Add tests for word-boundary matching edge cases
latenighthackathon referenced this pull request in latenighthackathon/NemoClaw Apr 24, 2026
Parallels the bot-token check: adds appTokenFormat / appTokenFormatHint
to ChannelDef, pins Slack's app token to ^xapp-[A-Za-z0-9_-]+$, and
wires the check into setupMessagingChannels so a bogus xapp- prompt
drops the channel from the enabled set and skips saveCredential for
SLACK_APP_TOKEN.

Note: if the bot token passes but the app token fails, the bot token
stays persisted (it was already saved before the app-token prompt).
That's acceptable — on a retry onboard the bot token lights up as
"already configured" and only the app-token prompt fires again.

Extends the existing regex test with xapp- valid/invalid cases and
adds a second interactive integration test covering the
bot-valid + app-invalid path.

Per @jyaunches suggestion #2 on NVIDIA#2130.

Signed-off-by: latenighthackathon <latenighthackathon@users.noreply.github.com>
latenighthackathon referenced this pull request in latenighthackathon/NemoClaw Apr 24, 2026
Parallels the bot-token check: adds appTokenFormat / appTokenFormatHint
to ChannelDef, pins Slack's app token to ^xapp-[A-Za-z0-9_-]+$, and
wires the check into setupMessagingChannels so a bogus xapp- prompt
drops the channel from the enabled set and skips saveCredential for
SLACK_APP_TOKEN.

Note: if the bot token passes but the app token fails, the bot token
stays persisted (it was already saved before the app-token prompt).
That's acceptable — on a retry onboard the bot token lights up as
"already configured" and only the app-token prompt fires again.

Extends the existing regex test with xapp- valid/invalid cases and
adds a second interactive integration test covering the
bot-valid + app-invalid path.

Per @jyaunches suggestion #2 on NVIDIA#2130.

Signed-off-by: latenighthackathon <latenighthackathon@users.noreply.github.com>
@cv
cv deleted the feat/custom-settings-build-endpoint branch June 28, 2026 00:24
cjagwani added a commit that referenced this pull request Jul 2, 2026
… paths (#6188)

Address advisor PRA-1 / PRA-7 and CodeRabbit Major #2:

- backupStateFile now returns { outcome, unreachable } instead of a
  bare string enum. Transport-level SSH failures (exit 255, signal
  kill, spawn error) during state-file backup set unreachable=true.
  The caller in backupSandboxState propagates this to the outer
  BackupResult.unreachable so NEMOCLAW_SKIP_UNREACHABLE_SANDBOX_BACKUP=1
  activates for state-file failures, not only the initial dir probe.

- getSshConfig-null branch in backupSandboxState now returns
  unreachable=true. For a sandbox already confirmed running (the
  maintenance.ts loop only reaches BackupResult.unreachable for
  running sandboxes), an ssh-config lookup failure is transport-level.
  Addresses CodeRabbit's Major finding at sandbox.ts:1128-1141.

Both branches now feed the same skip path already exercised by the
initial dir probe. Propagation is covered by manual E2E for the
running-sandbox-goes-unreachable-mid-loop case; unit tests would
require heavy spawnSync mocking that this PR does not introduce.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
cv pushed a commit that referenced this pull request Jul 6, 2026
…board dcode identity failure (#6343)

## Summary
Fixes two E2E-harness problems surfaced by the E2E dispatch on `main`:

1. **e2e-live source require hook** — the `hermes-inference-switch` live
suite (added in #6335) failed at collection with `Cannot find module
'../runner'` because the `e2e-live` Vitest project never loaded the
typed-source require hook.
2. **cloud-onboard observability** — the cloud-experimental check-04
(added in #6332) fails on `main` with `could not read initial dcode
identity`, but the real `dcode identity` error is captured into a shell
var and discarded, so it can't be diagnosed.

## Changes
- `vitest.config.ts`: add `setupFiles:
["test/helpers/onboard-script-mocks.cjs"]` to the `e2e-live` project
(mirrors `cli`). Registers the typed-source `.ts` require hook
**in-process** — deliberately not via `env.NODE_OPTIONS`, so `--require`
never leaks into the real CLI subprocesses live tests spawn.
-
`test/e2e/e2e-cloud-experimental/checks/04-deepagents-code-fresh-reonboard.sh`:
on both `dcode identity` reads, print the captured stdout+stderr to
stdout (so it lands in `result.json`) before `fail`. No change to
pass/fail logic — only makes the existing failure observable.

### Root cause — #1 (fixed here)
`hermes-inference-switch` is the first `e2e-live` suite to import a deep
`src` graph — its helpers import `src/lib/inference/config.ts`, which
transitively loads `ollama-runtime-context.ts`'s runtime
`require("../runner")`. Without the require hook, Node's native CJS
resolver can't resolve the extensionless `.ts` → suite throws at
collection (`0 tests`, ~37s). Only this suite hit it; others drive the
CLI as a subprocess and import only fixtures. `runner.ts` has no
circular dependency on the inference graph, so the in-process hook
resolves it fully.

### Root cause — #2 (observability only; product root cause pending)
`cloud-onboard` was green on `main` through 2026-07-06 00:56 UTC and
failed on the first main E2E after #6332 landed (19:50 UTC) — #6332
added check-04, which has never passed on main. `dcode identity` is the
NemoClaw wrapper (`agents/langchain-deepagents-code/dcode-wrapper.sh`);
its identity path returns 0 in isolation and #6332 did not modify it, so
the non-zero exit is a runtime condition in #6332's new "recreate/verify
live identity" onboarding flow. That can't be pinned without the
swallowed stderr — which this change surfaces. Product root cause is for
the #6332 author to fix once the next run shows the real error.

## Type of Change
- [x] Code change (feature, bug fix, or refactor)

## Quality Gates
- [x] Existing tests cover changed behavior — justification: both
changes are E2E-harness config/diagnostics. #1: the `e2e-live` suites
exercise the hook — verified the previously-failing
`hermes-inference-switch` suite now collects and all `e2e-live` suites
report 0 collection errors, and the full `e2e-all` dispatch ran the
switch job (hosted) green. #2: pure diagnostic output; no pass/fail
change.
- [x] Docs not applicable — justification: internal test-harness
config/diagnostics; no user-facing behavior.
- [x] Sensitive paths changed (onboarding/inference/runner adjacent via
test config)
- [x] Sensitive-path review completed or maintainer-approved waiver
recorded — justification: changes are confined to Vitest test-runner
setup (`vitest.config.ts`) and an E2E diagnostic print; no product
runtime code path is altered. The require hook is in-process only and
does not touch product CLI subprocesses.

## Verification
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed —
note: `pre-push` `tsc-cli` skipped for a pre-existing local-only
`noImplicitAny` false-positive in
`test/helpers/mcp-lifecycle-lock-properties.ts` and
`src/lib/state/mcp-lifecycle-lock-identity.test.ts` (unrelated to this
diff); CI `tsc-cli` covers it. Check-04 passed `bash -n` and the
`shellcheck`/`shfmt` pre-commit hooks.
- [x] Targeted behavior tests pass — command/result:
`NEMOCLAW_RUN_LIVE_E2E=1 npx vitest list --project e2e-live` → switch
suite collects; whole project 0 collection errors (before the fix it
reproduced `Cannot find module '../runner'`). Full `e2e-all` dispatch on
this branch ran `hermes-inference-switch (hosted)` green.
- [x] No secrets, API keys, or credentials committed

---
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* Improved reliability of live end-to-end test runs by loading the
required hook inside the test process, avoiding leakage into real CLI
subprocesses.
* Made identity checks in sandboxed cloud experimental flows more
robust, with clearer diagnostics when identity lookup fails.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
…board dcode identity failure (NVIDIA#6343)

## Summary
Fixes two E2E-harness problems surfaced by the E2E dispatch on `main`:

1. **e2e-live source require hook** — the `hermes-inference-switch` live
suite (added in NVIDIA#6335) failed at collection with `Cannot find module
'../runner'` because the `e2e-live` Vitest project never loaded the
typed-source require hook.
2. **cloud-onboard observability** — the cloud-experimental check-04
(added in NVIDIA#6332) fails on `main` with `could not read initial dcode
identity`, but the real `dcode identity` error is captured into a shell
var and discarded, so it can't be diagnosed.

## Changes
- `vitest.config.ts`: add `setupFiles:
["test/helpers/onboard-script-mocks.cjs"]` to the `e2e-live` project
(mirrors `cli`). Registers the typed-source `.ts` require hook
**in-process** — deliberately not via `env.NODE_OPTIONS`, so `--require`
never leaks into the real CLI subprocesses live tests spawn.
-
`test/e2e/e2e-cloud-experimental/checks/04-deepagents-code-fresh-reonboard.sh`:
on both `dcode identity` reads, print the captured stdout+stderr to
stdout (so it lands in `result.json`) before `fail`. No change to
pass/fail logic — only makes the existing failure observable.

### Root cause — rh-ai-quickstart#1 (fixed here)
`hermes-inference-switch` is the first `e2e-live` suite to import a deep
`src` graph — its helpers import `src/lib/inference/config.ts`, which
transitively loads `ollama-runtime-context.ts`'s runtime
`require("../runner")`. Without the require hook, Node's native CJS
resolver can't resolve the extensionless `.ts` → suite throws at
collection (`0 tests`, ~37s). Only this suite hit it; others drive the
CLI as a subprocess and import only fixtures. `runner.ts` has no
circular dependency on the inference graph, so the in-process hook
resolves it fully.

### Root cause — NVIDIA#2 (observability only; product root cause pending)
`cloud-onboard` was green on `main` through 2026-07-06 00:56 UTC and
failed on the first main E2E after NVIDIA#6332 landed (19:50 UTC) — NVIDIA#6332
added check-04, which has never passed on main. `dcode identity` is the
NemoClaw wrapper (`agents/langchain-deepagents-code/dcode-wrapper.sh`);
its identity path returns 0 in isolation and NVIDIA#6332 did not modify it, so
the non-zero exit is a runtime condition in NVIDIA#6332's new "recreate/verify
live identity" onboarding flow. That can't be pinned without the
swallowed stderr — which this change surfaces. Product root cause is for
the NVIDIA#6332 author to fix once the next run shows the real error.

## Type of Change
- [x] Code change (feature, bug fix, or refactor)

## Quality Gates
- [x] Existing tests cover changed behavior — justification: both
changes are E2E-harness config/diagnostics. rh-ai-quickstart#1: the `e2e-live` suites
exercise the hook — verified the previously-failing
`hermes-inference-switch` suite now collects and all `e2e-live` suites
report 0 collection errors, and the full `e2e-all` dispatch ran the
switch job (hosted) green. NVIDIA#2: pure diagnostic output; no pass/fail
change.
- [x] Docs not applicable — justification: internal test-harness
config/diagnostics; no user-facing behavior.
- [x] Sensitive paths changed (onboarding/inference/runner adjacent via
test config)
- [x] Sensitive-path review completed or maintainer-approved waiver
recorded — justification: changes are confined to Vitest test-runner
setup (`vitest.config.ts`) and an E2E diagnostic print; no product
runtime code path is altered. The require hook is in-process only and
does not touch product CLI subprocesses.

## Verification
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed —
note: `pre-push` `tsc-cli` skipped for a pre-existing local-only
`noImplicitAny` false-positive in
`test/helpers/mcp-lifecycle-lock-properties.ts` and
`src/lib/state/mcp-lifecycle-lock-identity.test.ts` (unrelated to this
diff); CI `tsc-cli` covers it. Check-04 passed `bash -n` and the
`shellcheck`/`shfmt` pre-commit hooks.
- [x] Targeted behavior tests pass — command/result:
`NEMOCLAW_RUN_LIVE_E2E=1 npx vitest list --project e2e-live` → switch
suite collects; whole project 0 collection errors (before the fix it
reproduced `Cannot find module '../runner'`). Full `e2e-all` dispatch on
this branch ran `hermes-inference-switch (hosted)` green.
- [x] No secrets, API keys, or credentials committed

---
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* Improved reliability of live end-to-end test runs by loading the
required hook inside the test process, avoiding leakage into real CLI
subprocesses.
* Made identity checks in sandboxed cloud experimental flows more
robust, with clearer diagnostics when identity lookup fails.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature PR adds or expands user-visible functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants