Skip to content

Commit 4de2f98

Browse files
authored
Make sure we don't override terminal content (agentclientprotocol#328)
- **Fix sending markdown content that replaces terminal content** - **Make sure to always updated content to be safe**
1 parent f5a5c73 commit 4de2f98

3 files changed

Lines changed: 28 additions & 619 deletions

File tree

src/tests/tools.test.ts

Lines changed: 26 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -451,27 +451,19 @@ describe("Bash terminal output", () => {
451451
type: "content",
452452
content: {
453453
type: "text",
454-
text: "```sh\nfile1.txt\nfile2.txt\n```",
454+
text: "```console\nfile1.txt\nfile2.txt\n```",
455455
},
456456
},
457457
],
458458
});
459459
expect(update._meta).toBeUndefined();
460460
});
461461

462-
it("should return formatted content with _meta when supportsTerminalOutput is true", () => {
462+
it("should return no content with _meta when supportsTerminalOutput is true", () => {
463463
const toolResult = makeBashResult("file1.txt\nfile2.txt", "", 0);
464464
const update = toolUpdateFromToolResult(toolResult, bashToolUse, true);
465465

466-
expect(update.content).toEqual([
467-
{
468-
type: "content",
469-
content: {
470-
type: "text",
471-
text: "```sh\nfile1.txt\nfile2.txt\n```",
472-
},
473-
},
474-
]);
466+
expect(update.content).toEqual([{ type: "terminal", terminalId: "toolu_bash" }]);
475467
expect(update._meta).toEqual({
476468
terminal_info: {
477469
terminal_id: "toolu_bash",
@@ -508,17 +500,17 @@ describe("Bash terminal output", () => {
508500
type: "content",
509501
content: {
510502
type: "text",
511-
text: "```sh\nsome error output\n```",
503+
text: "```console\nsome error output\n```",
512504
},
513505
},
514506
]);
515507
});
516508

517-
it("should return empty content array with _meta when output is empty and supportsTerminalOutput is true", () => {
509+
it("should return no content with _meta when output is empty and supportsTerminalOutput is true", () => {
518510
const toolResult = makeBashResult("", "", 0);
519511
const update = toolUpdateFromToolResult(toolResult, bashToolUse, true);
520512

521-
expect(update.content).toEqual([]);
513+
expect(update.content).toEqual([{ type: "terminal", terminalId: "toolu_bash" }]);
522514
expect(update._meta).toEqual({
523515
terminal_info: {
524516
terminal_id: "toolu_bash",
@@ -552,25 +544,17 @@ describe("Bash terminal output", () => {
552544
type: "content",
553545
content: {
554546
type: "text",
555-
text: "```sh\nhello\n```",
547+
text: "```console\nhello\n```",
556548
},
557549
},
558550
]);
559551
});
560552

561-
it("should trim trailing whitespace from output in content but preserve it in _meta data", () => {
553+
it("should preserve trailing whitespace in _meta data when supportsTerminalOutput is true", () => {
562554
const toolResult = makeBashResult("hello\n\n\n", "", 0);
563555
const update = toolUpdateFromToolResult(toolResult, bashToolUse, true);
564556

565-
expect(update.content).toEqual([
566-
{
567-
type: "content",
568-
content: {
569-
type: "text",
570-
text: "```sh\nhello\n```",
571-
},
572-
},
573-
]);
557+
expect(update.content).toEqual([{ type: "terminal", terminalId: "toolu_bash" }]);
574558
expect(update._meta?.terminal_output?.data).toBe("hello\n\n\n");
575559
});
576560

@@ -595,27 +579,19 @@ describe("Bash terminal output", () => {
595579
type: "content",
596580
content: {
597581
type: "text",
598-
text: "```sh\nCargo.lock\nCargo.toml\nREADME.md\n```",
582+
text: "```console\nCargo.lock\nCargo.toml\nREADME.md\n```",
599583
},
600584
},
601585
],
602586
});
603587
expect(update._meta).toBeUndefined();
604588
});
605589

606-
it("should format string content with _meta when supportsTerminalOutput is true", () => {
590+
it("should return no content with _meta when supportsTerminalOutput is true", () => {
607591
const toolResult = makeStringBashResult("Cargo.lock\nCargo.toml\nREADME.md");
608592
const update = toolUpdateFromToolResult(toolResult, bashToolUse, true);
609593

610-
expect(update.content).toEqual([
611-
{
612-
type: "content",
613-
content: {
614-
type: "text",
615-
text: "```sh\nCargo.lock\nCargo.toml\nREADME.md\n```",
616-
},
617-
},
618-
]);
594+
expect(update.content).toEqual([{ type: "terminal", terminalId: "toolu_bash" }]);
619595
expect(update._meta).toEqual({
620596
terminal_info: { terminal_id: "toolu_bash" },
621597
terminal_output: { terminal_id: "toolu_bash", data: "Cargo.lock\nCargo.toml\nREADME.md" },
@@ -649,11 +625,11 @@ describe("Bash terminal output", () => {
649625
expect(update).toEqual({});
650626
});
651627

652-
it("should return empty content with _meta for empty string content with terminal support", () => {
628+
it("should return no content with _meta for empty string content with terminal support", () => {
653629
const toolResult = makeStringBashResult("");
654630
const update = toolUpdateFromToolResult(toolResult, bashToolUse, true);
655631

656-
expect(update.content).toEqual([]);
632+
expect(update.content).toEqual([{ type: "terminal", terminalId: "toolu_bash" }]);
657633
expect(update._meta).toEqual({
658634
terminal_info: { terminal_id: "toolu_bash" },
659635
terminal_output: { terminal_id: "toolu_bash", data: "" },
@@ -676,7 +652,7 @@ describe("Bash terminal output", () => {
676652
type: "content",
677653
content: {
678654
type: "text",
679-
text: "```sh\nline1\nline2\n```",
655+
text: "```console\nline1\nline2\n```",
680656
},
681657
},
682658
],
@@ -794,7 +770,7 @@ describe("Bash terminal output", () => {
794770
expect((notifications[0].update as any)._meta).not.toHaveProperty("terminal_output");
795771
});
796772

797-
it("should still include formatted content regardless of terminal_output support", () => {
773+
it("should include formatted content only when terminal_output is not supported", () => {
798774
const withSupport = toAcpNotifications(
799775
[toolResult],
800776
"assistant",
@@ -814,21 +790,22 @@ describe("Bash terminal output", () => {
814790
mockLogger,
815791
);
816792

817-
const expectedContent = [
793+
// With support: output is delivered via terminal_output _meta, content references the terminal widget
794+
expect(withSupport).toHaveLength(2);
795+
expect((withSupport[1].update as any).content).toEqual([
796+
{ type: "terminal", terminalId: "toolu_bash" },
797+
]);
798+
799+
// Without support: content is on the only notification
800+
expect((withoutSupport[0].update as any).content).toEqual([
818801
{
819802
type: "content",
820803
content: {
821804
type: "text",
822-
text: "```sh\nfile1.txt\nfile2.txt\n```",
805+
text: "```console\nfile1.txt\nfile2.txt\n```",
823806
},
824807
},
825-
];
826-
827-
// With support: content is on the second (completion) notification
828-
expect(withSupport).toHaveLength(2);
829-
expect((withSupport[1].update as any).content).toEqual(expectedContent);
830-
// Without support: content is on the only notification
831-
expect((withoutSupport[0].update as any).content).toEqual(expectedContent);
808+
]);
832809
});
833810

834811
it("should preserve claudeCode in _meta alongside terminal_exit on completion notification", () => {

0 commit comments

Comments
 (0)