Skip to content

Commit 0b96300

Browse files
steipetesteipete
authored
refactor(shared): privatize internal helpers (#106868)
1 parent 4e689a2 commit 0b96300

13 files changed

Lines changed: 105 additions & 102 deletions

‎scripts/deadcode-exports.baseline.mjs‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1185,16 +1185,6 @@ export const KNIP_UNUSED_EXPORT_BASELINE = [
11851185
"src/sessions/session-state-events.ts: recordSessionStateEvent",
11861186
"src/sessions/session-state-events.ts: sessionStateEventStoreLimits",
11871187
"src/sessions/user-turn-transcript.ts: persistUserTurnTranscript",
1188-
"src/shared/config-eval.ts: evaluateRuntimeRequires",
1189-
"src/shared/config-eval.ts: isTruthy",
1190-
"src/shared/config-eval.ts: resolveConfigPath",
1191-
"src/shared/config-eval.ts: resolveRuntimePlatform",
1192-
"src/shared/device-auth-store.ts: DeviceAuthStoreAdapter",
1193-
"src/shared/node-match.ts: normalizeNodeKey",
1194-
"src/shared/runtime-import.ts: resolveRuntimeImportSpecifier",
1195-
"src/shared/silent-reply-policy.ts: DEFAULT_SILENT_REPLY_POLICY",
1196-
"src/shared/subagents-format.ts: formatTokenShort",
1197-
"src/shared/subagents-format.ts: resolveIoTokens",
11981188
"src/skills/discovery/chat-commands.ts: testing",
11991189
"src/skills/discovery/filter.ts: normalizeSkillFilterForComparison",
12001190
"src/skills/discovery/skill-index.ts: isSkillPromptVisible",

‎src/shared/config-eval.test.ts‎

Lines changed: 33 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,8 @@ import { afterEach, describe, expect, it, vi } from "vitest";
55
import { mockProcessPlatform } from "../test-utils/vitest-spies.js";
66
import {
77
evaluateRuntimeEligibility,
8-
evaluateRuntimeRequires,
98
hasBinary,
109
isConfigPathTruthyWithDefaults,
11-
isTruthy,
12-
resolveConfigPath,
13-
resolveRuntimePlatform,
1410
} from "./config-eval.js";
1511

1612
const originalPath = process.env.PATH;
@@ -32,15 +28,19 @@ afterEach(() => {
3228

3329
describe("config-eval helpers", () => {
3430
it("normalizes truthy values across primitive types", () => {
35-
expect(isTruthy(undefined)).toBe(false);
36-
expect(isTruthy(null)).toBe(false);
37-
expect(isTruthy(false)).toBe(false);
38-
expect(isTruthy(true)).toBe(true);
39-
expect(isTruthy(0)).toBe(false);
40-
expect(isTruthy(1)).toBe(true);
41-
expect(isTruthy(" ")).toBe(false);
42-
expect(isTruthy(" ok ")).toBe(true);
43-
expect(isTruthy({})).toBe(true);
31+
for (const [value, expected] of [
32+
[undefined, false],
33+
[null, false],
34+
[false, false],
35+
[true, true],
36+
[0, false],
37+
[1, true],
38+
[" ", false],
39+
[" ok ", true],
40+
[{}, true],
41+
] as const) {
42+
expect(isConfigPathTruthyWithDefaults({ value }, "value", {})).toBe(expected);
43+
}
4444
});
4545

4646
it("resolves nested config paths and missing branches safely", () => {
@@ -53,10 +53,10 @@ describe("config-eval helpers", () => {
5353
},
5454
};
5555

56-
expect(resolveConfigPath(config, "browser.enabled")).toBe(true);
57-
expect(resolveConfigPath(config, ".browser..nested.count.")).toBe(1);
58-
expect(resolveConfigPath(config, "browser.missing.value")).toBeUndefined();
59-
expect(resolveConfigPath("not-an-object", "browser.enabled")).toBeUndefined();
56+
expect(isConfigPathTruthyWithDefaults(config, "browser.enabled", {})).toBe(true);
57+
expect(isConfigPathTruthyWithDefaults(config, ".browser..nested.count.", {})).toBe(true);
58+
expect(isConfigPathTruthyWithDefaults(config, "browser.missing.value", {})).toBe(false);
59+
expect(isConfigPathTruthyWithDefaults("not-an-object", "browser.enabled", {})).toBe(false);
6060
});
6161

6262
it("blocks prototype keys while resolving config paths", () => {
@@ -66,10 +66,10 @@ describe("config-eval helpers", () => {
6666
},
6767
};
6868

69-
expect(resolveConfigPath(config, "safe.enabled")).toBe(true);
70-
expect(resolveConfigPath(config, "__proto__")).toBeUndefined();
71-
expect(resolveConfigPath(config, "constructor.name")).toBeUndefined();
72-
expect(resolveConfigPath(config, "prototype.polluted")).toBeUndefined();
69+
expect(isConfigPathTruthyWithDefaults(config, "safe.enabled", {})).toBe(true);
70+
expect(isConfigPathTruthyWithDefaults(config, "__proto__", {})).toBe(false);
71+
expect(isConfigPathTruthyWithDefaults(config, "constructor.name", {})).toBe(false);
72+
expect(isConfigPathTruthyWithDefaults(config, "prototype.polluted", {})).toBe(false);
7373
});
7474

7575
it("uses defaults only when config paths are unresolved", () => {
@@ -96,7 +96,14 @@ describe("config-eval helpers", () => {
9696

9797
it("returns the active runtime platform", () => {
9898
setPlatform("darwin");
99-
expect(resolveRuntimePlatform()).toBe("darwin");
99+
expect(
100+
evaluateRuntimeEligibility({
101+
os: ["darwin"],
102+
hasBin: () => true,
103+
hasEnv: () => true,
104+
isConfigPathTruthy: () => true,
105+
}),
106+
).toBe(true);
100107
});
101108

102109
it("caches binary lookups until PATH changes", () => {
@@ -147,9 +154,9 @@ describe("config-eval helpers", () => {
147154
});
148155
});
149156

150-
describe("evaluateRuntimeRequires", () => {
157+
describe("runtime requirements through eligibility", () => {
151158
it("accepts remote bins and remote any-bin matches", () => {
152-
const result = evaluateRuntimeRequires({
159+
const result = evaluateRuntimeEligibility({
153160
requires: {
154161
bins: ["node"],
155162
anyBins: ["bun", "deno"],
@@ -168,7 +175,7 @@ describe("evaluateRuntimeRequires", () => {
168175

169176
it("rejects when any required runtime check is still unsatisfied", () => {
170177
expect(
171-
evaluateRuntimeRequires({
178+
evaluateRuntimeEligibility({
172179
requires: { bins: ["node"] },
173180
hasBin: () => false,
174181
hasEnv: () => true,
@@ -177,7 +184,7 @@ describe("evaluateRuntimeRequires", () => {
177184
).toBe(false);
178185

179186
expect(
180-
evaluateRuntimeRequires({
187+
evaluateRuntimeEligibility({
181188
requires: { anyBins: ["bun", "node"] },
182189
hasBin: () => false,
183190
hasAnyRemoteBin: () => false,

‎src/shared/config-eval.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import path from "node:path";
44
import { isBlockedObjectKey } from "../infra/prototype-keys.js";
55

66
/** Normalizes primitive config values into the truthiness rules used by requirements checks. */
7-
export function isTruthy(value: unknown): boolean {
7+
function isTruthy(value: unknown): boolean {
88
if (value === undefined || value === null) {
99
return false;
1010
}
@@ -21,7 +21,7 @@ export function isTruthy(value: unknown): boolean {
2121
}
2222

2323
/** Resolves dotted config paths, tolerating extra dots and missing branches. */
24-
export function resolveConfigPath(config: unknown, pathStr: string): unknown {
24+
function resolveConfigPath(config: unknown, pathStr: string): unknown {
2525
const parts = pathStr.split(".").filter(Boolean);
2626
let current: unknown = config;
2727
for (const part of parts) {
@@ -77,7 +77,7 @@ type RuntimeRequirementEvalParams = {
7777
};
7878

7979
/** Evaluates binary/env/config requirements against local and optional remote capabilities. */
80-
export function evaluateRuntimeRequires(params: RuntimeRequirementEvalParams): boolean {
80+
function evaluateRuntimeRequires(params: RuntimeRequirementEvalParams): boolean {
8181
const requires = params.requires;
8282
if (!requires) {
8383
return true;
@@ -156,7 +156,7 @@ export function evaluateRuntimeEligibility(
156156
}
157157

158158
/** Returns the current Node runtime platform used by eligibility checks. */
159-
export function resolveRuntimePlatform(): string {
159+
function resolveRuntimePlatform(): string {
160160
return process.platform;
161161
}
162162

‎src/shared/device-auth-store.test.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,14 @@ import {
55
coerceDeviceAuthStore,
66
loadDeviceAuthTokenFromStore,
77
storeDeviceAuthTokenInStore,
8-
type DeviceAuthStoreAdapter,
98
} from "./device-auth-store.js";
109

11-
function createAdapter(initialStore: ReturnType<DeviceAuthStoreAdapter["readStore"]> = null) {
10+
type TestDeviceAuthStoreAdapter = Parameters<typeof loadDeviceAuthTokenFromStore>[0]["adapter"];
11+
12+
function createAdapter(initialStore: ReturnType<TestDeviceAuthStoreAdapter["readStore"]> = null) {
1213
let store = initialStore;
1314
const writes: unknown[] = [];
14-
const adapter: DeviceAuthStoreAdapter = {
15+
const adapter: TestDeviceAuthStoreAdapter = {
1516
readStore: () => store,
1617
writeStore: (next) => {
1718
store = next;

‎src/shared/device-auth-store.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import {
99
export type { DeviceAuthEntry, DeviceAuthStore } from "./device-auth.js";
1010

1111
/** Storage seam used by shared device-auth helpers and filesystem-backed infra wrappers. */
12-
export type DeviceAuthStoreAdapter = {
12+
type DeviceAuthStoreAdapter = {
1313
readStore: () => DeviceAuthStore | null;
1414
writeStore: (store: DeviceAuthStore) => void;
1515
};

‎src/shared/node-match.test.ts‎

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,27 @@
11
// Node match tests cover node selection from names, ids, and address hints.
22
import { describe, expect, it } from "vitest";
3-
import { normalizeNodeKey, resolveNodeIdFromCandidates } from "./node-match.js";
3+
import { resolveNodeIdFromCandidates } from "./node-match.js";
44

55
describe("shared/node-match", () => {
66
it("normalizes node keys by lowercasing and collapsing separators", () => {
7-
expect(normalizeNodeKey(" Mac Studio! ")).toBe("mac-studio");
8-
expect(normalizeNodeKey("---PI__Node---")).toBe("pi-node");
9-
expect(normalizeNodeKey("工作站 01")).toBe("工作站-01");
10-
expect(normalizeNodeKey("Cafe\u0301 01")).toBe("café-01");
11-
expect(normalizeNodeKey("किताब")).toBe("किताब");
12-
expect(normalizeNodeKey("Mac ❤️ Studio")).toBe("mac-studio");
13-
expect(normalizeNodeKey("Node 1️⃣")).toBe("node-1");
14-
expect(normalizeNodeKey("❤️")).toBe("");
15-
expect(normalizeNodeKey("###")).toBe("");
7+
for (const [displayName, query] of [
8+
[" Mac Studio! ", "mac-studio"],
9+
["---PI__Node---", "pi node"],
10+
["工作站 01", "工作站-01"],
11+
["Cafe\u0301 01", "café-01"],
12+
["किताब", "किताब"],
13+
["Mac ❤️ Studio", "mac studio"],
14+
["Node 1️⃣", "node 1"],
15+
] as const) {
16+
expect(resolveNodeIdFromCandidates([{ nodeId: "node-1", displayName }], query)).toBe(
17+
"node-1",
18+
);
19+
}
20+
for (const displayName of ["❤️", "###"]) {
21+
expect(() =>
22+
resolveNodeIdFromCandidates([{ nodeId: "node-1", displayName }], "named-node"),
23+
).toThrow(/unknown node/);
24+
}
1625
});
1726

1827
it("resolves unique matches and prefers a unique connected node", () => {

‎src/shared/node-match.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ type ScoredNodeMatch = {
3636
};
3737

3838
/** Normalizes human node names into stable lookup keys for fuzzy CLI/API matching. */
39-
export function normalizeNodeKey(value: string) {
39+
function normalizeNodeKey(value: string) {
4040
// Emoji components can also be marks (variation selectors and keycaps); drop
4141
// them so decorated and plain display-name selectors stay equivalent.
4242
// Retain script marks only when attached to a surviving letter/number; marks

‎src/shared/runtime-import.test.ts‎

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,13 @@
11
// Runtime import tests cover lazy runtime import caching and failure handling.
22
import { afterEach, describe, expect, it, vi } from "vitest";
33
import { toSafeImportPath } from "./import-specifier.js";
4-
import { importRuntimeModule, resolveRuntimeImportSpecifier } from "./runtime-import.js";
4+
import { importRuntimeModule } from "./runtime-import.js";
5+
6+
async function captureRuntimeImportSpecifier(baseUrl: string, parts: readonly string[]) {
7+
const importModule = vi.fn(async (specifier: string) => ({ specifier }));
8+
await importRuntimeModule(baseUrl, parts, importModule);
9+
return importModule.mock.calls[0]?.[0];
10+
}
511

612
describe("runtime-import", () => {
713
afterEach(() => {
@@ -22,33 +28,36 @@ describe("runtime-import", () => {
2228
);
2329
});
2430

25-
it("resolves runtime imports from Windows absolute base paths", () => {
31+
it("resolves runtime imports from Windows absolute base paths", async () => {
2632
vi.spyOn(process, "platform", "get").mockReturnValue("win32");
2733

2834
expect(
29-
resolveRuntimeImportSpecifier("C:\\Users\\alice\\openclaw\\dist\\subagent-registry.js", [
30-
"./subagent-registry.runtime.js",
31-
]),
35+
await captureRuntimeImportSpecifier(
36+
"C:\\Users\\alice\\openclaw\\dist\\subagent-registry.js",
37+
["./subagent-registry.runtime.js"],
38+
),
3239
).toBe("file:///C:/Users/alice/openclaw/dist/subagent-registry.runtime.js");
3340
});
3441

35-
it("resolves runtime imports from file URL base paths", () => {
42+
it("resolves runtime imports from file URL base paths", async () => {
3643
vi.spyOn(process, "platform", "get").mockReturnValue("win32");
3744

3845
expect(
39-
resolveRuntimeImportSpecifier("file:///C:/Users/alice/openclaw/dist/subagent-registry.js", [
40-
"./subagent-registry.runtime.js",
41-
]),
46+
await captureRuntimeImportSpecifier(
47+
"file:///C:/Users/alice/openclaw/dist/subagent-registry.js",
48+
["./subagent-registry.runtime.js"],
49+
),
4250
).toBe("file:///C:/Users/alice/openclaw/dist/subagent-registry.runtime.js");
4351
});
4452

45-
it("resolves absolute Windows runtime import parts directly", () => {
53+
it("resolves absolute Windows runtime import parts directly", async () => {
4654
vi.spyOn(process, "platform", "get").mockReturnValue("win32");
4755

4856
expect(
49-
resolveRuntimeImportSpecifier("file:///C:/Users/alice/openclaw/dist/subagent-registry.js", [
50-
"D:\\OpenClaw\\dist\\subagent-registry.runtime.js",
51-
]),
57+
await captureRuntimeImportSpecifier(
58+
"file:///C:/Users/alice/openclaw/dist/subagent-registry.js",
59+
["D:\\OpenClaw\\dist\\subagent-registry.runtime.js"],
60+
),
5261
).toBe("file:///D:/OpenClaw/dist/subagent-registry.runtime.js");
5362
});
5463

‎src/shared/runtime-import.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { toSafeImportPath } from "./import-specifier.js";
88
* Resolves lazy runtime import parts against the caller's module URL or path.
99
* Absolute normalized paths stay standalone; relative parts resolve against the normalized base.
1010
*/
11-
export function resolveRuntimeImportSpecifier(baseUrl: string, parts: readonly string[]): string {
11+
function resolveRuntimeImportSpecifier(baseUrl: string, parts: readonly string[]): string {
1212
const joined = parts.join("");
1313
const safeJoined = toSafeImportPath(joined);
1414
// Absolute Windows paths and UNC shares become standalone file URLs instead

‎src/shared/silent-reply-policy.test.ts‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Silent reply policy tests cover reply suppression decision rules.
22
import { describe, expect, it } from "vitest";
33
import {
4-
DEFAULT_SILENT_REPLY_POLICY,
54
classifySilentReplyConversationType,
65
resolveSilentReplyPolicyFromPolicies,
76
} from "./silent-reply-policy.js";
@@ -37,12 +36,8 @@ describe("classifySilentReplyConversationType", () => {
3736

3837
describe("silent reply default policy resolution", () => {
3938
it("uses defaults when no overrides exist", () => {
40-
expect(resolveSilentReplyPolicyFromPolicies({ conversationType: "direct" })).toBe(
41-
DEFAULT_SILENT_REPLY_POLICY.direct,
42-
);
43-
expect(resolveSilentReplyPolicyFromPolicies({ conversationType: "group" })).toBe(
44-
DEFAULT_SILENT_REPLY_POLICY.group,
45-
);
39+
expect(resolveSilentReplyPolicyFromPolicies({ conversationType: "direct" })).toBe("disallow");
40+
expect(resolveSilentReplyPolicyFromPolicies({ conversationType: "group" })).toBe("allow");
4641
});
4742
});
4843

0 commit comments

Comments
 (0)