-
Notifications
You must be signed in to change notification settings - Fork 4
fix(core): addressing issue with headers not being sent to run agent #9
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 2 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,33 @@ | ||
| name: test | ||
|
|
||
| on: | ||
| push: | ||
| branches: | ||
| - main | ||
| pull_request: | ||
| branches: | ||
| - main | ||
| workflow_dispatch: | ||
|
|
||
| jobs: | ||
| unit: | ||
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - uses: pnpm/action-setup@v3 | ||
| with: | ||
| version: 9 | ||
|
|
||
| - uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: 20 | ||
| cache: 'pnpm' | ||
| cache-dependency-path: pnpm-lock.yaml | ||
|
|
||
| - name: Install dependencies | ||
| run: pnpm install --frozen-lockfile | ||
|
|
||
| - name: Run tests | ||
| run: pnpm test |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,113 @@ | ||
| import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; | ||
| import { CopilotKitCore } from "../core"; | ||
| import { HttpAgent } from "@ag-ui/client"; | ||
| import { waitForCondition } from "./test-utils"; | ||
|
|
||
| describe("CopilotKitCore headers", () => { | ||
| const originalFetch = global.fetch; | ||
|
|
||
| beforeEach(() => { | ||
| vi.restoreAllMocks(); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| if (originalFetch) { | ||
| global.fetch = originalFetch; | ||
| } else { | ||
| delete (global as typeof globalThis & { fetch?: typeof fetch }).fetch; | ||
| } | ||
| }); | ||
|
|
||
| it("includes provided headers when fetching runtime info", async () => { | ||
| const fetchMock = vi.fn().mockResolvedValue({ | ||
| json: vi.fn().mockResolvedValue({ version: "1.0.0", agents: {} }), | ||
| }); | ||
| global.fetch = fetchMock as unknown as typeof fetch; | ||
|
|
||
| const headers = { | ||
| Authorization: "Bearer test-token", | ||
| "X-Custom-Header": "custom-value", | ||
| }; | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| const core = new CopilotKitCore({ | ||
| runtimeUrl: "https://runtime.example", | ||
| headers, | ||
| }); | ||
|
|
||
| await waitForCondition(() => fetchMock.mock.calls.length >= 1); | ||
|
|
||
| expect(fetchMock).toHaveBeenCalledWith( | ||
| "https://runtime.example/info", | ||
| expect.objectContaining({ | ||
| headers: expect.objectContaining(headers), | ||
| }) | ||
| ); | ||
| }); | ||
|
|
||
| it("uses updated headers for subsequent runtime requests", async () => { | ||
| const fetchMock = vi.fn().mockResolvedValue({ | ||
| json: vi.fn().mockResolvedValue({ version: "1.0.0", agents: {} }), | ||
| }); | ||
| global.fetch = fetchMock as unknown as typeof fetch; | ||
|
|
||
| const core = new CopilotKitCore({ | ||
| runtimeUrl: "https://runtime.example", | ||
| headers: { Authorization: "Bearer initial" }, | ||
| }); | ||
|
|
||
| await waitForCondition(() => fetchMock.mock.calls.length >= 1); | ||
|
|
||
| core.setHeaders({ Authorization: "Bearer updated", "X-Trace": "123" }); | ||
| core.setRuntimeUrl(undefined); | ||
| core.setRuntimeUrl("https://runtime.example"); | ||
|
|
||
| await waitForCondition(() => fetchMock.mock.calls.length >= 2); | ||
|
|
||
| const secondCall = fetchMock.mock.calls[1]; | ||
| expect(secondCall?.[1]?.headers).toMatchObject({ | ||
| Authorization: "Bearer updated", | ||
| "X-Trace": "123", | ||
| }); | ||
| }); | ||
|
|
||
| it("passes configured headers to HttpAgent runs", async () => { | ||
| const recorded: Array<Record<string, string>> = []; | ||
|
|
||
| class RecordingHttpAgent extends HttpAgent { | ||
| constructor() { | ||
| super({ url: "https://runtime.example" }); | ||
| } | ||
|
|
||
| async connectAgent(...args: Parameters<HttpAgent["connectAgent"]>) { | ||
| recorded.push({ ...this.headers }); | ||
| return Promise.resolve({ newMessages: [] }) as ReturnType<HttpAgent["connectAgent"]>; | ||
| } | ||
|
|
||
| async runAgent(...args: Parameters<HttpAgent["runAgent"]>) { | ||
| recorded.push({ ...this.headers }); | ||
| return Promise.resolve({ newMessages: [] }) as ReturnType<HttpAgent["runAgent"]>; | ||
| } | ||
| } | ||
|
|
||
| const agent = new RecordingHttpAgent(); | ||
|
|
||
| const core = new CopilotKitCore({ | ||
| runtimeUrl: undefined, | ||
| headers: { Authorization: "Bearer cfg", "X-Team": "angular" }, | ||
| agents: { default: agent }, | ||
| }); | ||
|
|
||
| await agent.runAgent(); | ||
| await core.connectAgent({ agent, agentId: "default" }); | ||
| await core.runAgent({ agent, agentId: "default" }); | ||
|
|
||
| expect(recorded).toHaveLength(3); | ||
| for (const headers of recorded) { | ||
| expect(headers).toMatchObject({ | ||
| Authorization: "Bearer cfg", | ||
| "X-Team": "angular", | ||
| }); | ||
| } | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -141,11 +141,24 @@ export class CopilotKitCore { | |
| this.headers = headers; | ||
| this.properties = properties; | ||
| this.localAgents = this.assignAgentIds(agents); | ||
| this.applyHeadersToAgents(this.localAgents); | ||
| this._agents = this.localAgents; | ||
| this._tools = tools; | ||
| this.setRuntimeUrl(runtimeUrl); | ||
| } | ||
|
|
||
| private applyHeadersToAgent(agent: AbstractAgent) { | ||
| if (agent instanceof HttpAgent) { | ||
| agent.headers = { ...this.headers }; | ||
| } | ||
| } | ||
|
|
||
| private applyHeadersToAgents(agents: Record<string, AbstractAgent>) { | ||
| Object.values(agents).forEach((agent) => { | ||
| this.applyHeadersToAgent(agent); | ||
| }); | ||
| } | ||
|
|
||
| private assignAgentIds(agents: Record<string, AbstractAgent>) { | ||
| Object.entries(agents).forEach(([id, agent]) => { | ||
| if (agent && !agent.agentId) { | ||
|
|
@@ -314,6 +327,7 @@ export class CopilotKitCore { | |
| agentId: id, | ||
| description: description, | ||
| }); | ||
| this.applyHeadersToAgent(agent); | ||
| return [id, agent]; | ||
| }) | ||
| ); | ||
|
|
@@ -387,6 +401,9 @@ export class CopilotKitCore { | |
| */ | ||
| setHeaders(headers: Record<string, string>) { | ||
| this.headers = headers; | ||
| this.applyHeadersToAgents(this.localAgents); | ||
| this.applyHeadersToAgents(this.remoteAgents); | ||
| this.applyHeadersToAgents(this._agents); | ||
| void this.notifySubscribers( | ||
| (subscriber) => | ||
| subscriber.onHeadersChanged?.({ | ||
|
|
@@ -411,7 +428,9 @@ export class CopilotKitCore { | |
|
|
||
| setAgents(agents: Record<string, AbstractAgent>) { | ||
| this.localAgents = this.assignAgentIds(agents); | ||
| this.applyHeadersToAgents(this.localAgents); | ||
| this._agents = { ...this.localAgents, ...this.remoteAgents }; | ||
| this.applyHeadersToAgents(this._agents); | ||
|
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. Applying to this._agents should be enough (it contains all agents) |
||
| void this.notifySubscribers( | ||
| (subscriber) => | ||
| subscriber.onAgentsChanged?.({ | ||
|
|
@@ -427,7 +446,9 @@ export class CopilotKitCore { | |
| if (!agent.agentId) { | ||
| agent.agentId = id; | ||
| } | ||
| this.applyHeadersToAgent(agent); | ||
| this._agents = { ...this.localAgents, ...this.remoteAgents }; | ||
| this.applyHeadersToAgents(this._agents); | ||
|
||
| void this.notifySubscribers( | ||
| (subscriber) => | ||
| subscriber.onAgentsChanged?.({ | ||
|
|
||
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.
this.applyHeadersToAgents(this._agents) is enough