Skip to content

Commit fef3113

Browse files
joshavantjoshavant
committed
fix(kiloclaw): tighten briefing run timeout and delivery observability
Rename timeout test semantics, add focused delivery-utils unit coverage, and emit structured delivery outcome events for sent/skipped/failed paths. Include latest Morning Briefing warmup/source-summary alignment tweak in the same push-ready set.
1 parent c2f8cc8 commit fef3113

6 files changed

Lines changed: 220 additions & 21 deletions

File tree

‎apps/web/src/app/(app)/claw/components/SettingsTab.tsx‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -670,6 +670,13 @@ function MorningBriefingCard({
670670
</p>
671671
)}
672672

673+
{isWarmupState && (
674+
<p className="text-muted-foreground mt-2 text-xs">
675+
Instance is still warming up. Morning Briefing controls will become available once
676+
the gateway is fully ready.
677+
</p>
678+
)}
679+
673680
<p className="text-muted-foreground mt-3 text-xs">{sourceSummaryText}</p>
674681
</div>
675682
</div>
@@ -742,13 +749,6 @@ function MorningBriefingCard({
742749
</div>
743750
</div>
744751

745-
{isWarmupState && (
746-
<p className="text-muted-foreground mt-2 text-xs">
747-
Instance is still warming up. Morning Briefing controls will become available once the
748-
gateway is fully ready.
749-
</p>
750-
)}
751-
752752
{!desiredEnabled && controlsEnabled && (
753753
<p className="text-muted-foreground mt-2 text-xs">
754754
Enable Morning Briefing to get a personalized briefing everyday.

‎services/kiloclaw/plugins/kiloclaw-morning-briefing/src/delivery-utils.test.ts‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
import { describe, expect, it } from 'vitest';
2+
import { parseStoredDelivery, resolveDeliveryRoute } from './delivery-utils';
3+
4+
describe('delivery-utils', () => {
5+
it('parseStoredDelivery ignores malformed entries and keeps valid ones', () => {
6+
const parsed = parseStoredDelivery([
7+
null,
8+
123,
9+
{ channel: 'telegram', status: 'sent', target: '-5055658641' },
10+
{ channel: 'discord', status: 'unknown' },
11+
{ channel: 'slack', status: 'failed', reason: 'send_failed', error: 'send failed' },
12+
{ channel: 'email', status: 'sent' },
13+
{ channel: 'telegram', status: 'skipped', reason: 'bogus_reason' },
14+
]);
15+
16+
expect(parsed).toEqual([
17+
{
18+
channel: 'telegram',
19+
status: 'sent',
20+
target: '-5055658641',
21+
},
22+
{
23+
channel: 'slack',
24+
status: 'failed',
25+
reason: 'send_failed',
26+
error: 'send failed',
27+
},
28+
{
29+
channel: 'telegram',
30+
status: 'skipped',
31+
},
32+
]);
33+
});
34+
35+
it('resolveDeliveryRoute infers single discord fallback channel target', () => {
36+
const resolution = resolveDeliveryRoute({
37+
channel: 'discord',
38+
channelsConfig: {
39+
discord: {
40+
enabled: true,
41+
guilds: {
42+
'guild-1': {
43+
channels: {
44+
'1234567890': { enabled: true },
45+
},
46+
},
47+
},
48+
},
49+
},
50+
});
51+
52+
expect(resolution).toEqual({
53+
configured: true,
54+
route: {
55+
channel: 'discord',
56+
target: 'channel:1234567890',
57+
},
58+
});
59+
});
60+
61+
it('resolveDeliveryRoute infers single slack fallback channel target', () => {
62+
const resolution = resolveDeliveryRoute({
63+
channel: 'slack',
64+
channelsConfig: {
65+
slack: {
66+
enabled: true,
67+
channels: {
68+
C123456: { enabled: true },
69+
},
70+
},
71+
},
72+
});
73+
74+
expect(resolution).toEqual({
75+
configured: true,
76+
route: {
77+
channel: 'slack',
78+
target: 'channel:C123456',
79+
},
80+
});
81+
});
82+
83+
it('resolveDeliveryRoute marks ambiguous fallback when multiple discord channels exist', () => {
84+
const resolution = resolveDeliveryRoute({
85+
channel: 'discord',
86+
channelsConfig: {
87+
discord: {
88+
enabled: true,
89+
guilds: {
90+
'guild-1': {
91+
channels: {
92+
'123': { enabled: true },
93+
'456': { enabled: true },
94+
},
95+
},
96+
},
97+
},
98+
},
99+
});
100+
101+
expect(resolution).toEqual({
102+
configured: true,
103+
route: null,
104+
skipReason: 'ambiguous_target',
105+
});
106+
});
107+
});

‎services/kiloclaw/plugins/kiloclaw-morning-briefing/src/delivery-utils.ts‎

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,17 @@ type DeliveryRoute = {
3232

3333
type DeliveryApi = CommandCapableRuntime & {
3434
config: unknown;
35-
logger: { warn?: (message: string) => void };
35+
logger: { info?: (message: string) => void; warn?: (message: string) => void };
3636
};
3737

3838
type SkipReason = Extract<DeliveryReason, 'missing_target' | 'ambiguous_target'>;
3939

40+
export type DeliveryRouteResolution = {
41+
configured: boolean;
42+
route: DeliveryRoute | null;
43+
skipReason?: SkipReason;
44+
};
45+
4046
function asObject(value: unknown): Record<string, unknown> {
4147
return typeof value === 'object' && value !== null && !Array.isArray(value)
4248
? (value as Record<string, unknown>)
@@ -87,14 +93,10 @@ function collectFallbackTargets(
8793
return channels.map(([channelId]) => `channel:${channelId}`);
8894
}
8995

90-
function resolveDeliveryRoute(params: {
96+
export function resolveDeliveryRoute(params: {
9197
channel: DeliveryChannel;
9298
channelsConfig: Record<string, unknown>;
93-
}): {
94-
configured: boolean;
95-
route: DeliveryRoute | null;
96-
skipReason?: SkipReason;
97-
} {
99+
}): DeliveryRouteResolution {
98100
const rawChannelConfig = asObject(params.channelsConfig[params.channel]);
99101
if (Object.keys(rawChannelConfig).length === 0 || rawChannelConfig.enabled === false) {
100102
return { configured: false, route: null };
@@ -294,6 +296,29 @@ export function formatDeliverySummary(delivery: BriefingDeliveryResult[]): strin
294296
});
295297
}
296298

299+
export function logDeliveryOutcomeEvents(
300+
api: Pick<DeliveryApi, 'logger'>,
301+
delivery: BriefingDeliveryResult[]
302+
): void {
303+
for (const entry of delivery) {
304+
const reason = entry.reason ?? 'none';
305+
const target = entry.target ?? 'none';
306+
const eventLine =
307+
`event=morning_briefing_delivery_outcome` +
308+
` outcome=${entry.status}` +
309+
` channel=${entry.channel}` +
310+
` reason=${reason}` +
311+
` target=${target}`;
312+
api.logger.info?.(eventLine);
313+
if (entry.status === 'failed') {
314+
const detail = entry.error ?? 'unknown_error';
315+
api.logger.warn?.(
316+
`event=morning_briefing_delivery_failure channel=${entry.channel} detail=${detail}`
317+
);
318+
}
319+
}
320+
}
321+
297322
export async function deliverBriefingToConfiguredChannels(
298323
api: DeliveryApi,
299324
markdown: string

‎services/kiloclaw/plugins/kiloclaw-morning-briefing/src/index.lifecycle.test.ts‎

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ type TestHarness = {
4141
accountId?: string;
4242
message: string;
4343
}>;
44+
loggerInfo: ReturnType<typeof vi.fn>;
45+
loggerWarn: ReturnType<typeof vi.fn>;
4446
runCommandWithTimeout: ReturnType<typeof vi.fn>;
4547
};
4648

@@ -240,6 +242,8 @@ async function createHarness(options?: {
240242
let statusHttpHandler: ((_req: unknown, res: FakeResponse) => Promise<void>) | null = null;
241243
let enableHttpHandler: ((req: unknown, res: FakeResponse) => Promise<void>) | null = null;
242244
let runHttpHandler: ((_req: unknown, res: FakeResponse) => Promise<void>) | null = null;
245+
const loggerInfo = vi.fn();
246+
const loggerWarn = vi.fn();
243247

244248
morningBriefingPlugin.register({
245249
runtime: {
@@ -254,7 +258,7 @@ async function createHarness(options?: {
254258
agents: { defaults: { userTimezone: 'America/Chicago' } },
255259
...(options?.omitRuntimeChannelsConfig ? {} : { channels: options?.channelsConfig ?? {} }),
256260
},
257-
logger: { warn: vi.fn() },
261+
logger: { info: loggerInfo, warn: loggerWarn },
258262
registerCommand: (def: { handler: (ctx: { args?: string }) => Promise<{ text: string }> }) => {
259263
commandHandler = def.handler;
260264
},
@@ -286,6 +290,8 @@ async function createHarness(options?: {
286290
runHttpHandler,
287291
cronJobs,
288292
sentMessages,
293+
loggerInfo,
294+
loggerWarn,
289295
runCommandWithTimeout,
290296
};
291297
}
@@ -877,4 +883,63 @@ describe('morning briefing lifecycle', () => {
877883
expect(sendCalls[0]?.[1]).toMatchObject({ timeoutMs: 120_000 });
878884
expect(sendCalls[1]?.[1]).toMatchObject({ timeoutMs: 120_000 });
879885
});
886+
887+
it('emits delivery outcome metric logs for sent/skipped/failed results', async () => {
888+
const harness = await createHarness({
889+
githubAuthReady: true,
890+
githubIssues: [
891+
{
892+
title: 'Delivery observability smoke check',
893+
url: 'https://github.com/Kilo-Org/cloud/issues/910',
894+
updatedAt: '2026-04-25T00:10:00Z',
895+
},
896+
],
897+
channelsConfig: {
898+
telegram: {
899+
enabled: true,
900+
defaultTo: '-5055658641',
901+
},
902+
discord: {
903+
enabled: true,
904+
},
905+
slack: {
906+
enabled: true,
907+
defaultTo: 'channel:C123',
908+
},
909+
},
910+
messageSendFailures: {
911+
slack: 'slack send failed',
912+
},
913+
});
914+
915+
const response = new FakeResponse();
916+
await harness.runHttpHandler({}, response);
917+
expect(response.statusCode).toBe(200);
918+
919+
const infoMessages = harness.loggerInfo.mock.calls.map(call => String(call[0]));
920+
expect(
921+
infoMessages.some(message =>
922+
message.includes('event=morning_briefing_delivery_outcome outcome=sent channel=telegram')
923+
)
924+
).toBe(true);
925+
expect(
926+
infoMessages.some(message =>
927+
message.includes('event=morning_briefing_delivery_outcome outcome=skipped channel=discord')
928+
)
929+
).toBe(true);
930+
expect(
931+
infoMessages.some(message =>
932+
message.includes('event=morning_briefing_delivery_outcome outcome=failed channel=slack')
933+
)
934+
).toBe(true);
935+
936+
const warnMessages = harness.loggerWarn.mock.calls.map(call => String(call[0]));
937+
expect(
938+
warnMessages.some(message =>
939+
message.includes(
940+
'event=morning_briefing_delivery_failure channel=slack detail=slack send failed'
941+
)
942+
)
943+
).toBe(true);
944+
});
880945
});

‎services/kiloclaw/plugins/kiloclaw-morning-briefing/src/index.ts‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
type BriefingDeliveryResult,
99
deliverBriefingToConfiguredChannels,
1010
formatDeliverySummary,
11+
logDeliveryOutcomeEvents,
1112
parseStoredDelivery,
1213
} from './delivery-utils';
1314
import { CommandExecutionError, runCommand } from './command-utils';
@@ -226,7 +227,7 @@ async function removeDuplicateBriefingCronJobs(
226227
) => Promise<{ stdout: string; stderr: string; code: number | null }>;
227228
};
228229
};
229-
logger: { warn?: (message: string) => void };
230+
logger: { info?: (message: string) => void; warn?: (message: string) => void };
230231
},
231232
canonicalId: string
232233
): Promise<void> {
@@ -266,7 +267,7 @@ function resolveDefaults(api: {
266267
}
267268

268269
function resolveEffectiveTimezone(
269-
api: { logger: { warn?: (message: string) => void } },
270+
api: { logger: { info?: (message: string) => void; warn?: (message: string) => void } },
270271
timezone: string,
271272
context: 'enable' | 'schedule' | 'date'
272273
): string {
@@ -408,7 +409,7 @@ async function ensureCronJob(
408409
) => Promise<{ stdout: string; stderr: string; code: number | null }>;
409410
};
410411
};
411-
logger: { warn?: (message: string) => void };
412+
logger: { info?: (message: string) => void; warn?: (message: string) => void };
412413
},
413414
config: StoredConfig
414415
): Promise<{ cronJobId: string; cron: string; timezone: string }> {
@@ -787,7 +788,7 @@ async function generateBriefing(
787788
};
788789
};
789790
config: unknown;
790-
logger: { warn?: (message: string) => void };
791+
logger: { info?: (message: string) => void; warn?: (message: string) => void };
791792
},
792793
dateKey: string
793794
): Promise<{
@@ -850,6 +851,7 @@ async function generateBriefing(
850851
error: errorText,
851852
}));
852853
}
854+
logDeliveryOutcomeEvents(api, delivery);
853855

854856
await patchStoredStatus(paths, {
855857
lastGeneratedDate: dateKey,
@@ -911,7 +913,7 @@ async function resolveDateKeyForOffset(
911913
};
912914
};
913915
pluginConfig?: Record<string, unknown>;
914-
logger: { warn?: (message: string) => void };
916+
logger: { info?: (message: string) => void; warn?: (message: string) => void };
915917
},
916918
offset: number
917919
): Promise<string> {

‎services/kiloclaw/src/routes/platform-morning-briefing.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ describe('platform morning-briefing warm-up handling', () => {
210210
expect(runMorningBriefing).toHaveBeenCalledTimes(1);
211211
});
212212

213-
it('returns warm-up payload for run timeout instead of generic 500', async () => {
213+
it('returns timeout code for run timeout instead of generic 500', async () => {
214214
const runMorningBriefing = vi
215215
.fn<() => Promise<unknown>>()
216216
.mockRejectedValue(

0 commit comments

Comments
 (0)