Skip to content

Commit 2bf886b

Browse files
obviyusobviyus
committed
fix(acp): reuse progress commentary config
1 parent 9ac9456 commit 2bf886b

10 files changed

Lines changed: 263 additions & 47 deletions

File tree

‎docs/gateway/configuration-reference.md‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1178,7 +1178,6 @@ Notes:
11781178
hiddenBoundarySeparator: "paragraph", // none | space | newline | paragraph
11791179
maxOutputChars: 50000,
11801180
maxSessionUpdateChars: 500,
1181-
assistantCommentary: false,
11821181
},
11831182

11841183
runtime: {
@@ -1202,7 +1201,6 @@ Notes:
12021201
- `stream.hiddenBoundarySeparator`: separator before visible text after hidden tool events (default: `"paragraph"`).
12031202
- `stream.maxOutputChars`: maximum assistant output characters projected per ACP turn.
12041203
- `stream.maxSessionUpdateChars`: maximum characters for projected ACP status/update lines.
1205-
- `stream.assistantCommentary`: when `true`, relay assistant commentary and ACP status progress allowed by `stream.tagVisibility` into ACP parent stream updates. Defaults to `false`.
12061204
- `stream.tagVisibility`: record of tag names to boolean visibility overrides for streamed events.
12071205
- `runtime.ttlMinutes`: idle TTL in minutes for ACP session workers before eligible cleanup.
12081206
- `runtime.installCommand`: optional install command to run when bootstrapping an ACP runtime environment.

‎docs/tools/acp-agents.md‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -548,10 +548,9 @@ Two ways to start an ACP session:
548548
requester session as system events. Accepted responses include
549549
`streamLogPath` pointing to a session-scoped JSONL log
550550
(`<sessionId>.acp-stream.jsonl`) you can tail for full relay history.
551-
Assistant commentary and ACP status progress text are hidden by default; set
552-
`acp.stream.assistantCommentary: true` to include commentary in parent stream
553-
updates while keeping final-answer delivery unchanged. Status progress still
554-
honors `acp.stream.tagVisibility`, so tags such as `plan` remain hidden
551+
Assistant commentary and ACP status progress text are shown for progress-mode
552+
parent channels unless `streaming.progress.commentary=false`. Status progress
553+
still honors `acp.stream.tagVisibility`, so tags such as `plan` remain hidden
555554
unless explicitly enabled.
556555
</ParamField>
557556

‎src/agents/acp-spawn-parent-stream.test.ts‎

Lines changed: 169 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
/** Tests ACP child-to-parent stream relay notices, routing, and log path resolution. */
22
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
3+
import type { OpenClawConfig } from "../config/types.openclaw.js";
34
import { mergeMockedModule } from "../test-utils/vitest-module-mocks.js";
45

56
const enqueueSystemEventMock = vi.fn();
@@ -48,10 +49,29 @@ vi.mock("../config/sessions/paths.js", async () => {
4849
});
4950

5051
let emitAgentEvent: typeof import("../infra/agent-events.js").emitAgentEvent;
51-
let resolveAcpProjectionSettings: typeof import("../auto-reply/reply/acp-stream-settings.js").resolveAcpProjectionSettings;
5252
let resolveAcpSpawnStreamLogPath: typeof import("./acp-spawn-parent-stream.js").resolveAcpSpawnStreamLogPath;
5353
let startAcpSpawnParentStreamRelay: typeof import("./acp-spawn-parent-stream.js").startAcpSpawnParentStreamRelay;
5454

55+
const progressCommentaryDeliveryContext = {
56+
channel: "forum",
57+
to: "-1001234567890",
58+
accountId: "default",
59+
threadId: 1122,
60+
};
61+
62+
function progressModeConfig(acp?: OpenClawConfig["acp"]): OpenClawConfig {
63+
return {
64+
...(acp ? { acp } : {}),
65+
channels: {
66+
forum: {
67+
streaming: {
68+
mode: "progress",
69+
},
70+
},
71+
},
72+
};
73+
}
74+
5575
function collectedTexts() {
5676
return enqueueSystemEventMock.mock.calls.map((call) =>
5777
typeof call[0] === "string" ? call[0] : (JSON.stringify(call[0]) ?? ""),
@@ -80,7 +100,6 @@ function firstMockCall(
80100
describe("startAcpSpawnParentStreamRelay", () => {
81101
beforeAll(async () => {
82102
({ emitAgentEvent } = await import("../infra/agent-events.js"));
83-
({ resolveAcpProjectionSettings } = await import("../auto-reply/reply/acp-stream-settings.js"));
84103
({ resolveAcpSpawnStreamLogPath, startAcpSpawnParentStreamRelay } =
85104
await import("./acp-spawn-parent-stream.js"));
86105
});
@@ -487,19 +506,20 @@ describe("startAcpSpawnParentStreamRelay", () => {
487506
relay.dispose();
488507
});
489508

490-
it("relays commentary-phase assistant text when enabled", () => {
509+
it("relays commentary-phase assistant text in parent progress mode by default", () => {
491510
const relay = startAcpSpawnParentStreamRelay({
492-
runId: "run-commentary-enabled",
511+
runId: "run-commentary-default",
493512
parentSessionKey: "agent:main:main",
494-
childSessionKey: "agent:codex:acp:child-commentary-enabled",
513+
childSessionKey: "agent:codex:acp:child-commentary-default",
495514
agentId: "codex",
515+
cfg: progressModeConfig(),
516+
deliveryContext: progressCommentaryDeliveryContext,
496517
streamFlushMs: 10,
497518
noOutputNoticeMs: 120_000,
498-
assistantCommentary: true,
499519
});
500520

501521
emitAgentEvent({
502-
runId: "run-commentary-enabled",
522+
runId: "run-commentary-default",
503523
stream: "assistant",
504524
data: {
505525
delta: "checking thread context; then post a tight progress reply here.",
@@ -516,24 +536,149 @@ describe("startAcpSpawnParentStreamRelay", () => {
516536
relay.dispose();
517537
});
518538

519-
it("relays ACP status progress when assistant commentary and tag visibility are enabled", () => {
539+
it("suppresses commentary-phase assistant text when parent progress commentary is disabled", () => {
520540
const relay = startAcpSpawnParentStreamRelay({
521-
runId: "run-status-commentary-enabled",
541+
runId: "run-commentary-disabled",
522542
parentSessionKey: "agent:main:main",
523-
childSessionKey: "agent:codex:acp:child-status-commentary-enabled",
543+
childSessionKey: "agent:codex:acp:child-commentary-disabled",
524544
agentId: "codex",
545+
cfg: {
546+
channels: {
547+
forum: {
548+
streaming: {
549+
mode: "progress",
550+
progress: {
551+
commentary: false,
552+
},
553+
},
554+
},
555+
},
556+
},
557+
deliveryContext: progressCommentaryDeliveryContext,
525558
streamFlushMs: 10,
526559
noOutputNoticeMs: 120_000,
527-
assistantCommentary: true,
528-
acpProjectionSettings: resolveAcpProjectionSettings({
529-
acp: {
530-
stream: {
531-
tagVisibility: {
532-
plan: true,
560+
});
561+
562+
emitAgentEvent({
563+
runId: "run-commentary-disabled",
564+
stream: "assistant",
565+
data: {
566+
delta: "checking thread context; then post a tight progress reply here.",
567+
phase: "commentary",
568+
},
569+
});
570+
vi.advanceTimersByTime(15);
571+
572+
expectNoTextWithFragment(collectedTexts(), "checking thread context");
573+
relay.dispose();
574+
});
575+
576+
it("inherits parent channel progress mode for account commentary overrides", () => {
577+
const relay = startAcpSpawnParentStreamRelay({
578+
runId: "run-account-commentary-enabled",
579+
parentSessionKey: "agent:main:main",
580+
childSessionKey: "agent:codex:acp:child-account-commentary-enabled",
581+
agentId: "codex",
582+
cfg: {
583+
channels: {
584+
forum: {
585+
streaming: {
586+
mode: "progress",
587+
},
588+
accounts: {
589+
work: {
590+
streaming: {
591+
progress: {
592+
commentary: true,
593+
},
594+
},
595+
},
533596
},
534597
},
535598
},
599+
},
600+
deliveryContext: {
601+
...progressCommentaryDeliveryContext,
602+
accountId: "work",
603+
},
604+
streamFlushMs: 10,
605+
noOutputNoticeMs: 120_000,
606+
});
607+
608+
emitAgentEvent({
609+
runId: "run-account-commentary-enabled",
610+
stream: "assistant",
611+
data: {
612+
delta: "checking account-scoped progress config.",
613+
phase: "commentary",
614+
},
615+
});
616+
vi.advanceTimersByTime(15);
617+
618+
expectTextWithFragment(collectedTexts(), "codex: checking account-scoped progress config.");
619+
relay.dispose();
620+
});
621+
622+
it("inherits legacy parent channel progress mode for account commentary overrides", () => {
623+
const relay = startAcpSpawnParentStreamRelay({
624+
runId: "run-account-legacy-commentary-enabled",
625+
parentSessionKey: "agent:main:main",
626+
childSessionKey: "agent:codex:acp:child-account-legacy-commentary-enabled",
627+
agentId: "codex",
628+
cfg: {
629+
channels: {
630+
forum: {
631+
streaming: "progress",
632+
accounts: {
633+
work: {
634+
streaming: {
635+
progress: {
636+
commentary: true,
637+
},
638+
},
639+
},
640+
},
641+
},
642+
},
643+
},
644+
deliveryContext: {
645+
...progressCommentaryDeliveryContext,
646+
accountId: "work",
647+
},
648+
streamFlushMs: 10,
649+
noOutputNoticeMs: 120_000,
650+
});
651+
652+
emitAgentEvent({
653+
runId: "run-account-legacy-commentary-enabled",
654+
stream: "assistant",
655+
data: {
656+
delta: "checking legacy progress config.",
657+
phase: "commentary",
658+
},
659+
});
660+
vi.advanceTimersByTime(15);
661+
662+
expectTextWithFragment(collectedTexts(), "codex: checking legacy progress config.");
663+
relay.dispose();
664+
});
665+
666+
it("relays ACP status progress when progress commentary and tag visibility are enabled", () => {
667+
const relay = startAcpSpawnParentStreamRelay({
668+
runId: "run-status-commentary-enabled",
669+
parentSessionKey: "agent:main:main",
670+
childSessionKey: "agent:codex:acp:child-status-commentary-enabled",
671+
agentId: "codex",
672+
cfg: progressModeConfig({
673+
stream: {
674+
tagVisibility: {
675+
plan: true,
676+
},
677+
},
536678
}),
679+
deliveryContext: progressCommentaryDeliveryContext,
680+
streamFlushMs: 10,
681+
noOutputNoticeMs: 120_000,
537682
});
538683

539684
emitAgentEvent({
@@ -552,15 +697,16 @@ describe("startAcpSpawnParentStreamRelay", () => {
552697
relay.dispose();
553698
});
554699

555-
it("does not relay hidden ACP status tags when assistant commentary is enabled", () => {
700+
it("does not relay hidden ACP status tags when progress commentary is enabled", () => {
556701
const relay = startAcpSpawnParentStreamRelay({
557702
runId: "run-status-commentary-hidden",
558703
parentSessionKey: "agent:main:main",
559704
childSessionKey: "agent:codex:acp:child-status-commentary-hidden",
560705
agentId: "codex",
706+
cfg: progressModeConfig(),
707+
deliveryContext: progressCommentaryDeliveryContext,
561708
streamFlushMs: 10,
562709
noOutputNoticeMs: 120_000,
563-
assistantCommentary: true,
564710
});
565711

566712
emitAgentEvent({
@@ -591,15 +737,16 @@ describe("startAcpSpawnParentStreamRelay", () => {
591737
relay.dispose();
592738
});
593739

594-
it("does not relay ACP status tags hidden by default when assistant commentary is enabled", () => {
740+
it("does not relay ACP status tags hidden by default when progress commentary is enabled", () => {
595741
const relay = startAcpSpawnParentStreamRelay({
596742
runId: "run-status-commentary-default-hidden",
597743
parentSessionKey: "agent:main:main",
598744
childSessionKey: "agent:codex:acp:child-status-commentary-default-hidden",
599745
agentId: "codex",
746+
cfg: progressModeConfig(),
747+
deliveryContext: progressCommentaryDeliveryContext,
600748
streamFlushMs: 10,
601749
noOutputNoticeMs: 120_000,
602-
assistantCommentary: true,
603750
});
604751

605752
emitAgentEvent({
@@ -624,10 +771,11 @@ describe("startAcpSpawnParentStreamRelay", () => {
624771
parentSessionKey: "agent:main:main",
625772
childSessionKey: "agent:codex:acp:child-commentary-visible-stall",
626773
agentId: "codex",
774+
cfg: progressModeConfig(),
775+
deliveryContext: progressCommentaryDeliveryContext,
627776
streamFlushMs: 1,
628777
noOutputNoticeMs: 1_000,
629778
noOutputPollMs: 250,
630-
assistantCommentary: true,
631779
});
632780

633781
emitAgentEvent({

0 commit comments

Comments
 (0)