Skip to content

fix(occ): accept nested-slash model IDs - #875

Merged
freeqaz merged 3 commits into
openclaw:mainfrom
ericcurtin:fix/813-nested-model-ids
Oct 3, 2026
Merged

freeqaz merged 3 commits into
openclaw:mainfrom
ericcurtin:fix/813-nested-model-ids

Conversation

@ericcurtin

@ericcurtin ericcurtin commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Model IDs with slashes (openai/vendor/model) fail admission. Fixes #813.

Change

Split model refs at the first slash with one shared helper.

Verification

  • configured-harness.ts run alone with the issue config: fails on main with the reported error, passes now.
  • Added cases to configuration-occ and kubernetes-compute conformance tests.
  • Reported by IsaiahStapleton on a local k3d deployment (cherry-picked onto main): revision admitted, full three-segment ID reaches the provider with HTTP 200. Both conformance files: 229 pass. eslint, prettier on changed files, and pnpm typecheck: pass.
  • Not run: full pnpm test:conformance, repo-wide pnpm lint (no installed deps here).

Risks

Low. A catalog ID of only the first segment (vendor) is no longer accepted.

@clawsweeper

clawsweeper Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review blocked

Automated review did not run, so no review verdict was produced.

Reason: The input-safety check rejected material in this revision. No detected value, path, or scanner output is reproduced here.

Automatic review is on hold, including after source changes. Request a fresh re-review after resolving the failure; maintainers can also release the hold through an intentional scanner-policy update.

Next step: If this is a genuine credential, remove and rotate it. If it is an intentional test fixture, a maintainer must review and qualify it.

View the workflow run.

@ericcurtin

Copy link
Copy Markdown
Contributor Author

If you work with OpenRouter-style model IDs like these, you might also like to try https://github.com/llmmanorg/llmman. Thanks!

@ericcurtin

Copy link
Copy Markdown
Contributor Author

CI is waiting for workflow approval. Could a maintainer approve the run? Thanks.

@ericcurtin

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
Exact review queued.

Re-review progress:

@IsaiahStapleton

Copy link
Copy Markdown
Contributor

Tested a6c4c021 cherry-picked onto current main (a10baed3). It applied cleanly.

Real deployment: local k3d, Kubernetes Compute, embedded OpenClaw, model ref anthropic/rits/zai-org/glm-5-3 with models.providers.anthropic.baseUrl pointing at an Anthropic-dialect model gateway.

  • main: resolveConfiguredHarnessId rejects it with Configured selectable provider models must match the primary model. The API returns the same error at deploy for openai/rits/zai-org/glm-5-3.
  • With this commit: the revision is admitted, the gateway gets OPENCLAW_HARNESS_MODEL=anthropic/rits/zai-org/glm-5-3, and the provider request returns HTTP 200 for the full three-segment ID.

Checks listed as not run, on that tree:

  • tests/conformance/configuration-occ.test.mjs and kubernetes-compute.test.mjs: 229 pass, 0 fail.
  • eslint and prettier on the changed files: clean.
  • pnpm typecheck: pass.
  • Not run: full pnpm test:conformance, repo-wide pnpm lint.

Getting that reasoning model to an active revision also needed a larger startup probe token cap, which is unrelated to this change: #920.

@ericcurtin
ericcurtin marked this pull request as ready for review October 3, 2026 01:42
@ericcurtin
ericcurtin requested a review from a team as a code owner October 3, 2026 01:42
@freeqaz

freeqaz commented Oct 3, 2026

Copy link
Copy Markdown
Collaborator

Free's clanker here (an AI agent reviewing on Free's behalf).

Checked head a6c4c02 against main 10a9756 (the merge is clean). The code change is right: splitModelRef splits at the first slash, which matches upstream OpenClaw's model-ref parsing, and every model-ref split site on main is converted (configured-harness.ts, plus kubernetes/index.ts lines 1381 and 2319 in the merged tree). Nothing landed on main since this branch's base touches that logic. On the merged tree tests/conformance/configuration-occ.test.mjs, tests/conformance/utils.test.mjs and pnpm typecheck pass. The new utils export sits fine next to hasControlCharacter, which #946 added today.

Verdict: needs one small change, then it is ready for a maintainer to merge once CI Required has run. It has not run on this fork head yet.

  1. docs/reference/harness-execution.md:70-74 takes the page over the docs hard limit. With main, node scripts/docs-site/word-count.mjs reports 2516 words against the 2500-word limit (main is at 2495; this hunk adds about 21). The docs:check-length step in CI Required will fail as soon as CI runs. Fix: shorten the new sentence (for example "Each entry's id is the full reference or the ID after its first slash; IDs may contain slashes.") and trim about 15 more words elsewhere on the page. The "Harness authentication" section, at about 1000 words, has the most room. Note that fix(runtime): give the startup model probe room for reasoning models #920 also adds 3 words to this page, so whichever lands second needs the trim to cover both.

Ordering: this should land before #913 and #923. #913 rewrites the same catalog-id comparison in packages/occ/src/configured-harness.ts, and the two conflict textually. This version is the complete one, because #913 leaves kubernetes/index.ts on split("/", 2). #923 advertises multi-segment model IDs that fail deployment on main without this fix.

@freeqaz

freeqaz commented Oct 3, 2026

Copy link
Copy Markdown
Collaborator

Free's clanker here (an AI agent reviewing on Free's behalf).

CI follow-up for run 36945652675: the only failing job is Checks and Conformance, in the docs length check (pnpm docs:check-length):

1 Markdown file exceeds the 2500-word hard limit:
- docs/reference/harness-execution.md: 2521 words

The code and tests pass. That run merged this branch into an older main (7a6cc93). Against current main (04d01d0) the page is 2495 words and this PR brings it to 2516, so it still fails after a sync.

What to do: merge current main, then trim at least 16 words from docs/reference/harness-execution.md. You could tighten the new sentence near line 70, plus a sentence or two in "Harness authentication" (1022 words, the longest section). Check with pnpm docs:check-length. #920 adds 3 words to the same page, so if it lands first you'll need to trim at least 19.

@ericcurtin

Copy link
Copy Markdown
Contributor Author

Merged main and trimmed harness-execution.md by about 20 words to clear docs:check-length. Thanks for the reviews.

@freeqaz
freeqaz merged commit 04f2f51 into openclaw:main Oct 3, 2026
30 checks passed
@freeqaz

freeqaz commented Oct 3, 2026

Copy link
Copy Markdown
Collaborator

Free's clanker here (an AI agent reviewing on Free's behalf).

Landed as 04f2f51 with Free's authorization, after an independent review of head 23a3f53. CI Required passed on that head, and the merge with main 708ebb4 was clean.

What the review checked:

  • splitModelRef matches every other model-ref split in the repo: the console's create and harness-auth code, the runtime probe's --model argument, and the three Kubernetes driver sites. All of them now split at the first slash.
  • The model ref only reaches an environment variable value (OPENCLAW_HARNESS_MODEL). It is never used in a label, annotation or env var name, so slashes are safe there.
  • Leading and trailing slashes are still rejected by configuredModels.
  • The one behavior change is the one listed under Risks: a catalog entry whose id is only the first segment (vendor) no longer matches openai/vendor/model. The console already writes the full remainder as the catalog id.
  • configuration-occ and utils conformance tests pass 26/26, on the head and on a local merge with current main (which reworded the unqualified-model error in the same file). kubernetes-compute passes 213/213.

Thanks for the fix and the docs trim. Next: #913 needs to drop its overlapping catalog-id edits in configured-harness.ts.

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.

Nested-slash model IDs are rejected in selectable provider catalogs

3 participants