Skip to content

Commit 4b0c14c

Browse files
fix: PR C — operational (round 4, bugs 7, 10, 12) (#80)
Bug 7 (HIGH): outbound webhook delivery not implemented - New webhook_deliveries table (id, subscriptionId, eventType, payload, status, attempts, nextRetryAt, lastStatusCode, lastError, etc.) - WebhookDeliveryService: async POST with HMAC-SHA256 signature, X-HAIP-Signature/Event-Id/Event-Type headers, 5s timeout - Exponential backoff 30s/2m/10m/1h/6h (5 attempts) - In-process setInterval scanner (30s) re-processes pending retries - GET /connect/subscriptions/:id/deliveries for observability - BullMQ deferred — Redis not wired up, pragmatic fallback per brief Bug 10 (HIGH): push-schema.ts missing tables - Added: agent_configs, agent_decisions, agent_training_snapshots, tax_profiles, tax_rules, guest_reviews, webhook_deliveries - Enums: agent_type, agent_mode, agent_decision_status, tax_rule_type, review_source, review_response_status, webhook_delivery_status - Long-term: replace push-schema.ts with real Drizzle migrations Bug 12 (MEDIUM): agent config mutations unaudited - updateConfig writes audit_logs row with per-field diff (isEnabled, mode, autopilotConfidenceThreshold, config), no-op changes skipped - approveDecision, rejectDecision also audited with actor from @currentuser - Reused existing auditLogs table; no new schema 562/562 tests passing (+9), build + typecheck clean. Co-authored-by: Dušan Milićević <dusanmilicevic33@gmail.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent d301937 commit 4b0c14c

12 files changed

Lines changed: 872 additions & 21 deletions

apps/api/src/modules/agent/agent.controller.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { eq, and, desc } from 'drizzle-orm';
1515
import { guestReviews } from '@haip/database';
1616
import { DRIZZLE } from '../../database/database.module';
1717
import { Roles } from '../auth/roles.decorator';
18+
import { CurrentUser, type AuthUser } from '../auth/current-user.decorator';
1819
import { AgentService } from './agent.service';
1920
import { UpdateAgentConfigDto } from './dto/agent-config.dto';
2021
import { RejectDecisionDto } from './dto/agent-decision.dto';
@@ -50,8 +51,9 @@ export class AgentController {
5051
@Param('propertyId', ParseUUIDPipe) propertyId: string,
5152
@Param('agentType') agentType: string,
5253
@Body() dto: UpdateAgentConfigDto,
54+
@CurrentUser() user?: AuthUser,
5355
) {
54-
return this.agentService.updateConfig(propertyId, agentType, dto as any);
56+
return this.agentService.updateConfig(propertyId, agentType, dto as any, user?.sub);
5557
}
5658

5759
@Post(':propertyId/:agentType/run')
@@ -83,8 +85,9 @@ export class AgentController {
8385
async approveDecision(
8486
@Param('propertyId', ParseUUIDPipe) propertyId: string,
8587
@Param('id', ParseUUIDPipe) id: string,
88+
@CurrentUser() user?: AuthUser,
8689
) {
87-
return this.agentService.approveDecision(propertyId, id);
90+
return this.agentService.approveDecision(propertyId, id, user?.sub);
8891
}
8992

9093
@Post(':propertyId/decisions/:id/reject')
@@ -93,8 +96,9 @@ export class AgentController {
9396
@Param('propertyId', ParseUUIDPipe) propertyId: string,
9497
@Param('id', ParseUUIDPipe) id: string,
9598
@Body() dto: RejectDecisionDto,
99+
@CurrentUser() user?: AuthUser,
96100
) {
97-
return this.agentService.rejectDecision(propertyId, id, undefined, dto.reason);
101+
return this.agentService.rejectDecision(propertyId, id, user?.sub, dto.reason);
98102
}
99103

100104
@Get(':propertyId/:agentType/performance')

apps/api/src/modules/agent/agent.service.spec.ts

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,78 @@ describe('AgentService', () => {
300300
await expect(service.getDecisions('prop-1', 'not_real')).rejects.toThrow(BadRequestException);
301301
});
302302

303+
// --- Audit logging ---
304+
305+
it('writes an audit log row when updateConfig changes a field', async () => {
306+
const disabledConfig = { ...existingConfig, isEnabled: false };
307+
const updatedConfig = { ...existingConfig, isEnabled: true };
308+
const db = createMockDb({
309+
selectResult: [disabledConfig],
310+
updateResult: [updatedConfig],
311+
});
312+
const module = await Test.createTestingModule({
313+
providers: [
314+
AgentService,
315+
{ provide: DRIZZLE, useValue: db },
316+
{ provide: WebhookService, useValue: { emit: vi.fn() } },
317+
],
318+
}).compile();
319+
const service = module.get(AgentService);
320+
321+
await service.updateConfig('prop-1', 'pricing', { isEnabled: true }, 'user-42');
322+
323+
// First insert is the audit row (no other inserts happen in updateConfig).
324+
expect(db.insert).toHaveBeenCalledTimes(1);
325+
const insertedValues = (db.insert.mock.results[0]!.value.values as any).mock.calls[0][0];
326+
expect(insertedValues.action).toBe('update');
327+
expect(insertedValues.entityType).toBe('agent_config');
328+
expect(insertedValues.userId).toBe('user-42');
329+
expect(insertedValues.previousValue).toEqual({ isEnabled: false });
330+
expect(insertedValues.newValue).toEqual({ isEnabled: true });
331+
});
332+
333+
it('skips audit log when updateConfig makes no effective change', async () => {
334+
// isEnabled already true, update passes the same value
335+
const db = createMockDb({
336+
selectResult: [existingConfig],
337+
updateResult: [existingConfig],
338+
});
339+
const module = await Test.createTestingModule({
340+
providers: [
341+
AgentService,
342+
{ provide: DRIZZLE, useValue: db },
343+
{ provide: WebhookService, useValue: { emit: vi.fn() } },
344+
],
345+
}).compile();
346+
const service = module.get(AgentService);
347+
348+
await service.updateConfig('prop-1', 'pricing', { isEnabled: true });
349+
expect(db.insert).not.toHaveBeenCalled();
350+
});
351+
352+
it('writes an audit log row when rejectDecision is called', async () => {
353+
const db = createMockDb({
354+
selectResult: [pendingDecision],
355+
updateResult: [{ ...pendingDecision, status: 'rejected' }],
356+
});
357+
const module = await Test.createTestingModule({
358+
providers: [
359+
AgentService,
360+
{ provide: DRIZZLE, useValue: db },
361+
{ provide: WebhookService, useValue: { emit: vi.fn() } },
362+
],
363+
}).compile();
364+
const service = module.get(AgentService);
365+
366+
await service.rejectDecision('prop-1', 'dec-1', 'user-99', 'not confident');
367+
368+
expect(db.insert).toHaveBeenCalledTimes(1);
369+
const insertedValues = (db.insert.mock.results[0]!.value.values as any).mock.calls[0][0];
370+
expect(insertedValues.entityType).toBe('agent_decision');
371+
expect(insertedValues.userId).toBe('user-99');
372+
expect(insertedValues.newValue).toMatchObject({ status: 'rejected', reason: 'not confident' });
373+
});
374+
303375
// --- getPerformance logic ---
304376

305377
it('calculates performance metrics correctly', () => {

apps/api/src/modules/agent/agent.service.ts

Lines changed: 60 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { Injectable, Inject, NotFoundException, BadRequestException } from '@nestjs/common';
22
import { eq, and, desc } from 'drizzle-orm';
3-
import { agentConfigs, agentDecisions, agentTrainingSnapshots } from '@haip/database';
3+
import { agentConfigs, agentDecisions, agentTrainingSnapshots, auditLogs } from '@haip/database';
44
import { DRIZZLE } from '../../database/database.module';
55
import { WebhookService } from '../webhook/webhook.service';
66
import type {
@@ -165,6 +165,17 @@ export class AgentService {
165165
.where(eq(agentDecisions.id, decisionId))
166166
.returning();
167167

168+
await this.db.insert(auditLogs).values({
169+
propertyId,
170+
action: 'update',
171+
entityType: 'agent_decision',
172+
entityId: decisionId,
173+
userId: userId ?? null,
174+
previousValue: { status: 'pending' },
175+
newValue: { status: 'approved', agentType: decision.agentType, decisionType: decision.decisionType },
176+
description: `Agent decision approved and executed: ${decision.agentType}/${decision.decisionType}`,
177+
});
178+
168179
await this.webhookService.emit(
169180
'agent.decision_executed',
170181
'agent_decision',
@@ -194,6 +205,17 @@ export class AgentService {
194205
.where(eq(agentDecisions.id, decisionId))
195206
.returning();
196207

208+
await this.db.insert(auditLogs).values({
209+
propertyId,
210+
action: 'update',
211+
entityType: 'agent_decision',
212+
entityId: decisionId,
213+
userId: userId ?? null,
214+
previousValue: { status: 'pending' },
215+
newValue: { status: 'rejected', reason: reason ?? null },
216+
description: `Agent decision rejected: ${decision.agentType}/${decision.decisionType}`,
217+
});
218+
197219
return updated;
198220
}
199221

@@ -260,8 +282,13 @@ export class AgentService {
260282
return config;
261283
}
262284

263-
/** Update agent config. */
264-
async updateConfig(propertyId: string, agentType: string, updates: Record<string, unknown>) {
285+
/** Update agent config — writes an audit log row describing the diff. */
286+
async updateConfig(
287+
propertyId: string,
288+
agentType: string,
289+
updates: Record<string, unknown>,
290+
userId?: string,
291+
) {
265292
const config = await this.getOrCreateConfig(propertyId, agentType);
266293

267294
const setValues: Record<string, unknown> = { updatedAt: new Date() };
@@ -277,6 +304,36 @@ export class AgentService {
277304
.where(eq(agentConfigs.id, config.id))
278305
.returning();
279306

307+
// Build diff of the fields the user actually changed (sensitive config only).
308+
const auditFields = ['isEnabled', 'mode', 'autopilotConfidenceThreshold', 'config'];
309+
const diff: Record<string, { old: unknown; new: unknown }> = {};
310+
for (const field of auditFields) {
311+
if (updates[field] === undefined) continue;
312+
const oldValue = (config as any)[field];
313+
const newValue = (updated as any)[field];
314+
// Only record if actually changed (string-compare to handle numeric thresholds).
315+
if (JSON.stringify(oldValue) !== JSON.stringify(newValue)) {
316+
diff[field] = { old: oldValue, new: newValue };
317+
}
318+
}
319+
320+
if (Object.keys(diff).length > 0) {
321+
await this.db.insert(auditLogs).values({
322+
propertyId,
323+
action: 'update',
324+
entityType: 'agent_config',
325+
entityId: config.id,
326+
userId: userId ?? null,
327+
previousValue: Object.fromEntries(
328+
Object.entries(diff).map(([k, v]) => [k, v.old]),
329+
),
330+
newValue: Object.fromEntries(
331+
Object.entries(diff).map(([k, v]) => [k, v.new]),
332+
),
333+
description: `Agent config updated: ${agentType} (${Object.keys(diff).join(', ')})`,
334+
});
335+
}
336+
280337
return updated;
281338
}
282339

apps/api/src/modules/connect/connect-events.service.spec.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,59 @@ describe('ConnectEventsService', () => {
131131
});
132132
});
133133

134+
describe('handleEvent', () => {
135+
it('enqueues a delivery via WebhookDeliveryService for each matching subscription', async () => {
136+
mockDb.select.mockImplementation(() => ({
137+
from: vi.fn().mockReturnValue({
138+
where: vi.fn().mockResolvedValue([mockSubscription]),
139+
}),
140+
}));
141+
142+
const deliveryService = { enqueue: vi.fn().mockResolvedValue({ id: 'del-1' }) };
143+
const svc = new ConnectEventsService(mockDb, deliveryService as any);
144+
145+
await svc.handleEvent({
146+
event: 'reservation.created',
147+
entityType: 'reservation',
148+
entityId: 'res-1',
149+
propertyId: 'prop-1',
150+
data: { foo: 'bar' },
151+
timestamp: new Date().toISOString(),
152+
});
153+
154+
expect(deliveryService.enqueue).toHaveBeenCalledWith(
155+
expect.objectContaining({
156+
eventType: 'reservation.created',
157+
propertyId: 'prop-1',
158+
entityType: 'reservation',
159+
entityId: 'res-1',
160+
}),
161+
'sub-1',
162+
);
163+
});
164+
165+
it('does nothing when no subscriptions match', async () => {
166+
mockDb.select.mockImplementation(() => ({
167+
from: vi.fn().mockReturnValue({
168+
where: vi.fn().mockResolvedValue([mockSubscription]),
169+
}),
170+
}));
171+
const deliveryService = { enqueue: vi.fn() };
172+
const svc = new ConnectEventsService(mockDb, deliveryService as any);
173+
174+
await svc.handleEvent({
175+
event: 'unrelated.event',
176+
entityType: 'x',
177+
entityId: 'y',
178+
propertyId: 'prop-1',
179+
data: {},
180+
timestamp: new Date().toISOString(),
181+
});
182+
183+
expect(deliveryService.enqueue).not.toHaveBeenCalled();
184+
});
185+
});
186+
134187
describe('pollEvents', () => {
135188
it('should return events filtered by type', async () => {
136189
const mockEvents = [

apps/api/src/modules/connect/connect-events.service.ts

Lines changed: 51 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,17 @@
1-
import { Injectable, Inject, NotFoundException } from '@nestjs/common';
1+
import { Injectable, Inject, NotFoundException, Optional } from '@nestjs/common';
22
import { OnEvent } from '@nestjs/event-emitter';
33
import { eq, and, gte, desc } from 'drizzle-orm';
44
import { agentWebhookSubscriptions, auditLogs } from '@haip/database';
55
import { DRIZZLE } from '../../database/database.module';
66
import type { CreateSubscriptionDto } from './dto/agent-event-subscription.dto';
7+
import { WebhookDeliveryService } from '../webhook/webhook-delivery.service';
78

89
@Injectable()
910
export class ConnectEventsService {
10-
constructor(@Inject(DRIZZLE) private readonly db: any) {}
11+
constructor(
12+
@Inject(DRIZZLE) private readonly db: any,
13+
@Optional() private readonly deliveryService?: WebhookDeliveryService,
14+
) {}
1115

1216
/**
1317
* Create an event subscription for an OTAIP agent.
@@ -174,19 +178,55 @@ export class ConnectEventsService {
174178
for (const sub of subscriptions) {
175179
const events = (sub.events ?? []) as string[];
176180
if (events.some((pattern: string) => this.matchesEventPattern(payload.event, pattern))) {
177-
// Log the delivery attempt (no actual HTTP call yet)
178-
await this.db
179-
.update(agentWebhookSubscriptions)
180-
.set({
181-
lastDeliveryAt: new Date(),
182-
lastDeliveryStatus: 'logged',
183-
updatedAt: new Date(),
184-
})
185-
.where(eq(agentWebhookSubscriptions.id, sub.id));
181+
if (this.deliveryService) {
182+
// Enqueue a real HTTP delivery (HMAC-signed, retried).
183+
await this.deliveryService.enqueue(
184+
{
185+
eventType: payload.event,
186+
propertyId: payload.propertyId,
187+
entityType: payload.entityType,
188+
entityId: payload.entityId,
189+
data: payload.data ?? {},
190+
timestamp: payload.timestamp ?? new Date().toISOString(),
191+
},
192+
sub.id,
193+
);
194+
} else {
195+
// Fallback — just log the match (for tests / environments without delivery service).
196+
await this.db
197+
.update(agentWebhookSubscriptions)
198+
.set({
199+
lastDeliveryAt: new Date(),
200+
lastDeliveryStatus: 'logged',
201+
updatedAt: new Date(),
202+
})
203+
.where(eq(agentWebhookSubscriptions.id, sub.id));
204+
}
186205
}
187206
}
188207
}
189208

209+
/**
210+
* List delivery attempts for a subscription (scoped by propertyId).
211+
*/
212+
async listDeliveries(subscriptionId: string, propertyId: string, limit = 50) {
213+
if (!this.deliveryService) return [];
214+
// Verify subscription belongs to propertyId before returning deliveries.
215+
const [subscription] = await this.db
216+
.select()
217+
.from(agentWebhookSubscriptions)
218+
.where(
219+
and(
220+
eq(agentWebhookSubscriptions.id, subscriptionId),
221+
eq(agentWebhookSubscriptions.propertyId, propertyId),
222+
),
223+
);
224+
if (!subscription) {
225+
throw new NotFoundException(`Subscription ${subscriptionId} not found`);
226+
}
227+
return this.deliveryService.listDeliveries(subscriptionId, propertyId, limit);
228+
}
229+
190230
/**
191231
* Match event type against subscription pattern.
192232
* Supports wildcards: 'reservation.*' matches 'reservation.created'.

apps/api/src/modules/connect/connect.controller.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,22 @@ export class ConnectController {
122122
return this.eventsService.testSubscription(id, propertyId);
123123
}
124124

125+
@Get('subscriptions/:id/deliveries')
126+
@ApiOperation({ summary: 'List webhook delivery attempts for a subscription' })
127+
@ApiQuery({ name: 'propertyId', required: true })
128+
@ApiQuery({ name: 'limit', required: false })
129+
async listDeliveries(
130+
@Param('id', ParseUUIDPipe) id: string,
131+
@Query('propertyId', ParseUUIDPipe) propertyId: string,
132+
@Query('limit') limit?: string,
133+
) {
134+
return this.eventsService.listDeliveries(
135+
id,
136+
propertyId,
137+
limit ? parseInt(limit, 10) : undefined,
138+
);
139+
}
140+
125141
// --- Event Polling ---
126142

127143
@Get('events')

0 commit comments

Comments
 (0)