Skip to content

Commit 4edd354

Browse files
authored
fix(notifications): fix CI and address speech narration review feedback
- Add enqueueSpeech to the cloud-task-update notification test harness deps so SessionService no longer throws "enqueueSpeech is not a function". - Allowlist the host-resident ElevenLabs speech service (the API key stays in the host process, matching secure-store/encryption), fixing the host boundary check. - composeUtterance: run the task-prefix guard before greeting injection so an already-prefixed agent line can't be double-prefixed when addressing by name; add a regression test. - Key speakCalls by taskRunId so a session teardown drops any speak calls still mid-stream instead of leaking them until reset(). Generated-By: PostHog Code Task-Id: 3553940c-2b6e-41dd-bc02-27f5df786a29
1 parent 07354dd commit 4edd354

5 files changed

Lines changed: 41 additions & 10 deletions

File tree

packages/core/src/sessions/cloudTaskUpdateNotifications.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ function createHarness() {
125125
let onUpdate: ((update: CloudTaskUpdatePayload) => void) | undefined;
126126
const notifyPromptComplete = vi.fn();
127127
const notifyPermissionRequest = vi.fn();
128+
const enqueueSpeech = vi.fn();
128129
const markActivity = vi.fn();
129130
const noopLog = {
130131
info: vi.fn(),
@@ -138,6 +139,7 @@ function createHarness() {
138139
log: noopLog,
139140
notifyPromptComplete,
140141
notifyPermissionRequest,
142+
enqueueSpeech,
141143
taskViewedApi: { markActivity },
142144
getPersistedConfigOptions: () => undefined,
143145
setPersistedConfigOptions: vi.fn(),

packages/core/src/sessions/sessionService.ts

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -557,9 +557,12 @@ export class SessionService {
557557
// accumulate their latest args; enqueue once the call reaches a terminal
558558
// status (full text), then delete the entry — which also dedupes re-fires.
559559
// A `null` value means "identified as speak, args not streamed in yet".
560+
// Keyed by taskRunId first so a session teardown can drop any of its
561+
// still-streaming speak calls (see unsubscribeFromChannel); the inner map is
562+
// keyed by toolCallId.
560563
private speakCalls = new Map<
561564
string,
562-
{ text: string; kind: SpeechKind } | null
565+
Map<string, { text: string; kind: SpeechKind } | null>
563566
>();
564567
// When the agent last narrated `done`/`needs_input` per task run (event ts).
565568
// The deterministic completion/needs-input backstops compare this against the
@@ -1680,6 +1683,9 @@ export class SessionService {
16801683
this.subscriptions.delete(taskRunId);
16811684
this.liveTurnContent.delete(taskRunId);
16821685
this.agentSpokeAt.delete(taskRunId);
1686+
// Drop any speak calls still mid-stream for this run (never reached a
1687+
// terminal status, so they were never enqueued or deleted above).
1688+
this.speakCalls.delete(taskRunId);
16831689
}
16841690

16851691
/**
@@ -2064,17 +2070,21 @@ export class SessionService {
20642070
};
20652071
const id = update.toolCallId;
20662072
if (id) {
2073+
const speakCalls = this.speakCalls.get(taskRunId);
20672074
// Only the top-level agent narrates. A `speak` from a sub-agent
20682075
// (spawned via the Task tool) carries a parentToolCallId; ignoring
20692076
// those prevents several sub-agents talking over each other.
20702077
if (
20712078
update._meta?.claudeCode?.toolName === SPEAK_TOOL_QUALIFIED_NAME &&
20722079
!update._meta.claudeCode.parentToolCallId &&
2073-
!this.speakCalls.has(id)
2080+
!speakCalls?.has(id)
20742081
) {
2075-
this.speakCalls.set(id, null);
2082+
const calls = speakCalls ?? new Map();
2083+
calls.set(id, null);
2084+
this.speakCalls.set(taskRunId, calls);
20762085
}
2077-
if (this.speakCalls.has(id)) {
2086+
const calls = this.speakCalls.get(taskRunId);
2087+
if (calls?.has(id)) {
20782088
// Accumulate the latest args — text grows across streamed updates.
20792089
const text = update.rawInput?.text;
20802090
if (typeof text === "string" && text.trim().length > 0) {
@@ -2085,13 +2095,14 @@ export class SessionService {
20852095
rawKind === "progress"
20862096
? rawKind
20872097
: "progress";
2088-
this.speakCalls.set(id, { text, kind });
2098+
calls.set(id, { text, kind });
20892099
}
20902100
// Speak only once the call is complete (full text). Deleting the
20912101
// entry both frees it and dedupes any later terminal event.
2092-
const pending = this.speakCalls.get(id);
2102+
const pending = calls.get(id);
20932103
if (isTerminalStatus(update.status) && pending) {
2094-
this.speakCalls.delete(id);
2104+
calls.delete(id);
2105+
if (calls.size === 0) this.speakCalls.delete(taskRunId);
20952106
if (pending.kind !== "progress") {
20962107
const spoke = this.agentSpokeAt.get(taskRunId) ?? {
20972108
needs_input: 0,

packages/core/src/speech/composeUtterance.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,18 @@ describe("composeUtterance", () => {
7676
).toBe("PostHog Code task 'x' — already prefixed");
7777
});
7878

79+
it("does not double-prefix even when addressing by name", () => {
80+
expect(
81+
composeUtterance({
82+
text: "PostHog Code task 'x' — already prefixed",
83+
taskTitle: "y",
84+
needsUser: true,
85+
addressByName: true,
86+
firstName: "Jon",
87+
}),
88+
).toBe("PostHog Code task 'x' — already prefixed");
89+
});
90+
7991
it("truncates a long task title", () => {
8092
const long = "a".repeat(60);
8193
const out = composeUtterance({ text: "hi", taskTitle: long });

packages/core/src/speech/composeUtterance.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,15 +39,18 @@ export function composeUtterance({
3939
}: ComposeInput): string {
4040
let body = text.trim();
4141

42+
// Agent already prefixed with a task reference — return it verbatim. Checked
43+
// before any greeting logic so the guard stays order-independent: injecting
44+
// "Hey <name>," first would push the prefix past the start of the string and
45+
// defeat this test, producing a double prefix.
46+
if (/^\s*posthog code task/i.test(body)) return body;
47+
4248
if (needsUser && addressByName && firstName) {
4349
// Normalize any leading greeting the agent added, then address by real name.
4450
body = body.replace(/^\s*hey\b[\s,]*/i, "").trimStart();
4551
body = `Hey ${firstName}, ${body}`;
4652
}
4753

48-
// Agent already prefixed with a task reference — don't double it.
49-
if (/^\s*posthog code task/i.test(body)) return body;
50-
5154
const title = taskTitle.trim();
5255
if (!title) return body;
5356
const shortTitle =

scripts/host-boundary-allowlist.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@
3434
"apps/code/src/main/services/secure-store/service.ts": [
3535
"injectable-outside-host"
3636
],
37+
"apps/code/src/main/services/speech/service.ts": [
38+
"injectable-outside-host"
39+
],
3740
"apps/code/src/main/services/workspace-server/service.ts": [
3841
"injectable-outside-host"
3942
],

0 commit comments

Comments
 (0)