Classify hooks settings as internal so they never reach API requests (Fixes #3218)

[acoliver]

TLDR

Fixes a model-config leak where hooksConfig (and hooks) — llxprt-internal hooks-system configuration — was serialized into outbound API request bodies, causing backends to reject requests with 400 Unsupported parameter: hooksConfig. The root cause is a single missing classification in the settings registry: unknown keys default to the modelParams pass-through bucket and flow to the wire.

Dive Deeper

Root cause

When settings flow from the global settings store into the runtime invocation context, they pass through separateSettings() (settings package). This function categorizes each key into one of four buckets: cliSettings, modelBehavior, modelParams, or customHeaders. Keys not registered in SETTINGS_REGISTRY and not in INTERNAL_SETTINGS_KEYS fall through to the unknown-key default: modelParams (pass-through to API).

hooksConfig (the hooks-system toggle/notifications/disabled config) and hooks (hook event definitions) were never registered in the settings package's SETTINGS_REGISTRY. They are defined in the CLI's settings schema (for the dialog UI) but that schema is separate from the settings package's runtime categorization registry. So both keys were treated as unknown and routed to modelParams.

The data flow:

1. Global settings (including hooksConfig) are loaded into the ephemerals snapshot via buildEphemeralsSnapshot()
2. separateSettings() categorizes the snapshot — hooksConfig falls through to modelParams because it has no spec
3. modelParams is spread into the request body by the provider's request builder
4. The API rejects the unknown hooksConfig parameter with HTTP 400

Fix

Add hooksConfig and hooks to INTERNAL_SETTINGS_KEYS in packages/settings/src/settings/settingsRegistry.ts. This is the same classification used for the tools object — another llxprt-internal config that must never reach the API. Both keys now route to cliSettings instead of modelParams, keeping them available to the CLI via context.getCliSetting() while preventing serialization onto any request body.

This fixes the leak for all providers (Codex/OpenAI Responses, OpenAI Chat Completions, Anthropic, Gemini), not just the Codex path where it was observed.

Files changed

• packages/settings/src/settings/settingsRegistry.ts — Add hooksConfig and hooks to INTERNAL_SETTINGS_KEYS
• packages/settings/src/__tests__/settingsRegistry.test.ts — 4 behavioral tests proving both keys route to cliSettings, not modelParams, across providers
• packages/providers/src/openai-responses/__tests__/OpenAIResponsesProvider.hooksConfigLeak.test.ts — 3 end-to-end provider tests proving hooksConfig/hooks do not appear in the Codex and non-Codex Responses request bodies

Reviewer Test Plan

1. Settings-level: Run bun test packages/settings/src/__tests__/settingsRegistry.test.ts and verify the 4 new tests under "hooks system config never leaks into modelParams" pass.

2. Provider-level: Run bun test packages/providers/src/openai-responses/__tests__/OpenAIResponsesProvider.hooksConfigLeak.test.ts and verify the 3 tests pass. These tests construct a real OpenAIResponsesProvider in Codex mode, inject hooksConfig into the ephemerals snapshot, capture the fetch request body, and assert hooksConfig is absent.

3. Regression verification: Temporarily revert the settingsRegistry.ts change and re-run the provider tests — they will fail with hooksConfig present in the request body, confirming the tests are genuine regression guards.

Testing Matrix

	🍏	🪟	🐧
npm run	✅	❓	❓
npx	✅	❓	❓
Docker	❓	❓	❓
Podman	❓	-	-
Seatbelt	❓	-	-

Verified on macOS: typecheck, lint, eslint guard, format, build, smoke test, settings + providers + core workspace test suites all pass.

Linked issues / bugs

Fixes #3218

Summary by CodeRabbit

• Bug Fixes

  • Prevented internal hook settings from being sent in OpenAI and Codex model requests.
  • Preserved legitimate model parameters during request processing.
  • Improved settings classification so hook configuration remains application-level rather than affecting model behavior.

• Tests

  • Added regression coverage for request serialization and settings handling across supported OpenAI providers.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 95aa7175-fc74-4461-bd22-735ed907a678

📥 Commits

Reviewing files that changed from the base of the PR and between 81a1365 and 859ed88.

📒 Files selected for processing (3)

• packages/providers/src/openai-responses/__tests__/OpenAIResponsesProvider.hooksConfigLeak.test.ts
• packages/settings/src/__tests__/settingsRegistry.test.ts
• packages/settings/src/settings/settingsRegistry.ts

---

📝 Walkthrough

Walkthrough

Changes

Hooks configuration boundary

Layer / File(s)	Summary
Classify hook settings as internal packages/settings/src/settings/settingsRegistry.ts, packages/settings/src/__tests__/settingsRegistry.test.ts	hooksConfig and hooks now route to cliSettings instead of modelParams. Tests cover Codex and OpenAI behavior.
Verify provider request serialization packages/providers/src/openai-responses/__tests__/OpenAIResponsesProvider.hooksConfigLeak.test.ts	Regression tests capture request bodies and verify that hook settings are omitted while valid model parameters remain serialized.

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

Mergeability Score: ⚪ Minimal · up to 859ed

The change keeps internal hooks settings out of outbound API requests while preserving CLI access, and no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main change and references issue #3218.
Description check	✅ Passed	The description explains the root cause, fix, tests, testing matrix, and linked issue.
Linked Issues check	✅ Passed	The changes address issue #3218 by preventing internal hooks settings from reaching API requests.
Out of Scope Changes check	✅ Passed	All production and test changes support the linked issue and stated objectives.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ 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 issue3218

---

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

[github-actions]

Walkthrough

Before this change, hooks-related configuration could still be classified as a regular setting and therefore travel through separateSettings() into modelParams, which meant it could be copied into outbound provider request payloads. After the PR, hooksConfig and hooks are marked as internal settings, so the settings registry keeps them out of modelParams and prevents them from leaking into API request bodies.

Release Notes

New Features

• Added an internal classification path for hooks-related settings so they are consistently treated as non-model settings.

Bug Fixes

• Fixed hooks configuration leaking into modelParams and outbound provider requests by classifying hooksConfig and hooks as internal settings.

Tests

• Added regression coverage for hooks settings so they are verified not to appear in modelParams or provider request payloads.

Refactor

• Tightened separateSettings() behavior by routing internal-only settings away from API-facing parameter buckets.

Changes

Layer	File(s)	Summary
core	packages/settings/src/settings/settingsRegistry.ts	Classifies hooksConfig and hooks as internal settings so separateSettings() excludes them from API request bodies.
tests	packages/settings/src/tests/settingsRegistry.test.ts, packages/providers/src/openai-responses/tests/OpenAIResponsesProvider.hooksConfigLeak.test.ts	Regression tests ensuring hooks configuration never leaks into modelParams or outbound provider request payloads.

Sequence Diagram

sequenceDiagram
  participant Config as Settings Config
  participant Registry as Settings Registry
  participant Separator as separateSettings()
  participant Provider as Provider Request Builder
  participant API as Outbound API

  Config->>Registry: Provide settings including hooksConfig and hooks
  Registry->>Registry: Classify hooksConfig and hooks as internal settings
  Registry->>Separator: Pass settings with internal classification
  Separator->>Separator: Route internal keys to cliSettings bucket
  Separator->>Separator: Exclude internal keys from modelParams
  Separator->>Provider: Return SeparatedSettings without hooks in modelParams
  Provider->>Provider: Build request body from modelParams only
  Provider->>API: Send API request without hooks configuration

Loading

Magnitude

🎯 1 (S)
258 additions, 0 deletions, 3 changed files across 2 packages, 0 acceptance criteria

Related

No related items found.

Pre-merge Checks

Check	Status	Note
Title	✅	Descriptive title that clearly states the fix mechanism (classify as internal) and outcome (never reach API requests), with proper issue reference.
Description	✅	All required sections present and substantive: TLDR summarizes the fix; Dive Deeper explains root cause and data flow; Reviewer Test Plan provides concrete verification steps; Testing Matrix covers platforms; Linked issues / bugs references #3218.
Linked Issues	✅	Fully addresses #3218. The 400 error from hooksConfig being sent to the API is fixed by adding hooksConfig and hooks to INTERNAL_SETTINGS_KEYS, preventing them from routing to modelParams. Tests verify both settings classification and provider request body serialization.
Out of Scope	—	Testing matrix shows only macOS verified; Windows, Linux, Docker, Podman, and Seatbelt remain untested. The PR description claims verification of typecheck, lint, build, and core workspace tests, but these artifacts are not included in the actual code changes.

---

Walkthrough generated by LLxprt PR Review. Planner issue: #2256

[github-actions]

OpenCodeReview — PR #3223

• Reviewed head SHA: 859ed889ed239b06479329e6357f01c00e0b221e
• Merge base: 81a13658bc070e74667b6346f3be912bf4a72cae
• Range: full from 81a13658bc070e74667b6346f3be912bf4a72cae
• Range fallback: none
• Scope: selected 3 file(s), +258/-0; cumulative 3 file(s), +258/-0
• Tokens: 116432 total (100782 input, 15650 output, 48000 cache)
• OCR version: open-code-review v1.8.4 (e78474478) linux/amd64 built at: 2026-08-01T03:27:37Z https://github.com/alibaba/open-code-review
• Phase: review
• Exit code: 0
• Run: https://github.com/vybestack/llxprt-code/actions/runs/31754567877
• No findings.
• Artifacts: ocr-review-output contains raw JSON, stdout, stderr, preview, phase, and exit-code diagnostics.
• WARNING: Changed-file coverage 0/3 preview files covered is below the 90% threshold.