Skip to content

Commit d7ff692

Browse files
jrusso1020claude
andcommitted
refactor(telemetry): address remaining PR #1035 review feedback
Three follow-ups from @miguel-heygen's review: 1. HERMES_QUIET — switch to existence check. `env["HERMES_QUIET"] === "1"` was brittle vs. future Hermes changes (e.g. if cli.py ever sets it to "true"). The var name itself is specific enough that existence is the right signal. 2. CI_PROVIDERS — convert to a discriminated union. `mode: "truthy" | "presence"` is stricter than the previous pair of optional boolean flags (which allowed entries with neither set). 3. Sandbox detection tests — add coverage. - Docker positive: /.dockerenv present → docker. - Negative case: plain Linux laptop with no markers → null. Together with the gVisor 4.4.0 fix in the previous commit, that addresses all three actionable callouts (the discriminated-union nit was non-blocking but worth doing while in the file). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 1188814 commit d7ff692

3 files changed

Lines changed: 72 additions & 18 deletions

File tree

packages/cli/src/telemetry/agent_runtime.test.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,57 @@ describe("detectAgentRuntime — Jules / Replit / Devin / Hermes / openclaw", ()
172172
});
173173
});
174174

175+
describe("detectSandboxRuntime — file-system path", () => {
176+
beforeEach(() => {
177+
vi.resetModules();
178+
});
179+
180+
afterEach(() => {
181+
vi.resetModules();
182+
vi.restoreAllMocks();
183+
});
184+
185+
it("reports docker when /.dockerenv exists", async () => {
186+
vi.doMock("node:os", async () => {
187+
const actual = await vi.importActual<typeof import("node:os")>("node:os");
188+
return { ...actual, release: () => "6.8.0-100-generic", platform: () => "linux" };
189+
});
190+
vi.doMock("node:fs", async () => {
191+
const actual = await vi.importActual<typeof import("node:fs")>("node:fs");
192+
return {
193+
...actual,
194+
existsSync: (path: string) => path === "/.dockerenv" || actual.existsSync(path),
195+
readFileSync: (path: string) =>
196+
path === "/proc/version" ? "Linux version 6.8.0-100-generic" : actual.readFileSync(path),
197+
};
198+
});
199+
const { detectSandboxRuntime } = await import("./agent_runtime.js");
200+
expect(detectSandboxRuntime()).toBe("docker");
201+
});
202+
203+
it("returns null on a plain non-sandboxed Linux laptop", async () => {
204+
vi.doMock("node:os", async () => {
205+
const actual = await vi.importActual<typeof import("node:os")>("node:os");
206+
return { ...actual, release: () => "6.8.0-100-generic", platform: () => "linux" };
207+
});
208+
vi.doMock("node:fs", async () => {
209+
const actual = await vi.importActual<typeof import("node:fs")>("node:fs");
210+
return {
211+
...actual,
212+
existsSync: () => false,
213+
readFileSync: (path: string) =>
214+
path === "/proc/version"
215+
? "Linux version 6.8.0-100-generic (buildd@lcy01)"
216+
: path === "/proc/1/cgroup"
217+
? "0::/user.slice/user-1000.slice"
218+
: actual.readFileSync(path),
219+
};
220+
});
221+
const { detectSandboxRuntime } = await import("./agent_runtime.js");
222+
expect(detectSandboxRuntime()).toBeNull();
223+
});
224+
});
225+
175226
describe("detectSandboxRuntime — kernel-string path", () => {
176227
beforeEach(() => {
177228
vi.resetModules();

packages/cli/src/telemetry/agent_runtime.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,11 +106,12 @@ const VENDOR_RULES: VendorRule[] = [
106106
// Nous Research Hermes Agent — cli.py:50 unconditionally executes
107107
// os.environ["HERMES_QUIET"] = "1"
108108
// at module load, so the marker propagates via os.environ to every
109-
// subprocess spawned by Hermes.
109+
// subprocess spawned by Hermes. Keying on existence (not the literal
110+
// "1") so we still match if Hermes ever changes the value.
110111
// Source: https://github.com/NousResearch/hermes-agent (cli.py:50)
111112
{
112113
name: "hermes",
113-
check: (env) => env["HERMES_QUIET"] === "1",
114+
check: (env) => typeof env["HERMES_QUIET"] === "string",
114115
},
115116
// openclaw — multi-channel AI gateway. When openclaw spawns a CLI
116117
// subprocess it builds the child env with OPENCLAW_STATE_DIR /

packages/cli/src/telemetry/system.ts

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -93,24 +93,26 @@ function detectDocker(): boolean {
9393
return false;
9494
}
9595

96-
// Each entry: env var name, optional named CI provider, predicate.
9796
// Named providers come first so getCIName() picks the most specific match.
98-
// `truthy` accepts 'true' or '1' to cover both common conventions.
99-
const CI_PROVIDERS: Array<{ name: string | null; envVar: string; truthy?: true; presence?: true }> =
100-
[
101-
{ name: "github_actions", envVar: "GITHUB_ACTIONS", truthy: true },
102-
{ name: "gitlab_ci", envVar: "GITLAB_CI", truthy: true },
103-
{ name: "circleci", envVar: "CIRCLECI", truthy: true },
104-
{ name: "jenkins", envVar: "JENKINS_URL", presence: true },
105-
{ name: "buildkite", envVar: "BUILDKITE", truthy: true },
106-
{ name: "travis", envVar: "TRAVIS", truthy: true },
107-
{ name: null, envVar: "CONTINUOUS_INTEGRATION", truthy: true },
108-
{ name: null, envVar: "CI", truthy: true },
109-
];
110-
111-
function matchesProvider(p: (typeof CI_PROVIDERS)[number]): boolean {
97+
// `truthy` accepts 'true' or '1'; `presence` matches any non-null value.
98+
type CIProvider =
99+
| { name: string | null; envVar: string; mode: "truthy" }
100+
| { name: string | null; envVar: string; mode: "presence" };
101+
102+
const CI_PROVIDERS: CIProvider[] = [
103+
{ name: "github_actions", envVar: "GITHUB_ACTIONS", mode: "truthy" },
104+
{ name: "gitlab_ci", envVar: "GITLAB_CI", mode: "truthy" },
105+
{ name: "circleci", envVar: "CIRCLECI", mode: "truthy" },
106+
{ name: "jenkins", envVar: "JENKINS_URL", mode: "presence" },
107+
{ name: "buildkite", envVar: "BUILDKITE", mode: "truthy" },
108+
{ name: "travis", envVar: "TRAVIS", mode: "truthy" },
109+
{ name: null, envVar: "CONTINUOUS_INTEGRATION", mode: "truthy" },
110+
{ name: null, envVar: "CI", mode: "truthy" },
111+
];
112+
113+
function matchesProvider(p: CIProvider): boolean {
112114
const v = process.env[p.envVar];
113-
if (p.presence) return v != null;
115+
if (p.mode === "presence") return v != null;
114116
return v === "true" || v === "1";
115117
}
116118

0 commit comments

Comments
 (0)