Skip to content

fix(config): fail when new-key prompt reaches EOF - #7565

Merged
cv merged 10 commits into
NVIDIA:mainfrom
harjothkhara:codex/fix-config-prompt-eof
Jul 26, 2026
Merged

fix(config): fail when new-key prompt reaches EOF#7565
cv merged 10 commits into
NVIDIA:mainfrom
harjothkhara:codex/fix-config-prompt-eof

Conversation

@harjothkhara

@harjothkhara harjothkhara commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Summary

config set previously could exit successfully without confirming or writing a new key when stdin reached EOF at its confirmation prompt. This change delegates that prompt to the shared hardened prompt implementation and fails nonzero with explicit opt-in guidance when input closes before an answer.

Changes

  • Reuse the shared EOF- and SIGINT-aware prompt implementation for new-key confirmation.
  • Convert only stdin EOF into a SandboxConfigError that points to --config-accept-new-path and NEMOCLAW_CONFIG_ACCEPT_NEW_PATH=1; preserve other prompt errors and existing yes/no matching.
  • Add source-level write-boundary tests and compiled-CLI pipe tests for EOF, empty and unterminated answers, negative answers, and whitespace-padded confirmation.
  • Keep the new source-level test free of if statements by restoring captured property descriptors through one named helper and using vi.stubEnv / vi.unstubAllEnvs, so the test satisfies the changed-test-file conditionals guardrail without changing what it asserts.

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:
  • 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: Exact product and nine-category security review PASS at 3aad9a99f8a35712321f54fb4814ea0b69375486 (tree ade6c05913d5d9929a3f16f09d693097b95cb502, base 2c30e11461ec910cdfe12e6889cc6d448606b2bf). EOF, empty, negative, and interrupted prompt paths fail before config or audit writes; only exact confirmation or the existing strict opt-in can write. Existing shields, credential-path, dotpath, SSRF/DNS pinning, schema, digest/CAS, and mutation-lock controls are unchanged; all nine categories pass with no findings. Stable authored patch ID 307b6919255becca30591d7fea7dcca2d66c2f01 is unchanged across path-disjoint refresh merges.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Documentation Writer Review

  • Documentation writer subagent reviewed the completed changes
  • Result: docs-updated
  • Changed documentation: docs/reference/commands.mdx
  • Evidence: The page accurately documents config set, EOF/no-write behavior, strict NEMOCLAW_CONFIG_ACCEPT_NEW_PATH=1 opt-in, OpenClaw/Hermes scope, and Deep Agents re-onboarding guidance. Review against WRITING.md and docs/CONTRIBUTING.md passed for terminology, active voice, structure, one-sentence-per-line source, copyable commands, and AgentOnly variant gating.
  • Validation: Focused suites 130/130; compiled CLI contracts 5/5; CLI build and typecheck passed; repository checks passed; npm run docs passed with unchanged generated variants and zero Fern errors. Printed Fern warnings were known baseline warnings.
  • Agent: Codex Desktop documentation-writer subagent

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 — exact head 3aad9a99f: CLI build and typecheck passed; source config tests 23/23, focused config/prompt/credentials integration suites 130/130, and compiled CLI prompt/dispatch contracts 5/5 passed; npm run docs passed with zero errors and two pre-existing warnings; final exact-head npm run check:diff passed.
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result: Not run; this is a focused CLI prompt fix with targeted source/compiled tests and normal hooks.
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings introduced by this PR — the two printed Fern warnings (unauthenticated redirect check, accent contrast) are pre-existing baseline warnings, reproduced on a checkout without this PR's changes
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: harjoth harjoth.khara@gmail.com

Summary by CodeRabbit

  • Bug Fixes
    • Improved config set when writing a previously-missing config path, with more consistent confirmation handling.
    • Stdin EOF now fails gracefully with clearer instructions to use --config-accept-new-path / NEMOCLAW_CONFIG_ACCEPT_NEW_PATH=1.
    • Whitespace-padded yes is accepted; explicit aborts (including newline-only) no longer write config or audit records.
  • Documentation
    • Expanded config set command reference, including agent-specific constraints and strict opt-in semantics for NEMOCLAW_CONFIG_ACCEPT_NEW_PATH.
  • Tests
    • Added unit and CLI contract tests covering EOF, abort, whitespace-yes, and success flows.

@copy-pr-bot

copy-pr-bot Bot commented Jul 26, 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.

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The config command now uses shared credential prompting for new-key confirmation, converts stdin EOF into a configuration error with acceptance guidance, and preserves other prompt errors. Unit tests, compiled CLI tests, and command documentation cover the updated behavior.

Changes

Config prompt handling

Layer / File(s) Summary
Shared prompt and EOF handling
src/lib/sandbox/config.ts
confirmYesNo delegates to askPrompt, while configSet converts EOF prompt failures into SandboxConfigError and rethrows other errors.
Prompt branch unit coverage
test/config-set-prompt-error.test.ts
Mocks config, audit, execution, and credential modules to verify EOF, non-EOF, affirmative, and negative prompt outcomes.
Compiled CLI prompt contract coverage
test/package-contract/cli/config-set-prompt-eof.test.ts
Runs the compiled CLI with controlled stdin to validate EOF, empty, whitespace-padded affirmative, and unterminated affirmative inputs.
Config command behavior documentation
docs/reference/commands.mdx
Documents config mutation rules, prompt behavior, supported flags, blocked paths, Deep Agents limitations, and strict acceptance-variable semantics.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant configSet
  participant confirmYesNo
  participant askPrompt
  participant ConfigStore
  User->>configSet: set previously-missing key
  configSet->>confirmYesNo: request confirmation
  confirmYesNo->>askPrompt: read answer
  askPrompt-->>confirmYesNo: yes/no or EOF
  alt affirmative answer
    confirmYesNo-->>configSet: true
    configSet->>ConfigStore: write config and audit entry
  else negative answer
    confirmYesNo-->>configSet: false
    configSet-->>User: Aborted.
  else EOF
    confirmYesNo-->>configSet: EOF error
    configSet-->>User: config acceptance guidance
  end
Loading

Suggested labels: v0.0.95

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: config set now fails when the new-key confirmation prompt hits EOF.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@github-actions

github-actions Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions

Model lanes

  • GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Model comparison: normalized findings match; normalized E2E selections differ; severity counts match.

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: None

Workflow run details

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

@apurvvkumaria apurvvkumaria self-assigned this Jul 26, 2026
@harjothkhara
harjothkhara marked this pull request as ready for review July 26, 2026 05:07

@cv cv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The fail-closed EOF behavior is correct, but the documentation at docs/reference/commands.mdx:3671 describes NEMOCLAW_CONFIG_ACCEPT_NEW_PATH as handling stored/discovered path drift under onboarding. The implementation uses it only when nemoclaw <name> config set targets a dotpath absent from the sandbox config (src/lib/config.ts:425-427,1164-1168). Please move the row to the existing-sandbox/config-set behavior and describe it directly, for example: “Allows nemoclaw <name> config set to write a dotpath that is absent from the sandbox config without prompting. All other validation still applies.” Then rerun the documentation writer for the exact refreshed head; current base/CI/E2E are stale and will also need one fresh cycle.

Signed-off-by: harjoth <harjoth.khara@gmail.com>
The codebase-growth-guardrails check rejects newly added `if` statements
in changed test files. Restore the captured stdin and require-cache
property descriptors through one named helper, and use `vi.stubEnv` with
`vi.unstubAllEnvs` for the two environment variables, so setup and
teardown stay linear.

Test behavior is unchanged.

Signed-off-by: harjoth <harjoth.khara@gmail.com>
… config set

The environment variable row described accepting a new sandbox config file
path during onboarding. The variable actually lets `config set` write a
dotpath that is absent from the sandbox config.

Move the row to the existing-sandbox table, describe the real behavior, and
add a `config set` reference section so `--config-accept-new-path` is
documented alongside it.

Signed-off-by: harjoth <harjoth.khara@gmail.com>
The new config set section rendered into the Deep Agents guide, where the
command always fails because dcode bakes its config into the sandbox image.

Gate the OpenClaw and Hermes body, point Deep Agents at the re-onboard path,
and state the end-of-input outcome alongside the interactive and no-TTY ones.

Signed-off-by: harjoth <harjoth.khara@gmail.com>
@harjothkhara

Copy link
Copy Markdown
Contributor Author

Thanks — you're right about that row. Moved it out of the onboarding table into the existing-sandbox table and described what it actually does: lets config set write a dotpath that isn't already in the sandbox config, without the prompt. Also added a config set reference section so --config-accept-new-path is documented next to it.

Rebased onto current main (d525e1518) and reran the doc writer at the new head. It caught that the new section was ungated and would render into the Deep Agents guide, where config set always fails on the baked-in TOML config — so that section is now gated to OpenClaw/Hermes with a Deep Agents block pointing at re-onboard. Also documented the EOF outcome next to the interactive and no-TTY ones.

Checks at a8ee43727: npm run docs clean, prompt tests 4/4, cli src/lib/sandbox 309/309, typecheck:cli clean.

Minor: the gate is in src/lib/sandbox/config.ts:425, not src/lib/config.ts.

#7572 had this same docs change as a standalone PR — closed as superseded.

@harjothkhara
harjothkhara requested a review from cv July 26, 2026 06:51

@cv cv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The runtime behavior and requested lifecycle-flag documentation are now correct, but this exact head still has two release-gate blockers:

  1. docs/reference/commands.mdx:3868 is outside an <AgentOnly> guard, so the generated Deep Agents guide says nemo-deepagents <name> config set can write a new dotpath even though that command is unavailable there. Please wrap that table row in <AgentOnly variant="openclaw,hermes">.
  2. The PR body still records the prior 882317693 docs-review head, no-docs-needed, no docs paths, code-only type, and docs-not-applicable. After the final docs fix, rerun the documentation writer and record docs-updated, docs/reference/commands.mdx, the final head SHA, and the current AGENTS blob. Refresh/explain the sensitive-path receipt for the rebased exact head as required.

Then run the exact-head CI/E2E gates and request re-review. The prompt path itself remains fail-closed: EOF becomes guidance, while SIGINT/other prompt failures propagate, and no negative/empty path writes config or audit state.

cv added 3 commits July 26, 2026 06:30
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@harjothkhara
harjothkhara requested a review from cv July 26, 2026 18:38

@cv cv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approved at exact head 3aad9a9. The two prior documentation blockers are fixed; exact-head docs receipt, product/security review, local validation, all 43 current checks, both automated advisor lanes, CodeRabbit, and the canonical merge gate pass with no unresolved threads.

@cv
cv merged commit 17a8ad1 into NVIDIA:main Jul 26, 2026
43 checks passed
@cv cv mentioned this pull request Jul 26, 2026
23 tasks
apurvvkumaria pushed a commit that referenced this pull request Jul 27, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Add the canonical `docs/changelog/2026-07-25.mdx` release entry with the
exact `## v0.0.96` heading.
The entry reconciles all 90 first-parent commits since v0.0.95 with all
92 merged PRs in the live `v0.0.96` label ledger and groups the
user-visible changes by operator journey.

## Changes

- Add the parser-safe dated MDX changelog entry for v0.0.96 with
root-absolute links to the focused user guides.
- Source summary:
- [#7194](#7194) ->
`docs/changelog/2026-07-25.mdx`: Document persistent baseline network
policy exclusions and their inspection, rebuild, and snapshot behavior.
- [#7188](#7188),
[#7427](#7427), and
[#7546](#7546) ->
`docs/changelog/2026-07-25.mdx`: Document DNS-backed HTTPS inference
routing, keyless loopback endpoints, and provider-marker isolation.
- [#7238](#7238) ->
`docs/changelog/2026-07-25.mdx`: Document blueprint sandbox and provider
identifier validation before state writes or OpenShell calls, with
bounded terminal-safe rejection previews.
- [#7319](#7319),
[#7274](#7274),
[#7528](#7528),
[#7353](#7353), and
[#7560](#7560) ->
`docs/changelog/2026-07-25.mdx`: Document the managed default gateway
service, onboarding readiness, and container-runtime identity
safeguards.
- [#7349](#7349),
[#7498](#7498),
[#7406](#7406),
[#7196](#7196),
[#7559](#7559),
[#7421](#7421),
[#7510](#7510),
[#7295](#7295), and
[#7565](#7565) ->
`docs/changelog/2026-07-25.mdx`: Document gateway-scoped status,
lifecycle diagnostics, managed MCP recovery, delete-edge safeguards, and
fail-closed CLI prompt and command output.
- [#7591](#7591) ->
`docs/changelog/2026-07-25.mdx`: Document opt-in authenticated MCP
tool-name discovery, its bounded and names-only contract, probe
interaction, and rebuild requirement.
- [#7305](#7305),
[#7480](#7480),
[#7471](#7471),
[#7365](#7365), and
[#7541](#7541) ->
`docs/changelog/2026-07-25.mdx`: Document installer version checks,
version-tag reporting, license guidance, WSL Ollama selection, and DGX
Station vLLM detection.
- [#7482](#7482),
[#7466](#7466),
[#7208](#7208),
[#7434](#7434), and
[#7586](#7586) ->
`docs/changelog/2026-07-25.mdx`: Document Ollama resource details,
reasoning precedence, Hermes onboarding behavior, and preserved managed
Hermes BuildKit failures.

- [#6830](#6830),
[#7492](#7492),
[#7563](#7563), and
[#7582](#7582) ->
`docs/changelog/2026-07-25.mdx`: Document the authoritative OpenClaw
production lock, fixed managed-image dependencies, immutable Hermes base
adoption, and Hermes image-size reduction.
- [#7505](#7505),
[#7530](#7530),
[#7547](#7547),
[#7508](#7508),
[#7548](#7548),
[#7549](#7549),
[#7537](#7537),
[#7534](#7534),
[#7515](#7515),
[#7511](#7511),
[#7551](#7551),
[#7562](#7562),
[#7575](#7575),
[#7496](#7496),
[#7594](#7594),
[#7595](#7595), and
[#7599](#7599) ->
`docs/changelog/2026-07-25.mdx`: Summarize release validation, transient
and bounded dispatch reconciliation, exact pre-tag qualification,
identity revalidation, npm-audit retry, sharding, image reuse, timeout,
telemetry, and workflow-hardening changes.
- Reconciled without separate changelog prose:
- [#7539](#7539),
[#7526](#7526),
[#7507](#7507),
[#7506](#7506),
[#7519](#7519),
[#7516](#7516),
[#7396](#7396),
[#7254](#7254),
[#7583](#7583),
[#7596](#7596), and
[#7598](#7598): Test-harness or
fixture-only changes.
- [#7403](#7403),
[#7161](#7161),
[#6877](#6877),
[#7531](#7531),
[#7525](#7525),
[#7522](#7522),
[#7536](#7536),
[#7552](#7552),
[#7566](#7566),
[#7553](#7553),
[#7561](#7561),
[#7577](#7577),
[#7569](#7569),
[#7585](#7585),
[#7584](#7584),
[#7592](#7592),
[#7580](#7580),
[#7571](#7571),
[#7517](#7517),
[#7589](#7589),
[#7402](#7402),
[#7558](#7558),
[#7544](#7544), and
[#7601](#7601): Dependency,
internal recovery, validation, contributor-workflow, E2E optimization,
telemetry, or CI trust changes with no separate user-facing release
claim.
- [#7556](#7556),
[#7573](#7573),
[#7576](#7576), and
[#7578](#7578): Experimental
repository-maintainer conflict automation with no canonical user
documentation surface.

## Type of Change

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

## Quality Gates

- [ ] Tests added or updated for changed behavior
- [x] Existing tests cover changed behavior — justification:
`test/changelog-docs.test.ts` validates dated changelog structure,
version headings, and published links.
- [ ] Tests not applicable — justification:
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [ ] 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

- [x] Documentation writer subagent reviewed the completed changes
- Result: `docs-updated`
- Evidence: Reviewed `docs/changelog/2026-07-25.mdx` at exact head
`0f5dedb47` against 90 first-parent release commits and 92 merged PRs
labeled `v0.0.96`. Verified parser-safe MDX SPDX, the exact version
heading, literal CLI names, writing style, skip terms, all 20
root-absolute published links, and the accepted #7591 opt-in
authenticated discovery bounds. #7544, #7599, and #7601 remain internal
or CI-only release-ledger entries. Changelog tests passed 6/6, the docs
build passed with 0 errors and two pre-existing Fern warnings, and `npm
run check:diff` plus the final diff check passed.
- Agent: Codex Desktop documentation-writer subagent
<!-- docs-review-head-sha: 0f5dedb -->
<!-- docs-review-agents-blob-sha: be20a09 -->

## DGX Station Hardware Evidence

- [ ] Tested on DGX Station
- Tested commit:
- Station profile/scenario:
- Result:
- Supporting evidence:

## Verification

- [x] PR description includes a `Signed-off-by:` line and every commit
appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — `npx vitest run
test/changelog-docs.test.ts`: 6/6 passed.
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result: Not applicable to this
prose-only changelog entry.
- [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) — the
build passed with 0 errors and 2 existing Fern warnings; the
published-route check passed.
- [x] 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)
— native changelog files use the required parser-safe MDX SPDX comment
and no frontmatter.

---
Signed-off-by: Carlos Villela <cvillela@nvidia.com>


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

* **New Features**
* Persistent network policy exclusions with consistent restore/exclusion
reporting across rebuilds/snapshots.
* Opt-in MCP tool discovery via `mcp status --tools` with bounded,
redacted authenticated traffic.
* Improved HTTPS inference switching for custom endpoints and refreshed
onboarding/model menu details.
* Refined OpenShell gateway defaults for port `8080`, including more
reliable readiness checks.
* **Bug Fixes**
* Prevent incorrect provider/model restoration after compatible-provider
update failures.
* Preserve managed MCP state after exec loss and tighten gateway/doctor
status scoping.
* **Tests**
* Stronger, fail-closed release validation with hardened
evidence/artifact handoff and bounded timeouts/retries.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Co-authored-by: Prekshi Vyas <prekshiv@nvidia.com>
@wscurran wscurran added area: cli Command line interface, flags, terminal UX, or output bug-fix PR fixes a bug or regression labels Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: cli Command line interface, flags, terminal UX, or output bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants