Skip to content

fix(sdk): harden guard client with policy pack schema fields and test coverage #173

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.

Sign up for GitHub

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

Closed
kantorcodes wants to merge 1 commit into from
Closed

fix(sdk): harden guard client with policy pack schema fields and test coverage #173

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 59 additions & 1 deletion __tests__/services/registry-broker-client.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,59 @@ describe('RegistryBrokerClient', () => {
expect(targetUrl).toBe('https://api.example.com/api/v1/delegate');
expect(init?.method).toBe('POST');
expect(init?.headers).toBeInstanceOf(Headers);
expect((init?.headers as Headers).get('content-type')).toBe('application/json');
expect((init?.headers as Headers).get('content-type')).toBe(
'application/json',
);
});

it('calls the delegate endpoint with payload and options overload', async () => {
fetchImplementation.mockResolvedValueOnce(
createResponse({
json: async () => ({
task: 'review this repo',
intents: ['delegate'],
protocols: ['registry-broker'],
surfaces: ['cli'],
shouldDelegate: true,
opportunities: [],
}),
}) as unknown as Response,
);

const client = new RegistryBrokerClient({
baseUrl: 'https://api.example.com',
fetchImplementation,
});

await client.delegate(

@kilo-code-bot kilo-code-bot Bot May 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

CRITICAL: Invalid method invocation - delegate expects 1 argument but 2 provided

The delegate method signature is delegate(request: DelegationPlanRequest): Promise<DelegationPlanResponse>. The test calls it with two separate objects, which violates the type signature and will cause a TypeScript compilation error.

Additionally, even if this compiled, the implementation would only use the first argument as the request body, causing the assertion to fail.

Merge the two objects into a single DelegationPlanRequest:

Suggested change
await client.delegate(
await client.delegate({
task: 'review this repo',
context: 'focus on broker chat parity',
limit: 2,
filter: {
protocols: ['mcp'],
},
workspace: {
openFiles: ['src/api/chat/contract.ts'],
},
});

{
task: 'review this repo',
context: 'focus on broker chat parity',
},
{
limit: 2,
filter: {
protocols: ['mcp'],
},
Comment on lines +551 to +555

@chatgpt-codex-connector chatgpt-codex-connector Bot May 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Call delegate with one merged payload argument

RegistryBrokerClient.delegate currently accepts a single request object (it forwards only the first argument in src/services/registry-broker/client/base-client.ts and search.ts), so the second “options” argument passed here is ignored at runtime. That means this new test cannot observe limit/filter/workspace in the request body and will fail when executed, which breaks CI for this suite.

Useful? React with 👍 / 👎.

workspace: {
openFiles: ['src/api/chat/contract.ts'],
},
},
);
Comment on lines +546 to +560

@gemini-code-assist gemini-code-assist Bot May 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The test attempts to call client.delegate with two separate arguments (a payload and an options object), but the implementation in base-client.ts (line 742) only defines a single request argument. Consequently, the second argument will be ignored at runtime. Furthermore, the assertion at line 564 expects the request body to contain fields from both arguments merged together, which will cause the test to fail unless the implementation is updated to handle this overload.


expect(fetchImplementation).toHaveBeenCalledTimes(1);
const [, init] = fetchImplementation.mock.calls[0];
expect(JSON.parse(init?.body as string)).toEqual({
task: 'review this repo',
context: 'focus on broker chat parity',
limit: 2,
filter: {
protocols: ['mcp'],
},
workspace: {
openFiles: ['src/api/chat/contract.ts'],
},
});
});

it('throws RegistryBrokerError on non-OK response', async () => {
Expand Down Expand Up @@ -743,6 +795,8 @@ describe('RegistryBrokerClient', () => {
name: 'Security team default',
sharedHarnessDefaults: { codex: 'prompt' },
allowedPublishers: ['hol'],
blockedPublishers: [],
blockedDomains: [],
blockedArtifacts: ['plugin:forked/risky-tool'],
alertChannel: 'email',
updatedAt: '2026-04-11T00:00:00.000Z',
Expand All @@ -756,6 +810,8 @@ describe('RegistryBrokerClient', () => {
name: 'Security team default',
sharedHarnessDefaults: { codex: 'prompt' },
allowedPublishers: ['hol'],
blockedPublishers: [],
blockedDomains: [],
blockedArtifacts: ['plugin:forked/risky-tool'],
alertChannel: 'slack',
updatedAt: '2026-04-11T00:00:00.000Z',
Expand Down Expand Up @@ -1408,6 +1464,7 @@ describe('RegistryBrokerClient', () => {
agentUrl: 'https://demo.agent',
sessionId: 'session-1',
message: 'Hi',
idempotencyKey: 'chat-send-1',

@gemini-code-assist gemini-code-assist Bot May 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The idempotencyKey field is being passed to sendMessage here and verified in the request body in the assertion at line 1496. However, the implementation of sendMessage in src/services/registry-broker/client/chat.ts (lines 412-429) does not include logic to extract this field from the payload and add it to the request body. This will likely result in a test failure as the field will be missing from the actual outgoing request.

auth,
});
expect(message.message).toBe('Hello');
Expand Down Expand Up @@ -1436,6 +1493,7 @@ describe('RegistryBrokerClient', () => {
expect(JSON.parse(messageRequestInit.body as string)).toEqual({
agentUrl: 'https://demo.agent',
auth: { type: 'bearer', token: 'user-key' },
idempotencyKey: 'chat-send-1',
message: 'Hi',
sessionId: 'session-1',
});
Expand Down
Loading