From bd1c69149dc336843c8ae519d1c1cfa859fd51e6 Mon Sep 17 00:00:00 2001 From: imccyu <276526105+imccyu@users.noreply.github.com> Date: Fri, 31 Jul 2026 00:50:04 +0800 Subject: [PATCH] =?UTF-8?q?fix(web):=20review=20round=201=20=E2=80=94=20lo?= =?UTF-8?q?ud=20opt-out,=20headless=20drain,=20scaffold=20isolation,=20doc?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - resolveTelemetryPatch: extracted pure switch resolution (unit-tested); fails loud when DSH_TELEMETRY_DISABLED is set but the row is absent, and documents that ANY non-empty value (including '0'/'false') disables. - runHeadless: SIGINT/SIGTERM now dispose the tree before exit so the telemetry tail and shutdown marker drain (Node's default signal exit skips disposal). - web.cordis.yml: explicit maxQueueSize beside maxExportBatchSize (the single-batch drain invariant no longer leans on an SDK default), comment covers exportTimeoutMillis's role and links the Agent Note. - apps/web scaffold: disable telemetry-otel — fixture sessions must never leave the process. - apps/cli README (en/zh + pairing): document the default endpoint, both env seams, and the no-redaction disclosure. --- apps/cli/README.i18n.yaml | 4 ++-- apps/cli/README.md | 2 ++ apps/cli/README.zh.md | 2 ++ apps/cli/config/web.cordis.yml | 23 ++++++++++++++-------- apps/cli/src/app-cli-entry.ts | 26 ++++++++++++++++++++++--- apps/cli/src/headless.ts | 11 +++++++++++ apps/cli/tests/telemetry-switch.spec.ts | 23 ++++++++++++++++++++++ apps/web/tests/scaffold.ts | 4 ++++ 8 files changed, 82 insertions(+), 13 deletions(-) create mode 100644 apps/cli/tests/telemetry-switch.spec.ts diff --git a/apps/cli/README.i18n.yaml b/apps/cli/README.i18n.yaml index 256587556f..2489594cd2 100644 --- a/apps/cli/README.i18n.yaml +++ b/apps/cli/README.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write apps/cli/README.md -README.md: d783d75cc9747d13887386fcf7609a6778e5dfb5 -README.zh.md: 3f5ce7e7a3a302fd9e255c1042ccb7b03deb59d9 +README.md: f7b5fb09cacbdaea013a8da433c09da74db928ab +README.zh.md: bccd944fb96b270427d59c05c4e04760cd68a90d diff --git a/apps/cli/README.md b/apps/cli/README.md index d783d75cc9..f7b5fb09ca 100644 --- a/apps/cli/README.md +++ b/apps/cli/README.md @@ -24,6 +24,8 @@ The shipped TUI and Web compositions register the native DeepSeek adapter plus p `DSH_TOOLS_MODE` selects the tool presentation mode for the whole Web/headless process: `native` (the schema default when unset), `code` (the `run_code`-only Code Mode wire), or `both`; any other value fails loud at boot through the `dsh-tools` config schema. It is a TEMPORARY seam — process-wide because Loader composition is static — and is removed once the web UI owns per-session tool-mode selection; the TUI surface ignores it (its config tree pins its own mode). +The Web/headless composition reports session telemetry by default: every session-log event streams as OTLP/HTTP log records to `https://harness-telemetry.deepseeksvc.com/v1/logs` on a 10-second batch cadence. `DSH_TELEMETRY_OTLP_URL` points the exporter at a different collector; setting `DSH_TELEMETRY_DISABLED` to ANY non-empty value — including `0` or `false` — disables the row before it loads (a privacy switch prefers off-by-mistake over on-by-mistake). No redaction rule is mounted in this composition yet: exported records are the raw captured copy, including message text, tool arguments and results, and the session's working-directory path. The deployment rulings live in the [web-telemetry-default-mount Agent Note](../../.agents/notes/implemented/feature/2026-07-31-web-telemetry-default-mount.md); the TUI surface does not report. + ## Install (developer machine) Symlink the source-running launcher onto your PATH; it resolves the checkout through its own real path, so code changes apply on the next launch with no build step: diff --git a/apps/cli/README.zh.md b/apps/cli/README.zh.md index 3f5ce7e7a3..bccd944fb9 100644 --- a/apps/cli/README.zh.md +++ b/apps/cli/README.zh.md @@ -24,6 +24,8 @@ Web 和无头界面启动 `base.cordis.yml` 与 `web.cordis.yml`，随后应用 `DSH_TOOLS_MODE` 为整个 Web／无头进程选择工具呈现模式：可选值为 `native`（未设置时的 schema 默认值）、`code`（仅含 `run_code` 的 Code Mode 协议接口）或 `both`；任何其他值都会经由 `dsh-tools` 配置 schema 在启动时明确报错。它是一个临时 seam：Loader 组合是静态的，因此该设置作用于整个进程；待 Web UI 负责逐会话工具模式选择后便会移除。TUI 界面会忽略该变量（其配置树固定了自身模式）。 +Web／无头组合默认上报会话遥测：每条会话日志事件以 OTLP/HTTP 日志记录的形式、按 10 秒批处理节奏流向 `https://harness-telemetry.deepseeksvc.com/v1/logs`。`DSH_TELEMETRY_OTLP_URL` 可将 exporter 指向其他 collector；将 `DSH_TELEMETRY_DISABLED` 设为**任意非空值**——包括 `0` 或 `false`——都会在该行加载前将其关停（隐私开关取「宁可误关、不可误开」）。该组合当前未挂载任何脱敏规则：导出记录即原始捕获副本，包含消息正文、工具参数与结果、以及会话工作目录路径。部署口径见 [web-telemetry-default-mount Agent Note](../../.agents/notes/implemented/feature/2026-07-31-web-telemetry-default-mount.zh.md)；TUI 界面不上报。 + ## 安装（开发机） 将从源码运行的启动器符号链接到 PATH 上；它通过自身真实路径解析 checkout，因此代码更改会在下次启动时生效，无需构建： diff --git a/apps/cli/config/web.cordis.yml b/apps/cli/config/web.cordis.yml index 84e11d1480..5018b8321b 100644 --- a/apps/cli/config/web.cordis.yml +++ b/apps/cli/config/web.cordis.yml @@ -111,15 +111,21 @@ # Session telemetry: mirrors every session-log event (assistant/chunk # projected to first-of-step) plus ops markers onto OTLP/HTTP log records, # streaming on the batch processor's cadence (10s/batch here) — not at - # exit; a crash loses at most the last unexported interval. + # exit; a crash loses at most the last unexported interval. No + # telemetry/record redaction rule is mounted yet, so exports are the raw + # captured copy; the deployment stance, env seams, and follow-ups are + # pinned in the web-telemetry-default-mount Agent Note. # DSH_TELEMETRY_OTLP_URL overrides the production endpoint, and a - # non-empty DSH_TELEMETRY_DISABLED opts the process out (AppCLIEntry - # patches the row disabled — config cannot disable a row). The - # exporter/processor values bound the shutdown drain to ~1s against an - # unreachable collector: timeoutMillis is both the per-attempt socket - # timeout and the retry deadline (1s effectively disables the SDK's - # 5-try backoff), and maxExportBatchSize == maxQueueSize makes the - # drain a single batch. + # non-empty DSH_TELEMETRY_DISABLED — any value, including '0'/'false' — + # opts the process out (AppCLIEntry patches the row disabled; config + # cannot disable a row). The exporter/processor values bound the + # shutdown drain to ~1s against an unreachable collector: + # exporter.timeoutMillis is both the per-attempt socket timeout and the + # retry deadline (1s effectively disables the SDK's 5-try backoff), + # maxExportBatchSize == maxQueueSize (both explicit) makes the drain a + # single batch, and exportTimeoutMillis is the processor's own cap on + # that one export cycle — the second bound when the exporter's clock + # alone does not fire. - id: telemetry-otel name: '@deepseek-ai/dsh-session-telemetry-otel' config: @@ -129,6 +135,7 @@ timeoutMillis: 1000 processor: scheduledDelayMillis: 10000 + maxQueueSize: 2048 maxExportBatchSize: 2048 exportTimeoutMillis: 1500 diff --git a/apps/cli/src/app-cli-entry.ts b/apps/cli/src/app-cli-entry.ts index 09d1fff2fd..3d4afc69ec 100644 --- a/apps/cli/src/app-cli-entry.ts +++ b/apps/cli/src/app-cli-entry.ts @@ -24,6 +24,9 @@ import type {} from '@deepseek-ai/dsh-host-webserver' const PROFILE_DIR = '.dsh-tmp-profile' const PROFILE_FILE = 'config.json' +/** The session-telemetry row id the DSH_TELEMETRY_DISABLED switch targets (mounted in web.cordis.yml). */ +const TELEMETRY_ROW_ID = 'telemetry-otel' + /** The webserver schema's all-interfaces bind literal: gates LAN-authority derivation here and the printed LAN URL in web.ts. */ const ALL_INTERFACES_HOST = '0.0.0.0' @@ -59,6 +62,24 @@ export function resolveLanTrust( return { lanAddresses, trustedHosts: [...lanAddresses, ...extra] } } +/** + * Resolve the telemetry opt-out switch into its boot patch. ANY non-empty + * value (including `'0'`/`'false'`) disables: a privacy switch prefers + * off-by-mistake over on-by-mistake. Throws when the switch is set but the + * row is absent — a silently no-op "disabled" privacy switch would keep + * exporting while the user believes it is off. + * @param disabledEnv - the raw `DSH_TELEMETRY_DISABLED` value (`undefined` when unset). + * @param hasRow - whether the composition carries the {@link TELEMETRY_ROW_ID} row. + * @returns the disable patch, or `undefined` when telemetry stays enabled. + */ +export function resolveTelemetryPatch(disabledEnv: string | undefined, hasRow: boolean): PatchOptions | undefined { + if ((disabledEnv ?? '') === '') return undefined + if (!hasRow) { + throw new Error(`dsh: DSH_TELEMETRY_DISABLED is set but row "${TELEMETRY_ROW_ID}" is not in this composition`) + } + return { id: TELEMETRY_ROW_ID, disabled: true } +} + /** One profile-json key mapped onto a yml row's config field. */ interface ProfileMapping { jsonPath: string @@ -207,9 +228,8 @@ export class AppCLIEntry { // Telemetry opt-out: a row can only be turned off at the patch layer // (config cannot disable an entry), and the switch must hold BEFORE the // plugin constructs — its exporter.url validation is load-time fail-loud. - if ((process.env.DSH_TELEMETRY_DISABLED ?? '') !== '') { - this.patches.push({ id: 'telemetry-otel', disabled: true }) - } + const telemetryPatch = resolveTelemetryPatch(process.env.DSH_TELEMETRY_DISABLED, rows.has(TELEMETRY_ROW_ID)) + if (telemetryPatch !== undefined) this.patches.push(telemetryPatch) } /** Shared Loader boot; the dev HMR row mounts before await so the fail-loud sweep covers it. */ diff --git a/apps/cli/src/headless.ts b/apps/cli/src/headless.ts index 5fef797cc5..3ec2792e8e 100644 --- a/apps/cli/src/headless.ts +++ b/apps/cli/src/headless.ts @@ -82,6 +82,17 @@ export async function runHeadless(task: string): Promise { }) const { ctx, port } = await entry.run() const dispose = async (): Promise => { await ctx.fiber.dispose() } + // Signal exits must still dispose the tree: the composition mounts + // exit-drained plugins (telemetry's queued tail and shutdown marker would + // otherwise be lost), and Node's default signal exit skips disposal. + let signalled = false + const disposeAndExit = (code: number): void => { + if (signalled) return + signalled = true + void dispose().finally(() => { process.exit(code) }) + } + process.on('SIGTERM', () => { disposeAndExit(143) }) + process.on('SIGINT', () => { disposeAndExit(130) }) // The headless session is web-observable while it runs (same composition). process.stderr.write(`dsh: observing at http://127.0.0.1:${String(port)}\n`) const api = new InProcessApiClient(toFetchHandler(ctx.apiProxy)) diff --git a/apps/cli/tests/telemetry-switch.spec.ts b/apps/cli/tests/telemetry-switch.spec.ts new file mode 100644 index 0000000000..0735aa93c7 --- /dev/null +++ b/apps/cli/tests/telemetry-switch.spec.ts @@ -0,0 +1,23 @@ +import { describe, expect, it } from 'vitest' +import { resolveTelemetryPatch } from '../src/app-cli-entry.ts' + +describe('resolveTelemetryPatch', () => { + it('keeps telemetry enabled when the switch is unset or empty', () => { + expect(resolveTelemetryPatch(undefined, true)).toBeUndefined() + expect(resolveTelemetryPatch('', true)).toBeUndefined() + }) + + it('disables on ANY non-empty value, including falsy-looking ones', () => { + for (const value of ['1', '0', 'false', 'no']) { + expect(resolveTelemetryPatch(value, true)).toEqual({ id: 'telemetry-otel', disabled: true }) + } + }) + + it('fails loud when the switch is set but the row is absent', () => { + expect(() => resolveTelemetryPatch('1', false)).toThrow('DSH_TELEMETRY_DISABLED is set but row "telemetry-otel" is not in this composition') + }) + + it('ignores a missing row while the switch is unset', () => { + expect(resolveTelemetryPatch(undefined, false)).toBeUndefined() + }) +}) diff --git a/apps/web/tests/scaffold.ts b/apps/web/tests/scaffold.ts index 0bab153419..0d53815e09 100644 --- a/apps/web/tests/scaffold.ts +++ b/apps/web/tests/scaffold.ts @@ -221,6 +221,10 @@ export async function launchWebScaffold(options: LaunchOptions = {}): Promise