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
9 changes: 5 additions & 4 deletions packages/agent/src/adapters/local-tools/tools/speak.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,11 @@ export const SPEAK_TOOL_DESCRIPTION =
* speakers, so it just acknowledges. The desktop renderer observes the
* surfaced `tool_call` (carrying `text`/`needsUser` in its rawInput) and routes
* it to the speech queue, exactly like completion/permission notifications are
* pure side effects off the event stream. Gated on `spokenNarration`: local
* sessions pass the user's setting at session start, while cloud sessions
* resolve it to true at the adapter (the sandbox can't know which clients are
* listening, so cloud emits always and consumers gate playback).
* pure side effects off the event stream. Gated on `spokenNarration`, which is
* strictly opt-in (see `resolveSpokenNarration`): the desktop passes it true
* only when the feature flag and the user's setting are both enabled. Headless
* cloud runs (Slack threads, Signals scouts) never enable it, so the tool and
* its instructions never load and never cost tokens there.
*/
export const speakTool = defineLocalTool({
name: SPEAK_TOOL_NAME,
Expand Down
37 changes: 15 additions & 22 deletions packages/agent/src/adapters/session-meta.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,18 @@ describe("resolveSpokenNarration", () => {
vi.unstubAllEnvs();
});

// Narration is strictly opt-in: only an explicit `spokenNarration: true`
// enables it. Cloud/sandbox runs no longer default on, so headless runs
// (Slack threads, Signals scouts) never load the tool or its instructions.
it.each([
{
name: "explicit true on local",
meta: { environment: "local" as const, spokenNarration: true },
name: "explicit true",
meta: { spokenNarration: true },
expected: true,
},
{
name: "explicit false on cloud",
meta: { environment: "cloud" as const, spokenNarration: false },
expected: false,
},
{
name: "cloud default",
meta: { environment: "cloud" as const },
expected: true,
},
{
name: "local default",
meta: { environment: "local" as const },
name: "explicit false",
meta: { spokenNarration: false },
expected: false,
},
{ name: "no meta", meta: undefined, expected: false },
Expand All @@ -35,20 +28,20 @@ describe("resolveSpokenNarration", () => {
});

it.each([
{ name: "no meta", meta: undefined, expected: true },
{ name: "empty meta", meta: {}, expected: true },
{ name: "no meta", meta: undefined, expected: false },
{ name: "empty meta", meta: {}, expected: false },
{
name: "explicit false",
meta: { spokenNarration: false },
expected: false,
name: "explicit true",
meta: { spokenNarration: true },
expected: true,
},
{
name: "explicit local environment",
meta: { environment: "local" as const },
name: "explicit false",
meta: { spokenNarration: false },
expected: false,
},
])(
"resolves $name to $expected in a sandbox without an environment tag",
"resolves $name to $expected in a sandbox (no default-on)",
({ meta, expected }) => {
vi.stubEnv("IS_SANDBOX", "1");
expect(resolveSpokenNarration(meta)).toBe(expected);
Expand Down
17 changes: 9 additions & 8 deletions packages/agent/src/adapters/session-meta.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import { isCloudRun } from "../utils/common";

/** Minimal shape needed to resolve the effective task id from session meta. */
interface TaskIdSource {
taskId?: string;
Expand All @@ -19,18 +17,21 @@ export function resolveTaskId(

/** Minimal shape needed to resolve spoken narration from session meta. */
interface SpokenNarrationSource {
environment?: "local" | "cloud";
spokenNarration?: boolean;
}

/**
* An explicit setting wins; otherwise cloud runs (including sandbox runs
* detected via `IS_SANDBOX`) default to on because the sandbox can't know
* which clients are listening, so consumers gate playback. Local runs stay
* silent. Shared by the Claude and Codex adapters.
* Spoken narration is strictly opt-in: it is on only when a caller explicitly
* sets `spokenNarration` true at session start. The desktop is the only client
* that can play audio and the only place that knows the feature flag and the
* user's setting, so it computes that boolean and passes it. Everything else
* (headless cloud runs like Slack threads and Signals scouts, sandboxes, local
* runs without the setting) stays silent, so the `speak` tool and its
* instructions never load and never cost tokens where nothing is listening.
* Shared by the Claude and Codex adapters.
*/
export function resolveSpokenNarration(
meta: SpokenNarrationSource | undefined,
): boolean {
return meta?.spokenNarration ?? isCloudRun(meta);
return meta?.spokenNarration === true;
Comment thread
pauldambra marked this conversation as resolved.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

🤖 Automated comment by QA Swarm — not written by a human

[convergent: paul-reviewer + xp-reviewer] 🟠 HIGH

Strict opt-in currently makes narration local-only because interactive cloud runs have no backend-supported way to carry the desktop flag + user opt-in decision. Supporting cloud narration requires a coordinated server contract or another explicit transport; otherwise this PR intentionally leaves cloud runs silent.

}
9 changes: 5 additions & 4 deletions packages/core/src/sessions/sessionService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,7 @@ export interface SessionServiceDeps {
rtkEnabledLocal?: boolean;
rtkEnabledCloud?: boolean;
spokenNotifications?: boolean;
spokenNarrationEnabled?: boolean;
};
usageLimit: { show: (...args: any[]) => any };
readonly addDirectoryDialog: { open: boolean };
Expand Down Expand Up @@ -1087,14 +1088,14 @@ export class SessionService {
this.d.log.warn("Failed to verify workspace", { taskId, err });
});

const { customInstructions, rtkEnabledLocal, spokenNotifications } =
const { customInstructions, rtkEnabledLocal, spokenNarrationEnabled } =
this.d.settings;
const result = await this.d.trpc.agent.reconnect.mutate({
taskId,
taskRunId,
repoPath,
rtkEnabled: rtkEnabledLocal,
spokenNarration: spokenNotifications === true,
spokenNarration: spokenNarrationEnabled === true,
apiHost: auth.apiHost,
projectId: auth.projectId,
logUrl,
Expand Down Expand Up @@ -1417,7 +1418,7 @@ export class SessionService {
const {
customInstructions: startCustomInstructions,
rtkEnabledLocal,
spokenNotifications,
spokenNarrationEnabled,
} = this.d.settings;
const preferredModel = model ?? this.d.DEFAULT_GATEWAY_MODEL;
const result = await this.d.trpc.agent.start.mutate({
Expand All @@ -1430,7 +1431,7 @@ export class SessionService {
adapter,
customInstructions: startCustomInstructions || undefined,
Comment thread
pauldambra marked this conversation as resolved.
rtkEnabled: rtkEnabledLocal,
spokenNarration: spokenNotifications === true,
spokenNarration: spokenNarrationEnabled === true,
effort: effortLevelSchema.safeParse(reasoningLevel).success
? (reasoningLevel as EffortLevel)
: undefined,
Expand Down
55 changes: 54 additions & 1 deletion packages/ui/src/features/sessions/sessionServiceHost.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -236,8 +236,14 @@ const mockSpeechNotifier = vi.hoisted(() => ({
speak: vi.fn(),
}));

const mockFeatureFlags = vi.hoisted(() => ({
isEnabled: vi.fn(() => false),
onFlagsLoaded: vi.fn(() => vi.fn()),
}));

const mockSettingsState = vi.hoisted(() => ({
customInstructions: "",
spokenNotifications: false,
syncCustomInstructionsFromFile: false,
syncedCustomInstructions: null as {
path: string;
Expand Down Expand Up @@ -314,6 +320,9 @@ vi.mock("@posthog/di/container", () => ({
if (typeof token === "function" && token.name === "SpeechNotifier") {
return mockSpeechNotifier;
}
if (token === Symbol.for("posthog.ui.featureFlags")) {
return mockFeatureFlags;
}
throw new Error(`resolveService: unmocked token ${String(token)}`);
},
}));
Expand Down Expand Up @@ -382,7 +391,11 @@ vi.mock("@posthog/core/sessions/sessionEvents", async () => {
});

import { toast } from "@posthog/ui/primitives/toast";
import { getSessionService, resetSessionService } from "./sessionServiceHost";
import {
getSessionService,
resetSessionService,
shouldEnableSpokenNarration,
} from "./sessionServiceHost";

// --- Test Fixtures ---

Expand Down Expand Up @@ -426,6 +439,8 @@ describe("SessionService", () => {
mockConvertStoredEntriesToEvents.mockImplementation(() => []);
resetSessionService();
mockSettingsState.customInstructions = "";
mockSettingsState.spokenNotifications = false;
mockFeatureFlags.isEnabled.mockReturnValue(false);
mockSettingsState.syncCustomInstructionsFromFile = false;
mockSettingsState.syncedCustomInstructions = null;
mockGetIsOnline.mockReturnValue(true);
Expand Down Expand Up @@ -514,6 +529,42 @@ describe("SessionService", () => {
});
});

describe("spoken narration availability", () => {
it.each([
{
userOptedIn: true,
flagEnabled: true,
isDevelopment: false,
expected: true,
},
{
userOptedIn: false,
flagEnabled: true,
isDevelopment: false,
expected: false,
},
{
userOptedIn: true,
flagEnabled: false,
isDevelopment: false,
expected: false,
},
{
userOptedIn: true,
flagEnabled: false,
isDevelopment: true,
expected: true,
},
])(
"returns $expected for opt-in=$userOptedIn flag=$flagEnabled dev=$isDevelopment",
({ userOptedIn, flagEnabled, isDevelopment, expected }) => {
expect(
shouldEnableSpokenNarration(userOptedIn, flagEnabled, isDevelopment),
).toBe(expected);
},
);
});

describe("connectToTask", () => {
it("skips local connection for cloud runs", async () => {
const service = getSessionService();
Expand Down Expand Up @@ -5036,6 +5087,8 @@ describe("SessionService", () => {

it("preserves codex runtime selection when resuming a terminal cloud run", async () => {
const service = getSessionService();
mockSettingsState.spokenNotifications = true;
mockFeatureFlags.isEnabled.mockReturnValue(true);
mockSessionStoreSetters.getSessionByTaskId.mockReturnValue(
createMockSession({
isCloud: true,
Expand Down
20 changes: 20 additions & 0 deletions packages/ui/src/features/sessions/sessionServiceHost.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,17 @@ import {
HOST_TRPC_CLIENT,
type HostTrpcClient,
} from "@posthog/host-router/client";
import { SPOKEN_NARRATION_FLAG } from "@posthog/shared";
import {
createAuthenticatedClient,
getAuthenticatedClient,
} from "@posthog/ui/features/auth/authClientImperative";
import { fetchAuthState } from "@posthog/ui/features/auth/authQueries";
import { useUsageLimitStore } from "@posthog/ui/features/billing/usageLimitStore";
import {
FEATURE_FLAGS,
type FeatureFlags,
} from "@posthog/ui/features/feature-flags/identifiers";
import { useAddDirectoryDialogStore } from "@posthog/ui/features/folder-picker/addDirectoryDialogStore";
import { NotificationBus } from "@posthog/ui/features/notifications/notifications";
import { SpeechNotifier } from "@posthog/ui/features/notifications/speechNotifier";
Expand Down Expand Up @@ -53,6 +58,14 @@ export { SessionService };

const log = logger.scope("session-service");

export function shouldEnableSpokenNarration(
userOptedIn: boolean,
flagEnabled: boolean,
isDevelopment: boolean,
): boolean {
return userOptedIn && (flagEnabled || isDevelopment);
}

function hostClient(): HostTrpcClient {
return resolveService<HostTrpcClient>(HOST_TRPC_CLIENT);
}
Expand Down Expand Up @@ -117,6 +130,13 @@ function buildSessionServiceDeps(): SessionServiceDeps {
return {
...state,
customInstructions: getEffectiveCustomInstructions(state),
spokenNarrationEnabled: shouldEnableSpokenNarration(
state.spokenNotifications,
resolveService<FeatureFlags>(FEATURE_FLAGS).isEnabled(
Comment thread
pauldambra marked this conversation as resolved.
SPOKEN_NARRATION_FLAG,
),
import.meta.env.DEV,
),
};
},
usageLimit: {
Expand Down
5 changes: 3 additions & 2 deletions packages/workspace-server/src/services/agent/schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,9 @@ export const startSessionInput = z.object({
rtkEnabled: z.boolean().optional(),
/**
* The user's spoken-narration setting at session start. Gates the agent's
* speak tool and its prompt instructions; when absent the adapter defaults
* by environment (cloud on, local off).
* speak tool and its prompt instructions. Strictly opt-in: only the desktop
* sets it true (feature flag + setting); when absent the adapter leaves
* narration off, so headless runs never load the tool.
*/
spokenNarration: z.boolean().optional(),
});
Expand Down
Loading