Skip to content

feat: implement xAI adapter - #571

Closed
jhosepm352-design wants to merge 1 commit into
profullstack:masterfrom
jhosepm352-design:feat/adapter-ai-xai
Closed

jhosepm352-design wants to merge 1 commit into
profullstack:masterfrom
jhosepm352-design:feat/adapter-ai-xai

Conversation

@jhosepm352-design

Copy link
Copy Markdown
Contributor

Implements the xAI provider using the OpenAI-compatible protocol.

@greptile-apps

greptile-apps Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR refactors the xAI adapter to align more closely with the OpenAI-compatible adapter pattern, but introduces several regressions that break existing tests and remove a security safeguard.

  • Runtime breakage: xAI_LOWER (line 19) and xAI (line 42) are referenced but never defined or imported — every call to generate() throws a ReferenceError.
  • Security regression: The redact() helper that prevented XAI_API_KEY from appearing in thrown error messages was removed; raw API error text is now sliced and thrown verbatim.
  • Functional regressions: Trailing-slash normalization on baseUrl was dropped (produces double-slash URLs for user-provided base URLs), and the default model was changed to the legacy grok-beta, breaking the dry-run test assertion.

Confidence Score: 1/5

Not safe to merge — the adapter throws a ReferenceError on every invocation, the API key redaction safety net was removed, and three existing tests will fail.

The generate function references two variables (xAI_LOWER and xAI) that do not exist anywhere in the codebase, making the adapter completely non-functional. The removal of the redact helper means API keys can surface in error messages captured by logging infrastructure. The trailing-slash normalization regression and the stale default model name compound the breakage, with existing tests covering all three behaviours now set to fail.

packages/ai/xai/src/index.ts needs attention on all four issues before this is mergeable.

Security Review

  • API key leakage in error messages (packages/ai/xai/src/index.ts, line 42): The redact() helper that scrubbed XAI_API_KEY from error response bodies was removed. API error text is now truncated and thrown verbatim, meaning a key that crosses the 200-character boundary can leak partially into thrown Error messages — which may be captured by logging pipelines, Sentry, or other observability tools.

Important Files Changed

Filename Overview
packages/ai/xai/src/index.ts Introduces four regressions: undefined variables xAI_LOWER/xAI (ReferenceError on every call), removed redact helper exposing API key in error messages, removed trailing-slash normalization producing double-slash URLs, and default model changed to the legacy grok-beta — all of which break existing tests.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant xAIAdapter as xAI Adapter (index.ts)
    participant Vault as Secret Vault
    participant xAIAPI as xAI API

    Caller->>xAIAdapter: generate(ctx, prompt, opts, config)
    xAIAdapter->>Vault: ctx.secret('XAI_API_KEY')
    Vault-->>xAIAdapter: apiKey
    Note over xAIAdapter: xAI_LOWER is not defined - execution stops here
    xAIAdapter->>xAIAPI: POST /v1/chat/completions
    alt res.ok
        xAIAPI-->>xAIAdapter: 200 JSON
        xAIAdapter-->>Caller: text, model, tokens
    else res not ok
        xAIAPI-->>xAIAdapter: 4xx/5xx error body
        Note over xAIAdapter: API key NOT redacted from error text
        xAIAdapter-->>Caller: throw Error with raw body
    end
Loading

Reviews (1): Last reviewed commit: "feat: add xAI adapter" | Re-trigger Greptile

const model = opts.model ?? 'grok-3';
ctx.log(`xai · model=${model} · ${prompt.length} chars in`);
const model = opts.model ?? 'grok-beta';
ctx.log(`${xAI_LOWER} · model=${model} · ${prompt.length} chars in`);

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.

P0 Undefined variable xAI_LOWER causes ReferenceError

xAI_LOWER is not defined, imported, or declared anywhere in this file or the repository. Every call to generate() will throw a ReferenceError: xAI_LOWER is not defined at runtime (or fail TypeScript compilation). The same applies to xAI on line 42. The original code used string literals 'xai' and 'xAI' directly.

}),
});
if (!res.ok) throw new Error(`xAI ${res.status}: ${redact(await res.text(), apiKey).slice(0, 200)}`);
if (!res.ok) throw new Error(`${xAI} ${res.status}: ${(await res.text()).slice(0, 200)}`);

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.

P1 security Removal of redact exposes API key in error messages

The redact helper was deleted, so API error responses are now sliced and thrown verbatim. The existing test (index.test.ts lines 84–103) explicitly verifies that the error message contains [redacted] and does not contain the API key or its first 10 characters — that test will now fail. An API key that spans the 200-character truncation boundary can leak partially in the thrown Error message, which may surface in logs, Sentry, or other observability tools.

'content-type': 'application/json',
};

const res = await fetch(`${config.baseUrl ?? DEFAULT_BASE}/v1/chat/completions`, {

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.

P1 Trailing slash in baseUrl produces a double-slash URL

The removed chatCompletionsUrl helper called .replace(/\/+$/, '') to strip trailing slashes before appending /v1/chat/completions. The bare template literal ${config.baseUrl ?? DEFAULT_BASE}/v1/chat/completions no longer does that normalization. When a caller passes { baseUrl: 'https://proxy.example.com/' }, the resulting URL is https://proxy.example.com//v1/chat/completions. The test at index.test.ts line 81 explicitly asserts the single-slash form and will fail with this change.

Comment on lines +18 to 20
const model = opts.model ?? 'grok-beta';
ctx.log(`${xAI_LOWER} · model=${model} · ${prompt.length} chars in`);
if (ctx.dryRun) return { text: '[dry-run]', model };

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.

P1 Default model changed to grok-beta breaks the dry-run test

The dry-run test in index.test.ts (line 24) asserts { text: '[dry-run]', model: 'grok-3' }. With the default now changed to 'grok-beta', that assertion will fail. Additionally, grok-beta is the older xAI model name; the xAI API currently supports grok-3, grok-3-mini, and grok-2-latest — the model list was fully narrowed down to a legacy alias.

@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

2 similar comments
@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@ralyodio

ralyodio commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

tests are failing please fix.

@ralyodio ralyodio closed this Jun 4, 2026
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.

2 participants