Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
81 changes: 81 additions & 0 deletions apps/mobile/src/app/(app)/agent-chat/[session-id].mounted.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1424,3 +1424,84 @@ describe('SessionDetailScreen malformed part transcript', () => {
expect(findByType(renderer.root, 'QueryError')).toHaveLength(0);
});
});

// KILO-APP-BZ: a patch part with no `files` on the wire crashed the whole agent
// chat screen (`part.files.length` in partRendersContent). This repro mounts the
// real route and renders the real transcript build and bubbles over the stored
// messages, so the screen the reporter screenshotted — the root error boundary
// with "Something went wrong" — is what the assertion rules out.
describe('SessionDetailScreen malformed patch part transcript', () => {
it('renders the transcript without crashing when a patch part arrives with no files', async () => {
realTranscriptProbe.active = true;
onTestFinished(() => {
realTranscriptProbe.active = false;
});

// The wire omits `files` (per-event schemas are `.passthrough()`), so the
// cast is the fixture, not a smell. The patch part's id sorts before the
// answer part's id (storage orders parts by id), so the malformed part is
// the first one the transcript's `.some(partRendersContent)` visits.
const patchNoFiles = {
id: 'part-a-broken-patch',
sessionID: 'sess-1',
messageID: 'msg-assistant-broken-patch',
type: 'patch',
hash: 'abc',
} as unknown as Part;
const assistantInfo: AssistantMessage = {
id: 'msg-assistant-broken-patch',
sessionID: 'sess-1',
role: 'assistant',
time: { created: 2 },
parentID: 'msg-user-question',
modelID: 'claude',
providerID: 'anthropic',
mode: 'code',
agent: 'build',
path: { cwd: '/', root: '/' },
cost: 0,
tokens: { input: 0, output: 0, reasoning: 0, cache: { read: 0, write: 0 } },
};
rootPageMock.mockResolvedValueOnce({
kind: 'success',
info: { id: 'sess-1' },
messages: [
{
info: stubUserMessage({ id: 'msg-user-question', sessionID: 'sess-1' }),
parts: [
stubTextPart({
id: 'part-question',
sessionID: 'sess-1',
messageID: 'msg-user-question',
text: 'Question about the patch',
}),
],
},
{
info: assistantInfo,
parts: [
patchNoFiles,
{
id: 'part-z-answer',
sessionID: 'sess-1',
messageID: 'msg-assistant-broken-patch',
type: 'text',
text: 'Answer visible after broken patch',
},
],
},
],
nextCursor: null,
omittedItemCount: 0,
} satisfies SessionSnapshotPageOutcome);

useLocalSearchParamsMock.mockReturnValue({ 'session-id': 'sess-1', organizationId: 'org-a' });
const renderer = await mountRoute();

expect(findByType(renderer.root, 'SessionDetailContent')).toHaveLength(1);
const transcript = transcriptText(renderer);
expect(transcript).toContain('Question about the patch');
expect(transcript).toContain('Answer visible after broken patch');
expect(findByType(renderer.root, 'QueryError')).toHaveLength(0);
});
});
13 changes: 13 additions & 0 deletions apps/mobile/src/components/agents/message-visibility.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,19 @@ describe('partRendersContent', () => {
it('returns false for a patch part with no files', () => {
expect(partRendersContent(patchPart([]))).toBe(false);
});

it('returns false for a patch part whose files field the wire omitted', () => {
// Sentry KILO-APP-BZ: the live `message.part.updated` payload is parsed
// with `.passthrough()`, so a patch part can reach storage without `files`.
const part = {
id: 'p4',
sessionID: 's1',
messageID: 'm1',
type: 'patch',
hash: 'abc',
} as unknown as Part;
expect(partRendersContent(part)).toBe(false);
});
});

describe('messageRendersContent', () => {
Expand Down
12 changes: 11 additions & 1 deletion apps/mobile/src/components/agents/message-visibility.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,17 @@ export function partRendersContent(part: Part): boolean {
// property of the part, not of the stream.
return shouldRenderReasoningPart(part, false);
}
return isFilePart(part) || isCompactionPart(part) || (isPatchPart(part) && part.files.length > 0);
// A patch part whose `files` the wire omitted means the same as an empty one:
// nothing to draw. The generated type declares the array unconditionally
// (the live per-event schemas are `.passthrough()`), so the runtime shape is
// read through a declared optional. The chat processor fills the array, but a
// part can still reach a reader unfixed (a unit caller, a stored part from an
// older build), and a throw here replaces the whole app with the error
// boundary (Sentry KILO-APP-BZ).
const files = (part as { files?: readonly string[] }).files;
return (
isFilePart(part) || isCompactionPart(part) || (isPatchPart(part) && (files ?? []).length > 0)
);
}

/**
Expand Down
45 changes: 45 additions & 0 deletions packages/cloud-agent-sdk/src/chat-processor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type {
FilePart,
Message,
Part,
PatchPart,
ReasoningPart,
TextPart,
ToolPart,
Expand Down Expand Up @@ -506,6 +507,50 @@ describe('createChatProcessor', () => {
expect(stored).toHaveLength(1);
expect((stored[0] satisfies Part as ReasoningPart).text).toBe('streamed reasoning');
});

// --- files-less patch part normalization (KILO-APP-BZ) ---
// The generated Part types declare `files: Array<string>`, but the wire can
// omit the field (per-event schemas are `.passthrough()`), so a files-less
// patch part used to be stored verbatim and crashed the mobile transcript
// reader on `part.files.length`.

it('stores a patch part whose files field is missing with files: []', () => {
const storage = createMemoryStorage();
const processor = createChatProcessor(storage);
// The cast is the fixture: the wire really omits `files`.
const part = {
id: 'part-patch',
sessionID: 'ses-1',
messageID: 'msg-1',
type: 'patch' as const,
hash: 'abc',
} as unknown as Part;

processor.process({ type: 'message.part.updated', part });

const stored = storage.getParts('msg-1');
expect(stored).toHaveLength(1);
expect((stored[0] satisfies Part as PatchPart).files).toEqual([]);
});

it('stores a patch part whose files were sent intact', () => {
const storage = createMemoryStorage();
const processor = createChatProcessor(storage);
const part: Part = {
id: 'part-patch',
sessionID: 'ses-1',
messageID: 'msg-1',
type: 'patch',
hash: 'abc',
files: ['src/a.ts', 'src/b.ts'],
};

processor.process({ type: 'message.part.updated', part });

const stored = storage.getParts('msg-1');
expect(stored).toHaveLength(1);
expect((stored[0] satisfies Part as PatchPart).files).toEqual(['src/a.ts', 'src/b.ts']);
});
});

describe('message.part.delta', () => {
Expand Down
15 changes: 10 additions & 5 deletions packages/cloud-agent-sdk/src/chat-processor.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import { normalizeMissingPartText, stripPartContentIfFile } from './part-utils';
import {
normalizeMissingPartText,
normalizeMissingPatchFiles,
stripPartContentIfFile,
} from './part-utils';
import type { ChatEvent } from './normalizer';
import type { SessionStorage } from './storage/types';
import type { UserMessage, TextPart, Part } from '@kilocode/app-shared/opencode';
Expand Down Expand Up @@ -110,10 +114,11 @@ function createChatProcessor(
break;
case 'message.part.updated': {
// Normalize BEFORE the strip/guard logic: the wire can omit `text` on
// text/reasoning parts (KILO-APP-99), and the empty-text guard below
// must also see textless updates so they cannot clobber already
// streamed text. Identity-preserving for everything else.
const normalized = normalizeMissingPartText(event.part);
// text/reasoning parts (KILO-APP-99) and `files` on patch parts
// (KILO-APP-BZ), and the empty-text guard below must also see
// textless updates so they cannot clobber already streamed text.
// Identity-preserving for everything else.
const normalized = normalizeMissingPatchFiles(normalizeMissingPartText(event.part));
if (options?.onToolAttachment) {
emitToolAttachmentsBeforeStrip(normalized, options.onToolAttachment);
}
Expand Down
62 changes: 60 additions & 2 deletions packages/cloud-agent-sdk/src/part-utils.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import type { Part, ReasoningPart, StepFinishPart, TextPart, ToolPart } from './types';
import { getStepFinishRoutedModel, normalizeMissingPartText } from './part-utils';
import type { Part, PatchPart, ReasoningPart, StepFinishPart, TextPart, ToolPart } from './types';
import {
getStepFinishRoutedModel,
normalizeMissingPartText,
normalizeMissingPatchFiles,
} from './part-utils';

function stepFinishPart(overrides: Partial<StepFinishPart> = {}): StepFinishPart {
return {
Expand Down Expand Up @@ -192,3 +196,57 @@ describe('normalizeMissingPartText', () => {
expect(normalizeMissingPartText(stepFinishPart)).toBe(stepFinishPart);
});
});

describe('normalizeMissingPatchFiles', () => {
function patchPart(overrides: Partial<PatchPart> = {}): PatchPart {
return {
id: 'p-patch',
sessionID: 'ses-1',
messageID: 'msg-1',
type: 'patch',
hash: 'abc',
files: ['src/a.ts'],
...overrides,
};
}

it('returns the identical object when files is already an array', () => {
const part = patchPart();

// Identity is preserved so memoized stored messages keep their reference.
expect(normalizeMissingPatchFiles(part)).toBe(part);
});

it('fills an empty array for a patch part the wire sent without files', () => {
const part = {
id: 'p-patch',
sessionID: 'ses-1',
messageID: 'msg-1',
type: 'patch',
hash: 'abc',
} as unknown as PatchPart;

const normalized = normalizeMissingPatchFiles(part);
expect(normalized).not.toBe(part);
expect((normalized satisfies Part as PatchPart).files).toEqual([]);
expect(normalized.id).toBe('p-patch');
});

it('fills an empty array for a files field that is not an array', () => {
const part = { ...patchPart(), files: null } as unknown as PatchPart;

expect((normalizeMissingPatchFiles(part) satisfies Part as PatchPart).files).toEqual([]);
});

it('passes non-patch parts through unchanged', () => {
const textPart: TextPart = {
id: 'p-text',
sessionID: 'ses-1',
messageID: 'msg-1',
type: 'text',
text: 'hello',
};

expect(normalizeMissingPatchFiles(textPart)).toBe(textPart);
});
});
23 changes: 23 additions & 0 deletions packages/cloud-agent-sdk/src/part-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,29 @@ export function normalizeMissingPartText(part: Part): Part {
return { ...part, text: '' };
}

/**
* The generated `Part` types declare `files: Array<string>` on patch parts, but
* the wire can omit the field (the per-event schemas are `.passthrough()`), and
* a files-less part stored verbatim crashes every mobile reader
* (`part.files.length` — Sentry KILO-APP-BZ). Normalize once at the
* chat-processor seam — the single funnel every part write goes through — so
* all downstream readers see an array. A missing list and an empty one mean the
* same thing to a reader: the standalone patch part carries only file paths, so
* there is nothing to draw.
*
* Identity-preserving: when nothing needs fixing the SAME object is returned
* so memoized stored messages keep their reference.
*/
export function normalizeMissingPatchFiles(part: Part): Part {
if (part.type !== 'patch') {
return part;
}
if (Array.isArray(part.files)) {
return part;
}
return { ...part, files: [] };
}

/**
* The concrete routed model stamped by the CLI onto step-finish parts for
* kilo-auto turns (`{ providerID, modelID }`). Preserved at runtime on both
Expand Down