Skip to content

fix(inference): use completion token limits for GPT-6 probes - #12691

Merged
prekshivyas merged 8 commits into
mainfrom
fix/gpt-6-probe-token-field
Oct 6, 2026
Merged

prekshivyas merged 8 commits into
mainfrom
fix/gpt-6-probe-token-field

Conversation

@ericksoa

@ericksoa ericksoa commented Oct 6, 2026 •

Copy link
Copy Markdown
Member

Outcome

Host onboarding probes and sandbox smoke checks use max_completion_tokens for GPT-6 models, including gpt-6-astra.

Reason

Azure rejected the probe's max_tokens parameter. The shared resolver recognized GPT-5 and o-series models but omitted GPT-6.

Related issues

Fixes #12690.

Changes

  • Add the GPT-6 prefix and extend existing request-payload tests.
  • Repair the inherited installer test using the existing bootstrap and its already-approved digest. Preserve missing-trust and tampering rejection tests.
  • Pin the MCP SDK to 1.31.0 for GHSA-6qxp-vccf-f47h in the affected mcporter, OpenClaw, messaging, and MCP-discovery graphs. Align the reviewed archive audit with those runtime overrides. Regenerate the shipped discovery bundle and update the required lock digests. Align the two OpenClaw SDK dependency edges with their explicit 1.31.0 overrides so the offline cache resolver uses the installed version.

Verification

  • Focused resolver, host-probe, and sandbox-smoke tests: 78 passed, 1 skipped. Only two GPT-6 additions remain, checking the actual host and sandbox request payloads.
  • npx --no-install vitest run --project integration test/install/installer-hash-check.test.ts: 84 passed with the fixture removal.
  • Mcporter supply-chain and audit-policy tests: 10 passed.
  • Mcporter production audit: zero high/critical findings; 120 registry signatures verified, 15 attestations verified.
  • Real mcporter initialize, list-tools, call-tool, and close smoke test: passed with SDK 1.31.0.
  • CLI type check and remaining commit hooks: passed. The local Pi receipt guard requires refreshed validation artifacts; those are outside the authorized scope. Published under maintainer direction with that gate unresolved. Fresh GitHub CI is pending.
  • OpenClaw and MCP-discovery production audits: zero high/critical findings.
  • Discovery runtime tests: 3 passed. Reviewed audit tests passed; all 14 locked-install tests pass after updating the required digest.
  • Linux MCP cache materialization and integrity verification: 97 locked archives validated.
  • Messaging offline cache resolver: 419 archives resolved for AMD64 and ARM64.
  • Focused cache-materialization and locked-install suites: 34 passed after the dependency-edge correction.
  • No secrets, API keys, or credentials in the diff.

Review notes

Self-review of d346da9130fa348754c5ec2598e23250b134750e covered the inference/onboarding changes, installer test correction, and mcporter SDK patch. Installer parser rules are unchanged; dependency records bind the updated locks. Independent review is pending; the PR remains draft.


Signed-off-by: Aaron Erickson aerickson@nvidia.com

Summary by CodeRabbit

  • New Features
    • GPT-6 models now use the appropriate completion-token setting, consistent with GPT-5 and reasoning models.
  • Security
    • Updated the MCP SDK to version 1.31.0 across relevant runtimes.
    • Refreshed verified runtime integrity checks and security review records.
  • Bug Fixes
    • Strengthened validation of the installer’s npm cleanup template and rejection of unsafe modifications.

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@ericksoa ericksoa self-assigned this Oct 6, 2026
@copy-pr-bot

copy-pr-bot Bot commented Oct 6, 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 Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: NVIDIA/NemoClaw/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: 5eb74918-8f82-444a-9006-7ae38e0aa03d
📥 Commits

Reviewing files that changed from the base of the PR and between 1a245e4 and 927f270.

⛔ Files ignored due to path filters (4)
  • agents/openclaw/managed-image-messaging-runtime/package-lock.json is excluded by !**/package-lock.json
  • agents/openclaw/mcporter-runtime/package-lock.json is excluded by !**/package-lock.json
  • agents/openclaw/openclaw-runtime/package-lock.json is excluded by !**/package-lock.json
  • tools/mcp-tool-discovery-runtime/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (17)
  • Dockerfile
  • Dockerfile.base
  • agents/openclaw/dependency-review.md
  • agents/openclaw/managed-image-messaging-runtime/package.json
  • agents/openclaw/mcporter-runtime/package.json
  • agents/openclaw/openclaw-runtime/package.json
  • ci/reviewed-npm-audit.json
  • scripts/audit-reviewed-npm-graph.mts
  • src/lib/inference/max-tokens-field.ts
  • src/lib/inference/onboard-probes.test.ts
  • src/lib/onboard/compatible-endpoint-smoke-gpt5.test.ts
  • test/agents/openclaw/openclaw-locked-install.test.ts
  • test/install/installer-hash-check.test.ts
  • tools/mcp-tool-discovery-runtime/package.json
  • tools/mcp-tool-discovery-runtime/reviewed-runtime-bundle/mcp-tool-discovery/BUNDLED_PACKAGES.json
  • tools/mcp-tool-discovery-runtime/reviewed-runtime-bundle/mcp-tool-discovery/THIRD_PARTY_LICENSES.txt
  • tools/mcp-tool-discovery-runtime/reviewed-runtime-bundle/mcp-tool-discovery/mcp-tool-discovery.bundle

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.


📝 Walkthrough

Walkthrough

The change updates OpenClaw and MCP SDK dependency pins and their reviewed lockfile records, adds GPT-6 to token-field selection and onboarding tests, and revises installer tests for the canonical npm cleanup template.

Changes

SDK runtime pins and verification

Layer / File(s) Summary
SDK pins and graph configuration
agents/openclaw/*-runtime/package.json, tools/mcp-tool-discovery-runtime/package.json, scripts/audit-reviewed-npm-graph.mts, tools/mcp-tool-discovery-runtime/reviewed-runtime-bundle/...
Runtime overrides pin @modelcontextprotocol/sdk to 1.31.0. The archive graph manifest adds overrides for that SDK and proxy-addr. The bundled package and license records identify SDK version 1.31.0.
Reviewed runtime state and hash checks
agents/openclaw/dependency-review.md, ci/reviewed-npm-audit.json, Dockerfile, Dockerfile.base, test/agents/openclaw/openclaw-locked-install.test.ts
The dependency review record updates its audit details and requires the exact SDK version. Reviewed lockfile records and Docker checks use updated hashes. The locked-install test expects the updated OpenClaw digest.

GPT-6 token-field handling

Layer / File(s) Summary
GPT-6 resolution and probe coverage
src/lib/inference/max-tokens-field.ts, src/lib/inference/onboard-probes.test.ts, src/lib/onboard/compatible-endpoint-smoke-gpt5.test.ts
The resolver selects max_completion_tokens for GPT-6 model IDs. Probe and smoke tests add gpt-6-astra and expect that field instead of max_tokens.

npm cleanup template trust

Layer / File(s) Summary
Canonical template validation
test/install/installer-hash-check.test.ts
The hash tests validate the canonical Brev template against its expected digest. They also check rejection of a mismatched digest and altered templates that broaden deletion or bypass checksums.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: cv

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The installer hash-check test changes and the MCP SDK 1.31.0 security updates do not implement or test the GPT-6 token-field behavior in [#12690]. The SDK pins, lock and audit updates, and regenerated… Move the installer test correction and MCP SDK security updates, including their lock, audit, digest, and bundle changes, to separate PRs. Keep the GPT-6 resolver and request-payload changes in this PR.
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 6 files. (10 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Issue [#12690] requires GPT-6 model IDs to use max_completion_tokens in the shared resolver, host probes, and sandbox smoke requests. src/lib/inference/max-tokens-field.ts adds the GPT-6 prefix. T…
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: using completion-token limits for GPT-6 probes.
Full details: Out of Scope Changes check

Explanation

The installer hash-check test changes and the MCP SDK 1.31.0 security updates do not implement or test the GPT-6 token-field behavior in [#12690]. The SDK pins, lock and audit updates, and regenerated discovery bundle form a separate dependency-security change. The installer fixture correction is also unrelated to the issue.

Full details: Docstring Coverage

Explanation

Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 6 files. (10 skipped: 10 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@github-code-quality

github-code-quality Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall line coverage in commit 927f270 in the fix/gpt-6-probe-toke... branch is 97%. The line coverage in commit 63002cd in the main branch is 96%.

Show a line coverage summary of the most impacted files.
File main 63002cd fix/gpt-6-probe-toke... 927f270 +/-
nemoclaw/src/onboard/config.ts 98% 96% -2%
nemoclaw/src/index.ts 94% 93% -1%
nemoclaw/src/bl...t-management.ts 100% 100% 0%
nemoclaw/src/co.../config-show.ts 100% 100% 0%
nemoclaw/src/commands/slash.ts 100% 100% 0%
nemoclaw/src/on...native-route.ts 0% 100% +100%

Updated October 06, 2026 18:05 UTC

@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor finished for commit 573cd18. Include the Advisor findings in the complete PR feedback collection. Verify and group valid findings before repair.

Request review only when Require no Advisor blockers is green.

All previous runs

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@ericksoa
ericksoa marked this pull request as ready for review October 6, 2026 17:47
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@prekshivyas
prekshivyas enabled auto-merge (squash) October 6, 2026 18:18
@prekshivyas
prekshivyas merged commit aa9b06d into main Oct 6, 2026
65 of 68 checks passed
@prekshivyas
prekshivyas deleted the fix/gpt-6-probe-token-field branch October 6, 2026 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Azure GPT-6 onboarding fails because inference probes send max_tokens

3 participants