Skip to content

Commit af5cf8b

Browse files
committed
fix(api): plumb proposalId into all apply-dispatcher errors + escapeGlob cache name
bugbot MEDIUM: applySemanticThresholdAdjust, applyAgentToolTtlAdjust, applySemanticInvalidate were passing cache.name as the first arg to ApplyFailedError, which the constructor labels as proposalId — that wrong value was landing in applied_result.details. Plus, the constructor spread '{ proposalId, ...details }' allowed a stale proposalId in details to overwrite the explicit one. Pass proposalId through to all dispatcher methods, attach cacheName separately in details, and put the explicit proposalId last in the spread so it always wins. bugbot LOW: tool and session SCAN patterns interpolated cache.name without escaping while key_prefix did. Apply escapeGlob uniformly to cache.name across all three branches.
1 parent ea47b86 commit af5cf8b

3 files changed

Lines changed: 44 additions & 12 deletions

File tree

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,16 @@ describe('CacheApplyDispatcher', () => {
115115
expect(client.hsets).toEqual([]);
116116
});
117117

118+
it('threshold_adjust capability-missing error carries proposal id, not cache name', async () => {
119+
const client = new FakeClient();
120+
const dispatcher = buildDispatcher({ ...SEMANTIC_CACHE, capabilities: [] }, client);
121+
const p = proposal({ id: 'prop-42' });
122+
await expect(dispatcher.dispatch(p)).rejects.toMatchObject({
123+
code: 'APPLY_FAILED',
124+
details: expect.objectContaining({ proposalId: 'prop-42', cacheName: 'sc:prod' }),
125+
});
126+
});
127+
118128
it('agent tool_ttl_adjust writes JSON policy to {cache_name}:__tool_policies', async () => {
119129
const client = new FakeClient();
120130
const dispatcher = buildDispatcher(AGENT_CACHE, client);

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

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -55,17 +55,32 @@ export class CacheApplyDispatcher {
5555
const startedAt = Date.now();
5656

5757
if (proposal.cache_type === SEMANTIC_CACHE && proposal.proposal_type === 'threshold_adjust') {
58-
const out = await this.applySemanticThresholdAdjust(client, cache, proposal.proposal_payload);
58+
const out = await this.applySemanticThresholdAdjust(
59+
client,
60+
cache,
61+
proposal.proposal_payload,
62+
proposal.id,
63+
);
5964
return { ...out, durationMs: Date.now() - startedAt };
6065
}
6166

6267
if (proposal.cache_type === AGENT_CACHE && proposal.proposal_type === 'tool_ttl_adjust') {
63-
const out = await this.applyAgentToolTtlAdjust(client, cache, proposal.proposal_payload);
68+
const out = await this.applyAgentToolTtlAdjust(
69+
client,
70+
cache,
71+
proposal.proposal_payload,
72+
proposal.id,
73+
);
6474
return { ...out, durationMs: Date.now() - startedAt };
6575
}
6676

6777
if (proposal.cache_type === SEMANTIC_CACHE && proposal.proposal_type === 'invalidate') {
68-
const out = await this.applySemanticInvalidate(client, cache, proposal.proposal_payload);
78+
const out = await this.applySemanticInvalidate(
79+
client,
80+
cache,
81+
proposal.proposal_payload,
82+
proposal.id,
83+
);
6984
return { ...out, durationMs: Date.now() - startedAt };
7085
}
7186

@@ -82,12 +97,13 @@ export class CacheApplyDispatcher {
8297
client: Valkey,
8398
cache: ResolvedCache,
8499
payload: SemanticThresholdAdjustPayload,
100+
proposalId: string,
85101
): Promise<Omit<ApplyOutcome, 'durationMs'>> {
86102
if (!cache.capabilities.includes('threshold_adjust')) {
87103
throw new ApplyFailedError(
88-
cache.name,
104+
proposalId,
89105
`Cache '${cache.name}' does not advertise 'threshold_adjust' capability — it cannot read runtime threshold overrides`,
90-
{ reason: 'capability_missing' },
106+
{ reason: 'capability_missing', cacheName: cache.name },
91107
);
92108
}
93109
const configKey = `${cache.name}:__config`;
@@ -97,8 +113,9 @@ export class CacheApplyDispatcher {
97113
try {
98114
await client.hset(configKey, field, String(payload.new_threshold));
99115
} catch (err) {
100-
throw new ApplyFailedError(cache.name, `HSET ${configKey} failed`, {
116+
throw new ApplyFailedError(proposalId, `HSET ${configKey} failed`, {
101117
reason: 'valkey_command_failed',
118+
cacheName: cache.name,
102119
underlying: err instanceof Error ? err.message : String(err),
103120
});
104121
}
@@ -117,14 +134,16 @@ export class CacheApplyDispatcher {
117134
client: Valkey,
118135
cache: ResolvedCache,
119136
payload: AgentToolTtlAdjustPayload,
137+
proposalId: string,
120138
): Promise<Omit<ApplyOutcome, 'durationMs'>> {
121139
const policiesKey = `${cache.name}:__tool_policies`;
122140
const policy = JSON.stringify({ ttl: payload.new_ttl_seconds });
123141
try {
124142
await client.hset(policiesKey, payload.tool_name, policy);
125143
} catch (err) {
126-
throw new ApplyFailedError(cache.name, `HSET ${policiesKey} failed`, {
144+
throw new ApplyFailedError(proposalId, `HSET ${policiesKey} failed`, {
127145
reason: 'valkey_command_failed',
146+
cacheName: cache.name,
128147
underlying: err instanceof Error ? err.message : String(err),
129148
});
130149
}
@@ -142,6 +161,7 @@ export class CacheApplyDispatcher {
142161
client: Valkey,
143162
cache: ResolvedCache,
144163
payload: SemanticInvalidatePayload,
164+
proposalId: string,
145165
): Promise<Omit<ApplyOutcome, 'durationMs'>> {
146166
const indexName = `${cache.name}:__index`;
147167
let raw: unknown;
@@ -159,8 +179,9 @@ export class CacheApplyDispatcher {
159179
'2',
160180
);
161181
} catch (err) {
162-
throw new ApplyFailedError(cache.name, `FT.SEARCH ${indexName} failed`, {
182+
throw new ApplyFailedError(proposalId, `FT.SEARCH ${indexName} failed`, {
163183
reason: 'valkey_command_failed',
184+
cacheName: cache.name,
164185
underlying: err instanceof Error ? err.message : String(err),
165186
});
166187
}
@@ -179,8 +200,9 @@ export class CacheApplyDispatcher {
179200
try {
180201
deleted = await client.del(...keys);
181202
} catch (err) {
182-
throw new ApplyFailedError(cache.name, `DEL failed during invalidate`, {
203+
throw new ApplyFailedError(proposalId, `DEL failed during invalidate`, {
183204
reason: 'valkey_command_failed',
205+
cacheName: cache.name,
184206
underlying: err instanceof Error ? err.message : String(err),
185207
});
186208
}
@@ -200,7 +222,7 @@ export class CacheApplyDispatcher {
200222
proposalId: string,
201223
): Promise<Omit<ApplyOutcome, 'durationMs'>> {
202224
if (payload.filter_kind === 'tool') {
203-
const pattern = `${cache.name}:tool:${escapeGlob(payload.filter_value)}:*`;
225+
const pattern = `${escapeGlob(cache.name)}:tool:${escapeGlob(payload.filter_value)}:*`;
204226
const deleted = await scanAndDelete(client, pattern);
205227
return {
206228
actualAffected: deleted,
@@ -216,7 +238,7 @@ export class CacheApplyDispatcher {
216238
};
217239
}
218240
if (payload.filter_kind === 'session') {
219-
const pattern = `${cache.name}:session:${escapeGlob(payload.filter_value)}*`;
241+
const pattern = `${escapeGlob(cache.name)}:session:${escapeGlob(payload.filter_value)}*`;
220242
const deleted = await scanAndDelete(client, pattern);
221243
return {
222244
actualAffected: deleted,

apps/api/src/cache-proposals/errors.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ export class ProposalEditNotAllowedError extends CacheProposalError {
100100

101101
export class ApplyFailedError extends CacheProposalError {
102102
constructor(proposalId: string, message: string, details?: Record<string, unknown>) {
103-
super('APPLY_FAILED', message, { proposalId, ...details });
103+
super('APPLY_FAILED', message, { ...details, proposalId });
104104
this.name = 'ApplyFailedError';
105105
}
106106
}

0 commit comments

Comments
 (0)