-
Notifications
You must be signed in to change notification settings - Fork 88
refactor: migrate all direct Anthropic SDK usage to provider abstraction #7612
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,42 @@ | ||||||
| import { execSync } from 'node:child_process'; | ||||||
| import { describe, expect, test } from 'bun:test'; | ||||||
|
|
||||||
| /** | ||||||
| * Guard test: production files under assistant/src must not import | ||||||
| * `@anthropic-ai/sdk` directly. Only the canonical provider adapter is | ||||||
| * allowed to use the SDK. | ||||||
| * | ||||||
| * Allowlist entries should be kept minimal — add a path here only if it | ||||||
| * genuinely needs to talk to the Anthropic SDK without going through the | ||||||
| * provider abstraction. | ||||||
| */ | ||||||
| const ALLOWED_FILES = new Set([ | ||||||
| 'assistant/src/providers/anthropic/client.ts', | ||||||
| ]); | ||||||
|
|
||||||
| describe('no direct @anthropic-ai/sdk imports', () => { | ||||||
| test('production files do not import @anthropic-ai/sdk outside allowlist', () => { | ||||||
| let grepOutput = ''; | ||||||
| try { | ||||||
| grepOutput = execSync( | ||||||
| `git grep -l "@anthropic-ai/sdk" -- 'assistant/src/**/*.ts'`, | ||||||
| { encoding: 'utf-8', cwd: process.cwd() + '/../..' }, | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Guard test The Root Cause and ImpactWhen // assistant/src/__tests__/no-direct-anthropic-sdk-imports.test.ts:23
cwd: process.cwd() + '/../..'
// From {repo}/assistant/ this resolves to {repo}/../ — outside the repoThe fix should use Impact: The guard test cannot work as intended — it will always throw an error instead of detecting direct
Suggested change
Was this helpful? React with 👍 or 👎 to provide feedback. |
||||||
| ).trim(); | ||||||
| } catch (err) { | ||||||
| // Exit code 1 means no matches — that's the happy path | ||||||
| if ((err as { status?: number }).status === 1) { | ||||||
| return; | ||||||
| } | ||||||
| throw err; | ||||||
| } | ||||||
|
|
||||||
| const files = grepOutput | ||||||
| .split('\n') | ||||||
| .filter((f) => f.length > 0) | ||||||
| // Exclude test files — they legitimately mock the SDK | ||||||
| .filter((f) => !f.includes('/__tests__/')); | ||||||
| const violations = files.filter((f) => !ALLOWED_FILES.has(f)); | ||||||
|
|
||||||
| expect(violations).toEqual([]); | ||||||
| }); | ||||||
| }); | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new guard test computes
cwdasprocess.cwd() + '/../..', which points outside this repo under the normal workflow (cd assistant && bun testfromAGENTS.md), sogit greperrors withfatal: not a git repositoryand the test fails even when there are no violations. I verified this by runningcd assistant && bun test src/__tests__/no-direct-anthropic-sdk-imports.test.ts, which fails before checking imports.Useful? React with 👍 / 👎.