Skip to content

fix: Issue 67056 image runtime headers#67326

Open
sahilsatralkar wants to merge 4 commits intoopenclaw:mainfrom
sahilsatralkar:issue-67056-image-runtime-headers
Open

fix: Issue 67056 image runtime headers#67326
sahilsatralkar wants to merge 4 commits intoopenclaw:mainfrom
sahilsatralkar:issue-67056-image-runtime-headers

Conversation

@sahilsatralkar
Copy link
Copy Markdown
Contributor

@sahilsatralkar sahilsatralkar commented Apr 15, 2026

Summary

Describe the problem and fix in 2–5 bullets:

If this PR fixes a plugin beta-release blocker, title it fix(<plugin-id>): beta blocker - <summary> and link the matching Beta blocker: <plugin-name> - <summary> issue labeled beta-blocker. Contributors cannot label PRs, so the title is the PR-side signal for maintainers and automation.

  • Problem: Image runtime auth in src/media-understanding/image.ts only used getApiKeyForModel, so provider-level headers from model registry auth were dropped.
  • Why it matters: Custom OpenAI-compatible image models can require provider headers (for routing/auth attribution); dropping headers breaks those requests.
  • What changed: Runtime auth now prefers modelRegistry.getApiKeyAndHeaders(model) when available, falls back to getApiKeyForModel, and forwards resolved headers to complete(...). Added targeted tests for header forwarding + fallback behavior.
  • What did NOT change (scope boundary): No plugin manifest/API contract changes, no new endpoints, no changes to non-image runtime flows.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor required for the fix
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

Root Cause (if applicable)

For bug fixes or regressions, explain why this happened, not just what changed. Otherwise write N/A. If the cause is unclear, write Unknown.

  • Root cause: The image runtime path resolved only API key auth and never consumed modelRegistry.getApiKeyAndHeaders(...).
  • Missing detection / guardrail: No test asserted that image runtime forwards model-registry headers into complete(...).
  • Contributing context (if known): Header-aware auth existed in adjacent runtime paths, but image runtime had not been aligned.

Regression Test Plan (if applicable)

For bug fixes or regressions, name the smallest reliable test coverage that should catch this. Otherwise write N/A.

  • Coverage level that should have caught this:
    • Unit test
    • Seam / integration test
    • End-to-end test
    • Existing coverage already sufficient
    • Target test or file: src/media-understanding/image.test.ts
    • Scenario the test should lock in: When model registry returns { apiKey, headers }, complete(...) receives the same headers; when lookup is unavailable, fallback auth still works and no unexpected headers are injected.
    • Why this is the smallest reliable guardrail: The bug is in local auth-resolution + complete options assembly; unit tests directly validate that seam.
    • Existing test that already covers this (if any): Existing image runtime tests cover generic behavior but not provider-header forwarding.
    • If no new test is added, why not: N/A (new tests added).

User-visible / Behavior Changes

List user-visible changes (including defaults/config).
If none, write None.

Image requests using custom OpenAI-compatible models now preserve provider-level headers from model registry auth, matching expected provider behavior.

Diagram (if applicable)

For UI changes or non-trivial logic flows, include a small ASCII diagram reviewers can scan quickly. Otherwise write N/A.

Before:
[image tool request] -> [resolve API key only] -> [complete without provider headers]

After:
[image tool request] -> [resolve apiKey+headers from model registry (fallback to legacy API key path)] -> [complete with headers] -> [provider accepts request]

Security Impact (required)

  • New permissions/capabilities? (Yes/No) No
  • Secrets/tokens handling changed? (Yes/No) No
  • New/changed network calls? (Yes/No) No
  • Command/tool execution surface changed? (Yes/No) No
  • Data access scope changed? (Yes/No) No
  • If any Yes, explain risk + mitigation: N/A

Repro + Verification

Environment

  • OS: macOS
  • Runtime/container: Node.js + pnpm (local repo workspace)
  • Model/provider: custom OpenAI-compatible image model (mocked in unit test)
  • Integration/channel (if any): media-understanding image tool path
  • Relevant config (redacted): provider headers under model/provider config, API key redacted

Steps

  1. Configure an image-capable custom provider/model with provider-level headers.
  2. Run image description path that calls describeImageWithModel.
  3. Inspect complete(...) call options.

Expected

  • Provider headers from registry auth are forwarded to complete(...).

Actual

  • Before fix: headers missing.
  • After fix: headers forwarded; fallback path still works when registry lookup is unavailable.

Evidence

Attach at least one:

  • Failing test/log before + passing after
  • Trace/log snippets
  • Screenshot/recording
  • Perf numbers (if relevant)

Human Verification (required)

What you personally verified (not just CI), and how:

  • Verified scenarios: New header-forwarding test fails before implementation and passes after; fallback test passes; full image.test.ts and image-tool.test.ts pass.
  • Edge cases checked: Registry auth unavailable fallback; registry auth present with explicit headers.
  • What you did not verify: Live provider calls with real external credentials/endpoints.

Review Conversations

  • I replied to or resolved every bot review conversation I addressed in this PR.
  • I left unresolved only the conversations that still need reviewer or maintainer judgment.

If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers.

Compatibility / Migration

  • Backward compatible? (Yes/No) Yes
  • Config/env changes? (Yes/No) No
  • Migration needed? (Yes/No) No
  • If yes, exact upgrade steps: N/A

Risks and Mitigations

List only real risks for this PR. Add/remove entries as needed. If none, write None.

  • Risk: If registry auth returns headers but no API key and fallback auth fails, request still fails on missing API key.
    • Mitigation: Preserve requireApiKey fail-fast behavior and cover fallback path in tests.

Built with Codex

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 15, 2026

Greptile Summary

This PR fixes image runtime auth to forward provider-level headers from modelRegistry.getApiKeyAndHeaders() into complete(), so custom OpenAI-compatible models with routing/auth headers work correctly. The fix is well-targeted and backed by two new focused tests. It also bundles an unrelated addition to cli-registry-loader.ts that ensures the configured plugins.slots.memory plugin is always included in the primary-command activation scope — this change is not described in the PR description.

Confidence Score: 5/5

Safe to merge; the image auth fix is correct and well-tested, and the cli-registry-loader addition is logically sound.

All findings are P2. The image runtime fix correctly handles all auth resolution paths and is backed by targeted tests. The cli-registry-loader addition is correct but undocumented in the PR description — a transparency concern, not a correctness one.

src/plugins/cli-registry-loader.ts — the bundled memory-slot injection change is unmentioned in the PR description.

Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/plugins/cli-registry-loader.ts
Line: 105-119

Comment:
**Undescribed change bundled with image-headers fix**

`withPrimaryMemorySlotPluginId` and `resolveConfiguredMemorySlotPluginId` are unrelated to the image runtime header bug described in the PR. The PR description, root-cause section, and regression test plan all focus exclusively on `src/media-understanding/image.ts`. These CLI registry changes don't appear to be mentioned anywhere in the PR write-up, making them invisible to reviewers scanning the description. Consider either updating the PR description to cover this change or splitting it into a separate PR.

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "fix(image): preserve provider headers in..." | Re-trigger Greptile

Comment thread src/plugins/cli-registry-loader.ts Outdated
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c179ddd6fe

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/media-understanding/image.ts Outdated
Comment thread src/media-understanding/image.ts Outdated
@sahilsatralkar sahilsatralkar force-pushed the issue-67056-image-runtime-headers branch from c179ddd to acafa16 Compare April 15, 2026 18:34
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: acafa16387

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/media-understanding/image.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Image runtime does not preserve provider headers for custom OpenAI-compatible models

1 participant