Skip to content

Commit 05516bf

Browse files
miguel-heygenclaude
andcommitted
fix(media-use): address #2113 review — shared notice state, legacy id migration, stats robustness
- Notice-shown state now lives in the shared ~/.hyperframes/config.json (config.telemetryNoticeShown, the CLI's own field) instead of a media-use-only ~/.media marker — so shared-identity users see the first-run notice once per person, not once per tool. - Migrate a pre-existing ~/.media/anon-id into the shared config on upgrade, so media-use-only users keep their PostHog persona instead of resetting. - buildStats: --days only windows on a positive finite value (negative/NaN → all time, not an empty report); dropped the top-level catch that masked a real error as an all-zero "no usage" report (sub-reads are individually guarded). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H5k87mPZ4d6yiFwcWSb8Vv
1 parent 2cd6d34 commit 05516bf

4 files changed

Lines changed: 117 additions & 61 deletions

File tree

skills-manifest.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@
4646
"files": 10
4747
},
4848
"media-use": {
49-
"hash": "04b40c29ce570452",
49+
"hash": "b54a0c7135a3a88c",
5050
"files": 121
5151
},
5252
"motion-graphics": {

skills/media-use/scripts/lib/stats.mjs

Lines changed: 26 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -85,34 +85,33 @@ function diskBytes(records) {
8585
}
8686

8787
export function buildStats({ projectDir, days, now = Date.now() } = {}) {
88-
try {
89-
const cutoff =
90-
days == null || Number.isNaN(Number(days))
91-
? null
92-
: Number(now) - Number(days) * 24 * 60 * 60 * 1000;
93-
const records = (projectDir ? readManifest(projectDir) : []).filter((r) => inWindow(r, cutoff));
94-
const misses = readMisses().filter((miss) => inWindow(miss, cutoff));
95-
const globalRecords = readGlobalManifest();
96-
const report = emptyReport();
88+
// Only a positive finite --days windows the report; null / NaN / <= 0 mean
89+
// "all time" rather than silently excluding everything (a negative cutoff
90+
// would land in the future and drop every record). The reads below are each
91+
// best-effort (they return [] / skip on IO errors), so there is no top-level
92+
// catch masking a real logic bug as an all-zero "no usage" report.
93+
const n = Number(days);
94+
const cutoff = Number.isFinite(n) && n > 0 ? Number(now) - n * 24 * 60 * 60 * 1000 : null;
95+
const records = (projectDir ? readManifest(projectDir) : []).filter((r) => inWindow(r, cutoff));
96+
const misses = readMisses().filter((miss) => inWindow(miss, cutoff));
97+
const globalRecords = readGlobalManifest();
98+
const report = emptyReport();
9799

98-
report.total_resolves = records.length;
99-
report.misses = misses.length;
100-
for (const record of records) {
101-
increment(report.by_type, record?.type || "unknown");
102-
increment(report.by_source, sourceOf(record));
103-
increment(report.by_provider, record?.provenance?.provider);
104-
increment(report.by_via, record?.provenance?.via);
105-
}
100+
report.total_resolves = records.length;
101+
report.misses = misses.length;
102+
for (const record of records) {
103+
increment(report.by_type, record?.type || "unknown");
104+
increment(report.by_source, sourceOf(record));
105+
increment(report.by_provider, record?.provenance?.provider);
106+
increment(report.by_via, record?.provenance?.via);
107+
}
106108

107-
const attempts = report.total_resolves + report.misses;
108-
report.hit_rate = attempts === 0 ? null : report.total_resolves / attempts;
109-
report.top_missed_intents = topMissedIntents(misses);
110-
report.global_cache_assets = globalRecords.length;
111-
report.global_cache_disk_bytes = diskBytes(globalRecords);
112-
report.cross_project_reuse = globalRecords.filter((r) => r?.provenance?.reused_by).length;
109+
const attempts = report.total_resolves + report.misses;
110+
report.hit_rate = attempts === 0 ? null : report.total_resolves / attempts;
111+
report.top_missed_intents = topMissedIntents(misses);
112+
report.global_cache_assets = globalRecords.length;
113+
report.global_cache_disk_bytes = diskBytes(globalRecords);
114+
report.cross_project_reuse = globalRecords.filter((r) => r?.provenance?.reused_by).length;
113115

114-
return report;
115-
} catch {
116-
return emptyReport();
117-
}
116+
return report;
118117
}

skills/media-use/scripts/lib/telemetry.mjs

Lines changed: 63 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -32,26 +32,61 @@ export function optedOut() {
3232
);
3333
}
3434

35-
// Stable per-machine anonymous id, read from the shared CLI/studio contract.
36-
function anonymousId() {
35+
// CLI + studio share one install identity in ~/.hyperframes/config.json
36+
// (packages/cli/src/telemetry/config.ts — same path, same `anonymousId` /
37+
// `telemetryNoticeShown` fields). Read and write that same file so media-use is
38+
// the same PostHog person and shows the notice once per person, not per tool.
39+
// Computed per call (not a module const) so it honors HOME at runtime — tests
40+
// sandbox HOME, and os.homedir() re-reads it each call.
41+
function sharedConfigPath() {
42+
return join(homedir(), ".hyperframes", "config.json");
43+
}
44+
45+
function readSharedConfig() {
46+
try {
47+
const file = sharedConfigPath();
48+
if (existsSync(file)) {
49+
const parsed = JSON.parse(readFileSync(file, "utf8"));
50+
if (parsed && typeof parsed === "object" && !Array.isArray(parsed)) return parsed;
51+
}
52+
} catch {
53+
// unreadable config → treat as empty; never throw
54+
}
55+
return {};
56+
}
57+
58+
function writeSharedConfig(config) {
3759
const dir = join(homedir(), ".hyperframes");
38-
const file = join(dir, "config.json");
60+
if (!existsSync(dir)) mkdirSync(dir, { recursive: true });
61+
writeFileSync(join(dir, "config.json"), JSON.stringify(config, null, 2) + "\n");
62+
}
63+
64+
// Adopt a pre-existing media-use-only id (~/.media/anon-id from before this
65+
// change) so upgraders keep their PostHog persona instead of resetting to a new
66+
// one — otherwise cross-surface continuity would start over on upgrade.
67+
function legacyMediaAnonId() {
3968
try {
40-
let config = {};
69+
const file = join(homedir(), ".media", "anon-id");
4170
if (existsSync(file)) {
42-
try {
43-
const parsed = JSON.parse(readFileSync(file, "utf8"));
44-
if (parsed && typeof parsed === "object" && !Array.isArray(parsed)) config = parsed;
45-
} catch {
46-
config = {};
47-
}
48-
if (typeof config.anonymousId === "string" && config.anonymousId.trim()) {
49-
return config.anonymousId.trim();
50-
}
71+
const id = readFileSync(file, "utf8").trim();
72+
if (id) return id;
5173
}
52-
if (!existsSync(dir)) mkdirSync(dir, { recursive: true });
53-
const id = randomUUID();
54-
writeFileSync(file, JSON.stringify({ ...config, anonymousId: id }, null, 2) + "\n");
74+
} catch {
75+
// ignore
76+
}
77+
return null;
78+
}
79+
80+
// Stable per-machine id from the shared config; seeds it (adopting a legacy
81+
// media-use id when present) if absent.
82+
function anonymousId() {
83+
try {
84+
const config = readSharedConfig();
85+
if (typeof config.anonymousId === "string" && config.anonymousId.trim()) {
86+
return config.anonymousId.trim();
87+
}
88+
const id = legacyMediaAnonId() || randomUUID();
89+
writeSharedConfig({ ...config, anonymousId: id });
5590
return id;
5691
} catch {
5792
return "anon"; // best-effort; a shared bucket is fine if the fs is read-only
@@ -77,24 +112,20 @@ function heygenAccountDistinctId() {
77112

78113
function showTelemetryNotice() {
79114
if (optedOut()) return;
80-
const dir = join(homedir(), ".media");
81-
const file = join(dir, "telemetry-notice-shown");
82-
try {
83-
if (existsSync(file)) return;
84-
} catch {
85-
return;
86-
}
87-
console.error(
88-
[
89-
"media-use sends usage telemetry: media type, resolution source, and provider; never intent text, file names, or paths.",
90-
"If you sign in to HeyGen, usage links to your account email or username. Opt out with HYPERFRAMES_NO_TELEMETRY=1 or DO_NOT_TRACK=1.",
91-
].join("\n"),
92-
);
93115
try {
94-
if (!existsSync(dir)) mkdirSync(dir, { recursive: true });
95-
writeFileSync(file, new Date().toISOString() + "\n");
116+
const config = readSharedConfig();
117+
// Shared with the CLI (config.telemetryNoticeShown): shown once per person
118+
// across surfaces, not once per tool.
119+
if (config.telemetryNoticeShown === true) return;
120+
console.error(
121+
[
122+
"media-use sends usage telemetry: media type, resolution source, and provider; never intent text, file names, or paths.",
123+
"If you sign in to HeyGen, usage links to your account email or username. Opt out with HYPERFRAMES_NO_TELEMETRY=1 or DO_NOT_TRACK=1.",
124+
].join("\n"),
125+
);
126+
writeSharedConfig({ ...config, telemetryNoticeShown: true });
96127
} catch {
97-
// notice marker is best-effort; never surface into the command
128+
// notice is best-effort; never surface into the command
98129
}
99130
}
100131

skills/media-use/scripts/lib/telemetry.test.mjs

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,27 @@ test("anonymous id seeds missing config once and reuses it", () => {
122122
}
123123
});
124124

125+
test("anonymous id adopts a legacy ~/.media/anon-id on upgrade (persona continuity)", () => {
126+
const savedEnv = { ...process.env };
127+
const { root, home } = sandbox();
128+
try {
129+
withoutTelemetryOptOut();
130+
mkdirSync(join(home, ".media"), { recursive: true });
131+
writeFileSync(join(home, ".media/anon-id"), "legacy-media-id");
132+
// no ~/.hyperframes/config.json yet — the old media-use-only id must carry over
133+
assert.equal(__anonymousIdForTest(), "legacy-media-id");
134+
// and it is persisted into the shared config so CLI/studio see the same id
135+
assert.equal(
136+
JSON.parse(readFileSync(join(home, ".hyperframes/config.json"), "utf8")).anonymousId,
137+
"legacy-media-id",
138+
);
139+
} finally {
140+
restoreEnv(savedEnv);
141+
rmSync(root, { recursive: true, force: true });
142+
__resetTelemetryForTest();
143+
}
144+
});
145+
125146
test("track identifies a signed-in HeyGen account once and still sends events", async () => {
126147
const savedEnv = { ...process.env };
127148
const originalFetch = globalThis.fetch;
@@ -214,7 +235,12 @@ test("first run notice prints to stderr once and never stdout", async () => {
214235
assert.equal(stderr.length, 1);
215236
assert.match(stderr[0], /media-use sends usage telemetry/);
216237
assert.equal(stdout.length, 0);
217-
assert.ok(existsSync(join(home, ".media/telemetry-notice-shown")));
238+
// notice-shown lives in the shared config (config.telemetryNoticeShown), so
239+
// the CLI and media-use show it once per person — not a media-use-only marker.
240+
assert.equal(
241+
JSON.parse(readFileSync(join(home, ".hyperframes/config.json"), "utf8")).telemetryNoticeShown,
242+
true,
243+
);
218244
} finally {
219245
globalThis.fetch = originalFetch;
220246
console.error = originalError;

0 commit comments

Comments
 (0)