diff --git a/.changeset/task-tool-empty-result.md b/.changeset/task-tool-empty-result.md new file mode 100644 index 0000000000..2ec2ed66ab --- /dev/null +++ b/.changeset/task-tool-empty-result.md @@ -0,0 +1,5 @@ +--- +"@kilocode/cli": patch +--- + +Fix the task tool intermittently returning an empty result. Subagents that ran with memory context had a synthetic marker part appended after their answer, which was picked up as the final text part and surfaced as an empty `` to the parent agent. The task tool now ignores synthetic, ignored, and empty text parts, and background jobs no longer let an empty run overwrite an earlier successful result, so resumed tasks keep their real output. diff --git a/packages/core/src/background-job.ts b/packages/core/src/background-job.ts index cdffd212bc..285e65bf81 100644 --- a/packages/core/src/background-job.ts +++ b/packages/core/src/background-job.ts @@ -137,7 +137,7 @@ export const make = Effect.gen(function* () { if (job.info.status !== "running") return [{ info: snapshot(job) }, jobs] const pending = job.pending - 1 const output = - Exit.isSuccess(exit) && (!job.output || sequence > job.output.sequence) + Exit.isSuccess(exit) && exit.value && sequence > (job.output?.sequence ?? -1) // kilocode_change - empty outputs never clobber; only the latest non-empty result wins (#13469) ? { sequence, text: exit.value } : job.output if (Exit.isSuccess(exit) && pending > 0) { diff --git a/packages/core/test/background-job.test.ts b/packages/core/test/background-job.test.ts index 5ad1e061a5..8be1769c5e 100644 --- a/packages/core/test/background-job.test.ts +++ b/packages/core/test/background-job.test.ts @@ -86,6 +86,27 @@ describe("BackgroundJob", () => { }).pipe(Effect.provide(jobsLayer)), ) + // kilocode_change start - regression for #13469: an empty extended run must not clobber an earlier non-empty result + it.live("keeps the earlier non-empty output when an extended run returns empty", () => + Effect.gen(function* () { + const jobs = yield* BackgroundJob.Service + const first = yield* Deferred.make() + const job = yield* jobs.start({ + type: "test", + run: Deferred.await(first).pipe(Effect.as("real answer")), + }) + + expect(yield* jobs.extend({ id: job.id, run: Effect.succeed("") })).toBe(true) + + yield* Deferred.succeed(first, undefined) + expect(yield* jobs.wait({ id: job.id })).toMatchObject({ + timedOut: false, + info: { status: "completed", output: "real answer" }, + }) + }).pipe(Effect.provide(jobsLayer)), + ) + // kilocode_change end + it.live("interrupts live work without promising settlement after the owning process-local scope closes", () => Effect.gen(function* () { const scope = yield* Scope.make() diff --git a/packages/opencode/src/tool/task.ts b/packages/opencode/src/tool/task.ts index ee9f4d1b3f..f43608f7f5 100644 --- a/packages/opencode/src/tool/task.ts +++ b/packages/opencode/src/tool/task.ts @@ -274,7 +274,13 @@ export const TaskTool = Tool.define( return yield* Effect.fail(new Error(`${errorMessage(result.info.error)}\n${resumeHint(nextSession.id)}`)) } // kilocode_change end - return result.parts.findLast((item) => item.type === "text")?.text ?? "" + // kilocode_change start - ignore synthetic/ignored/empty text parts (e.g. the memory marker) when picking the task result (#13469) + return ( + result.parts + .filter((item): item is MessageV2.TextPart => item.type === "text") + .findLast((item) => !item.synthetic && !item.ignored && item.text.length > 0)?.text ?? "" + ) + // kilocode_change end }, Effect.ensuring(KiloTaskBackgroundProcess.finish(nextSession.id)), ) // kilocode_change - transfer inherited processes when the child run ends diff --git a/packages/opencode/test/tool/task.test.ts b/packages/opencode/test/tool/task.test.ts index ec2414e68a..905026238b 100644 --- a/packages/opencode/test/tool/task.test.ts +++ b/packages/opencode/test/tool/task.test.ts @@ -502,6 +502,70 @@ describe("tool.task", () => { }), ) + // kilocode_change start - regression for #13469: a trailing synthetic empty text part (the memory marker) + // or an ignored length-warning part must not be picked as the task result + it.instance("returns the real answer when synthetic or ignored text parts trail it", () => + Effect.gen(function* () { + const { chat, assistant } = yield* seed() + const tool = yield* TaskTool + const def = yield* tool.init() + const promptOps: TaskPromptOps = { + ...stubOps(), + prompt: (input) => + Effect.sync(() => { + const rep = reply(input, "the actual answer") + const id = MessageID.ascending() + return { + ...rep, + parts: [ + ...rep.parts, + { + id: PartID.ascending(), + messageID: id, + sessionID: input.sessionID, + type: "text", + text: "output limit hit", + ignored: true, + }, + { + id: PartID.ascending(), + messageID: id, + sessionID: input.sessionID, + type: "text", + text: "", + synthetic: true, + ignored: true, + }, + ], + } + }), + } + + const result = yield* def.execute( + { + description: "inspect bug", + prompt: "look into the cache key path", + subagent_type: "general", + }, + { + sessionID: chat.id, + messageID: assistant.id, + agent: "build", + abort: new AbortController().signal, + extra: { promptOps }, + messages: [], + metadata: () => Effect.void, + ask: () => Effect.void, + }, + ) + + expect(result.output).toContain("the actual answer") + expect(result.output).not.toContain("output limit hit") + expect(result.output).not.toContain("") + }), + ) + // kilocode_change end + it.instance("prevents subagents from launching subagents by default", () => Effect.gen(function* () { const sessions = yield* Session.Service