Skip to content

test(ollama): preserve proxy token during re-onboard - #7620

Merged
prekshivyas merged 2 commits into
mainfrom
codex/fix-ollama-token-reonboard
Jul 27, 2026
Merged

test(ollama): preserve proxy token during re-onboard#7620
prekshivyas merged 2 commits into
mainfrom
codex/fix-ollama-token-reonboard

Conversation

@sandl99

@sandl99 sandl99 commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Summary

Preserves the committed local Ollama proxy token during re-onboarding so the token mounted in the existing sandbox does not diverge from the restarted host proxy. Temporary compatible custom-endpoint setup still receives a fresh token and retains its rollback behavior.

Changes

  • Reuse the persisted proxy token when starting the committed local Ollama route.
  • Keep fresh-token generation explicit for the uncommitted compatible custom-endpoint route.
  • Update the provider-switch recovery test to protect token continuity while still requiring the backend to switch to local Ollama.

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: This restores credential continuity for the existing local Ollama workflow without changing commands, configuration, or required user actions.
  • 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: Codex CLI security review passed with no findings: test(ollama): preserve proxy token during re-onboard #7620 (comment)
  • 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: no-docs-needed
  • Evidence: No documentation paths changed. The commit restores the existing Local Ollama re-onboarding contract by retaining the persisted proxy token. It does not change the documented proxy setup, token file, commands, configuration, or user workflow.
  • Agent: Codex CLI

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 — Ollama proxy recovery, rollback, and startup integration tests (16 passed); setup-nim-ollama CLI tests (14 passed)
  • Applicable broad gate passed — npm run typecheck:cli
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: San Dang sdang@nvidia.com

Summary by CodeRabbit

  • Bug Fixes
    • Improved Ollama authentication proxy recovery when switching between compatible backends.
    • Preserved existing proxy tokens when available, helping maintain reliable authentication across restarts and provider changes.
    • Ensured no-auth connections receive a fresh token while using the specified endpoint.

Signed-off-by: San Dang <sdang@nvidia.com>
@sandl99 sandl99 added area: inference Inference routing, serving, model selection, or outputs bug-fix PR fixes a bug or regression platform: linux Affects non-Ubuntu Linux environments labels Jul 27, 2026
@sandl99 sandl99 self-assigned this Jul 27, 2026
@copy-pr-bot

copy-pr-bot Bot commented Jul 27, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Ollama proxy startup now separates token generation from launch, reuses persisted tokens when available, and defers persistence until provider confirmation. No-auth startup passes an explicit token and backend origin. Recovery tests now verify committed-token preservation during backend switching.

Changes

Ollama proxy token flow

Layer / File(s) Summary
Token generation and startup wiring
src/lib/inference/ollama/proxy.ts
Adds explicit token generation and token-aware startup, reuses persisted tokens, defers persistence, and passes endpoint origins for no-auth proxy startup.
Backend switching recovery validation
test/ollama-proxy-recovery.test.ts
Renames the switching scenario and expects the committed compatible-backend token to remain active.

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

Possibly related PRs

  • NVIDIA/NemoClaw#7427: Related Ollama compatible-endpoint routing changes that also affect proxy token creation and usage.

Suggested reviewers: brandonpelfrey, cv, ericksoa

🚥 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: preserving the Ollama proxy token during re-onboarding.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-ollama-token-reonboard

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

@github-code-quality

github-code-quality Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit d38b88a in the codex/fix-ollama-tok... branch remains at 96%, unchanged from commit 77e6e38 in the main branch.

TypeScript / code-coverage/cli

The overall coverage in commit d38b88a in the codex/fix-ollama-tok... branch is 80%. The coverage in commit 77e6e38 in the main branch is 81%.

Show a code coverage summary of the most impacted files.
File main 77e6e38 codex/fix-ollama-tok... d38b88a +/-
src/lib/actions...time-command.ts 100% 82% -18%
src/lib/actions...-add-restart.ts 19% 10% -9%
src/lib/actions...on-readiness.ts 100% 91% -9%
src/lib/actions...lution-probe.ts 95% 88% -7%
src/lib/actions...x/mcp-bridge.ts 41% 35% -6%
src/lib/state/o...oint-migrate.ts 96% 92% -4%
src/lib/actions...e-validation.ts 84% 81% -3%
src/lib/onboard...shboard-port.ts 93% 90% -3%
src/lib/actions...dbox/destroy.ts 95% 93% -2%
src/lib/onboard...eway-service.ts 82% 81% -1%

Updated July 27, 2026 07:44 UTC

@sandl99

sandl99 commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict

PASS. The PR is safe to merge from a security perspective. It preserves the already committed, host-managed Ollama proxy credential during local re-onboarding while retaining cryptographically random fresh credentials for temporary compatible-endpoint replacement and the existing rollback path.

Findings Table

The review found no security findings.

Detailed Analysis

  1. Secrets and Credentials — PASS: No real credentials are committed or logged; test tokens are inert placeholders. The production path reads the existing protected token file.
  2. Input Validation and Data Sanitization — PASS: No new user-controlled input or parsing is introduced.
  3. Authentication and Authorization — PASS: The authenticated proxy boundary remains enforced; re-onboarding now avoids host/sandbox credential divergence.
  4. Dependencies and Third-Party Libraries — PASS: No dependency changes.
  5. Error Handling and Logging — PASS: Token values are not added to diagnostics or logs.
  6. Cryptography and Data Protection — PASS: Fresh temporary credentials continue to use crypto.randomBytes(24); persisted-token file permissions and lifecycle are unchanged.
  7. Configuration and Security Headers — PASS: No ports, container privileges, policies, or HTTP security configuration change.
  8. Security Testing — PASS: The updated recovery test protects committed-token continuity, while existing startup and rollback tests verify fresh temporary tokens and restoration of committed state.
  9. System Security — PASS: The change narrows token selection by current consumer and does not weaken process ownership, health probes, file permissions, or rollback controls.

Files Reviewed

  • src/lib/inference/ollama/proxy.ts
  • test/ollama-proxy-recovery.test.ts

Reviewed head: da4e66e78507703434fb182c0888279c5f07843a

@sandl99
sandl99 marked this pull request as ready for review July 27, 2026 07:04

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/lib/inference/ollama/proxy.ts`:
- Around line 331-336: Update startOllamaAuthProxy so it no longer directly
calls loadPersistedProxyToken; route persisted-token retrieval through an
existing state-layer reader or inject that reader into the inference helper.
Keep token selection and startOllamaAuthProxyWithToken invocation in this
module, while ensuring persisted local/session file access remains owned by
src/lib/state.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 73bb2467-449c-481b-b9a5-4555f1716a88

📥 Commits

Reviewing files that changed from the base of the PR and between 87270de and da4e66e.

📒 Files selected for processing (2)
  • src/lib/inference/ollama/proxy.ts
  • test/ollama-proxy-recovery.test.ts

Comment thread src/lib/inference/ollama/proxy.ts
@github-actions

github-actions Bot commented Jul 27, 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 match; 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: inference-routing, network-policy

1 optional E2E recommendation
  • ollama-auth-proxy

Workflow run details

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

@sandl99 sandl99 changed the title fix(ollama): preserve proxy token during re-onboard e2e(ollama): preserve proxy token during re-onboard Jul 27, 2026
@sandl99 sandl99 changed the title e2e(ollama): preserve proxy token during re-onboard test(ollama): preserve proxy token during re-onboard Jul 27, 2026
@prekshivyas
prekshivyas merged commit 921e1b0 into main Jul 27, 2026
76 of 79 checks passed
@prekshivyas
prekshivyas deleted the codex/fix-ollama-token-reonboard branch July 27, 2026 08:12
@cv cv mentioned this pull request Jul 27, 2026
23 tasks
cv added a commit that referenced this pull request Jul 27, 2026
<!-- markdownlint-disable MD041 -->
## Summary

`docs/changelog/2026-07-25.mdx` now includes the user-facing fixes that
merged after #7607 and before the v0.0.96 tag.
The follow-up covers safer bulk backup and clone restore behavior,
policy and inference repairs, cleaner onboarding diagnostics, and
OpenClaw base-image validation while leaving test-only and
maintainer-internal merges out of the release entry.

## Changes

- Document the Shields-safe `backup-all` flow from #7557 and the
clone-specific restore pairing publication from #7608.
- Record the Claude Code resolved-launcher policy repair from #7581,
Hermes namespaced-model handling from #7604, and persisted Ollama
proxy-token reuse from #7620.
- Record OpenClaw immutable base-inventory validation from #7606, hidden
route-only reservations from #7621, and clean invalid gateway-management
errors from #7630.
- Link the gateway lifecycle and snapshot authorities, retain #7622's
already-merged Docker Engine wording, and exclude internal or test-only
merges from the release entry.

## 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
- [ ] Existing tests cover changed behavior — justification:
- [x] Tests not applicable — justification: This PR changes
release-entry prose only. The changelog contract test and Fern
validation cover the dated entry, published routes, and rendering
requirements.
- [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: At exact PR head `29316da26`, a Codex Desktop documentation
writer reviewed `docs/changelog/2026-07-25.mdx` against `AGENTS.md`,
`WRITING.md`, and `docs/CONTRIBUTING.md`. The review confirmed that the
full entry accurately reflects the merged user-visible behavior, retains
#7622's existing wording, appropriately excludes internal and test-only
PRs, and uses conforming terminology, structure, links, and release
classification. It also confirmed that the review follow-ups use active
third-person release-entry voice, name the actor and recovery
requirement directly, and accurately preserve the trusted-backup,
cached-release refresh, and local-build fallback constraints. The
changelog test passed 6/6, and the docs build completed with 0 errors
and 2 pre-existing hidden warnings.
- Agent: Codex Desktop
<!-- docs-review-head-sha: 29316da -->
<!-- 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 — command/result or justification: `npx
vitest run test/changelog-docs.test.ts` passed 6/6 tests after the final
review fix.
- [ ] 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 change.
- [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) — exited
0 with 0 errors and 2 pre-existing hidden warnings after the final
review fix.
- [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)

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


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

## Summary by CodeRabbit

* **Documentation**
* Expanded the changelog to clarify persistent `policy exclude`/`policy
restore` behavior across rebuilds and snapshot restores, including
reporting on removed endpoints and exclusion consistency.
* Updated `claude-code` preset guidance to allow the npm-installed
OpenShell launcher path while maintaining endpoint/HTTP method scope.
* Documented hardened handling for invalid gateway-management
declarations, improved gateway/agent-version diagnostics scope, and
clarified onboarding/restore credential and reasoning precedence.
* Tightened bulk backup/restore guidance (safety windows, approval
limits, and failure recovery) and refined OpenClaw base selection to
avoid incompatible cached releases and `:latest` fallback.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
jyaunches added a commit that referenced this pull request Jul 29, 2026
<!-- markdownlint-disable MD041 -->
## Summary

This PR corrects post-tag v0.0.96 documentation drift for exact-loopback
no-auth compatible endpoints and local Ollama onboarding.
It also aligns the Windows changelog link label and source-only
dependency review headings with their canonical titles and writing
rules.

## Changes

- Distinguish OpenAI-compatible exact-loopback no-auth mode from the
Anthropic-compatible non-empty placeholder requirement from #7427.
- Document the Ollama menu download size, approximate VRAM requirement,
and available or total GPU memory fields from #7482.
- Document persisted Ollama proxy-token reuse during re-onboarding from
#7620.
- Match the Windows changelog link label to the target page title and
remove colons from the source-only dependency review headings.

## 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
<!-- Check one tests line and one docs line. Check other lines when
applicable. Add every requested justification or approval reference. -->
- [ ] Tests added or updated for changed behavior
- [x] Existing tests cover changed behavior — justification: Source
tests from #7427, #7482, and #7620 protect the shipped behavior, and 21
changelog and link tests pass for this documentation diff.
- [ ] 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
<!-- Required for code and documentation changes after the changes and
applicable validation are complete. Keep one review checkbox and one
instance of each visible or hidden field. For Evidence, list changed
documentation paths. For documentation-only changes, also state that the
writing rules and documentation style were reviewed. For other results,
explain why no documentation change is needed or why the review is
blocked. For Agent, use a consistent product and surface name, such as
Codex Desktop, Codex CLI, Claude Code, or Cursor. After committing all
review changes, put `git rev-parse --short HEAD` and `git rev-parse
--short HEAD:AGENTS.md` in the hidden metadata below. Rerun the review
and refresh that metadata after any new commit. This receipt is advisory
during the data-collection pilot. -->
- [x] Documentation writer subagent reviewed the completed changes
- Result: `docs-updated`
- Evidence: Reviewed `docs/changelog/2026-07-25.mdx`,
`docs/inference/custom-endpoint-security.mdx`,
`docs/inference/set-up-ollama.mdx`, and
`docs/security/sandbox-base-2026-07-25-dependency-review.md` against
`docs/CONTRIBUTING.md` and `WRITING.md`. The exact-head review found no
remaining findings.
- Agent: Codex Desktop
<!-- docs-review-head-sha: 5d103d9 -->
<!-- docs-review-agents-blob-sha: be20a09 -->

## DGX Station Hardware Evidence
<!-- Required only when scripts/prepare-dgx-station-host.sh changes.
Maintainers must review the linked evidence before approving or merging.
This is human-reviewed evidence, not authenticated hardware provenance.
Exceptional bypasses use existing repository governance and must be
documented on the PR. -->
- [ ] Tested on DGX Station
- Tested commit: not applicable
- Station profile/scenario: not applicable
- Result: not applicable
- Supporting evidence: not applicable

## Verification
<!-- Check each applicable item only when supported by the requested
evidence. Run targeted tests once per relevant change set and rerun
after later edits or hook autofixes that can affect the tested behavior.
Do not rerun hook-covered checks. -->
- [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 test/check-docs-links.test.ts` passed 21
tests.
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — not run because this is a focused
documentation-only change.
- [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
command passed with 0 errors and two Fern warnings.
- [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)

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>


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

- **Documentation**
- Clarified authentication requirements for OpenAI- and
Anthropic-compatible endpoints, including options for unauthenticated
local services.
- Updated Ollama onboarding to show download size and VRAM needs,
explain GPU memory reporting behavior, and clarify persisted proxy token
reuse.
- Refreshed Windows setup link text and improved sandbox dependency
review heading formatting for consistency.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Co-authored-by: Julie Yaunches <jyaunches@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: inference Inference routing, serving, model selection, or outputs bug-fix PR fixes a bug or regression platform: linux Affects non-Ubuntu Linux environments

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants