Skip to content

Commit db14ecd

Browse files
committed
fix(api): address bugbot findings on apply dispatcher PR
- HIGH: parseFtSearchKeys was iterating with i+=2, missing every other key when FT.SEARCH was called with RETURN 0 (which behaves like NOCONTENT and returns just [count, key1, key2, ...]). Step by 1. - LOW: extract optionalString, optionalFiniteNumber, formatApprovalResult to controller-helpers.ts, share between cache-proposal.controller and mcp.controller - Add regression test exercising RETURN 0 response shape
1 parent 76ac9e0 commit db14ecd

5 files changed

Lines changed: 69 additions & 65 deletions

File tree

apps/api/src/cache-proposals/__tests__/cache-apply.dispatcher.spec.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,10 @@ class FakeClient {
2424
return [cursor === '0' ? '0' : '0', []];
2525
}
2626

27+
public ftSearchResponse: unknown = [0];
28+
2729
async call(): Promise<unknown> {
28-
return [0];
30+
return this.ftSearchResponse;
2931
}
3032
}
3133

@@ -133,6 +135,24 @@ describe('CacheApplyDispatcher', () => {
133135
]);
134136
});
135137

138+
it('semantic invalidate parses FT.SEARCH RETURN 0 response without skipping keys', async () => {
139+
const client = new FakeClient();
140+
client.ftSearchResponse = [3, 'sc:prod:k1', 'sc:prod:k2', 'sc:prod:k3'];
141+
const dispatcher = buildDispatcher(SEMANTIC_CACHE, client);
142+
const out = await dispatcher.dispatch(
143+
proposal({
144+
proposal_type: 'invalidate',
145+
proposal_payload: {
146+
filter_kind: 'valkey_search',
147+
filter_expression: '@model:{gpt}',
148+
estimated_affected: 10,
149+
},
150+
}),
151+
);
152+
expect(client.deletes).toEqual(['sc:prod:k1', 'sc:prod:k2', 'sc:prod:k3']);
153+
expect(out.actualAffected).toBe(3);
154+
});
155+
136156
it('fails when cache type changed since proposal creation', async () => {
137157
const client = new FakeClient();
138158
const dispatcher = buildDispatcher(AGENT_CACHE, client);

apps/api/src/cache-proposals/cache-apply.dispatcher.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ function parseFtSearchKeys(raw: unknown): string[] {
235235
return [];
236236
}
237237
const keys: string[] = [];
238-
for (let i = 1; i < raw.length; i += 2) {
238+
for (let i = 1; i < raw.length; i += 1) {
239239
const key = raw[i];
240240
if (typeof key === 'string') {
241241
keys.push(key);

apps/api/src/cache-proposals/cache-proposal.controller.ts

Lines changed: 3 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { ConnectionId } from '../common/decorators';
1717
import { parseOptionalInt } from '../common/utils/parse-query-param';
1818
import { CacheProposalService } from './cache-proposal.service';
1919
import { mapCacheProposalErrorToHttp } from './errors-http';
20+
import { formatApprovalResult, optionalFiniteNumber, optionalString } from './controller-helpers';
2021

2122
const ACTOR_SOURCE_UI = 'ui' as const;
2223

@@ -129,8 +130,8 @@ export class CacheProposalController {
129130
},
130131
): Promise<unknown> {
131132
try {
132-
const newThreshold = optionalNumber(body?.new_threshold, 'new_threshold');
133-
const newTtlSeconds = optionalNumber(body?.new_ttl_seconds, 'new_ttl_seconds');
133+
const newThreshold = optionalFiniteNumber(body?.new_threshold, 'new_threshold');
134+
const newTtlSeconds = optionalFiniteNumber(body?.new_ttl_seconds, 'new_ttl_seconds');
134135
const actor = optionalString(body?.actor, 'actor') ?? null;
135136
if (newThreshold === undefined && newTtlSeconds === undefined) {
136137
throw new BadRequestException('Either new_threshold or new_ttl_seconds is required');
@@ -148,34 +149,3 @@ export class CacheProposalController {
148149
}
149150
}
150151

151-
function optionalString(value: unknown, field: string): string | undefined {
152-
if (value === undefined || value === null) {
153-
return undefined;
154-
}
155-
if (typeof value !== 'string') {
156-
throw new BadRequestException(`${field} must be a string when provided`);
157-
}
158-
return value;
159-
}
160-
161-
function optionalNumber(value: unknown, field: string): number | undefined {
162-
if (value === undefined || value === null) {
163-
return undefined;
164-
}
165-
if (typeof value !== 'number' || !Number.isFinite(value)) {
166-
throw new BadRequestException(`${field} must be a finite number when provided`);
167-
}
168-
return value;
169-
}
170-
171-
function formatApprovalResult(result: {
172-
proposal: StoredCacheProposal;
173-
appliedResult: { success: boolean; error?: string; details?: Record<string, unknown> } | null;
174-
}): unknown {
175-
return {
176-
proposal_id: result.proposal.id,
177-
status: result.proposal.status,
178-
applied_result: result.appliedResult,
179-
};
180-
}
181-
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import { BadRequestException } from '@nestjs/common';
2+
import type { AppliedResult, StoredCacheProposal } from '@betterdb/shared';
3+
4+
export function optionalString(value: unknown, field: string): string | undefined {
5+
if (value === undefined || value === null) {
6+
return undefined;
7+
}
8+
if (typeof value !== 'string') {
9+
throw new BadRequestException(`${field} must be a string when provided`);
10+
}
11+
return value;
12+
}
13+
14+
export function optionalFiniteNumber(value: unknown, field: string): number | undefined {
15+
if (value === undefined || value === null) {
16+
return undefined;
17+
}
18+
if (typeof value !== 'number' || !Number.isFinite(value)) {
19+
throw new BadRequestException(`${field} must be a finite number when provided`);
20+
}
21+
return value;
22+
}
23+
24+
export interface ApprovalResultPayload {
25+
proposal_id: string;
26+
status: string;
27+
applied_result: AppliedResult | null;
28+
}
29+
30+
export function formatApprovalResult(result: {
31+
proposal: StoredCacheProposal;
32+
appliedResult: AppliedResult | null;
33+
}): ApprovalResultPayload {
34+
return {
35+
proposal_id: result.proposal.id,
36+
status: result.proposal.status,
37+
applied_result: result.appliedResult,
38+
};
39+
}

apps/api/src/mcp/mcp.controller.ts

Lines changed: 5 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@ import { StoragePort } from '../common/interfaces/storage-port.interface';
1212
import { CacheProposalService } from '../cache-proposals/cache-proposal.service';
1313
import { CacheReadonlyService } from '../cache-proposals/cache-readonly.service';
1414
import { mapCacheProposalErrorToHttp } from '../cache-proposals/errors-http';
15+
import {
16+
formatApprovalResult,
17+
optionalFiniteNumber,
18+
optionalString,
19+
} from '../cache-proposals/controller-helpers';
1520
import type { StoredCacheProposal } from '@betterdb/shared';
1621

1722
const INSTANCE_ID_RE = /^[a-zA-Z0-9_-]+$/;
@@ -789,16 +794,6 @@ function requireString(value: unknown, field: string): string {
789794
return value;
790795
}
791796

792-
function optionalString(value: unknown, field: string): string | undefined {
793-
if (value === undefined || value === null) {
794-
return undefined;
795-
}
796-
if (typeof value !== 'string') {
797-
throw new BadRequestException(`${field} must be a string when provided`);
798-
}
799-
return value;
800-
}
801-
802797
function optionalNullableString(value: unknown, field: string): string | null | undefined {
803798
if (value === undefined) {
804799
return undefined;
@@ -829,24 +824,4 @@ function formatProposalResult(result: { proposal: StoredCacheProposal; warnings:
829824
};
830825
}
831826

832-
function formatApprovalResult(result: {
833-
proposal: StoredCacheProposal;
834-
appliedResult: { success: boolean; error?: string; details?: Record<string, unknown> } | null;
835-
}) {
836-
return {
837-
proposal_id: result.proposal.id,
838-
status: result.proposal.status,
839-
applied_result: result.appliedResult,
840-
};
841-
}
842-
843-
function optionalFiniteNumber(value: unknown, field: string): number | undefined {
844-
if (value === undefined || value === null) {
845-
return undefined;
846-
}
847-
if (typeof value !== 'number' || !Number.isFinite(value)) {
848-
throw new BadRequestException(`${field} must be a finite number when provided`);
849-
}
850-
return value;
851-
}
852827

0 commit comments

Comments
 (0)