mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-01 19:42:03 +00:00
fix(cli): address PR review feedback from miguel-heygen
- flushSync: use detached spawn + unref() instead of execFileSync, so process.exit() paths don't block up to 5s on slow networks - showTelemetryNotice: persist notice flag BEFORE printing/tracking, so users are never tracked without having seen the disclosure - Config dir: set mode 0o700 on ~/.hyperframes/ directory (was umask default) - $ip: null comment: clarify this is belt-and-suspenders with server-side discard - shouldTrack: update comment — phc_ prefix check is a safety net, not dead code - env.ts: add comment explaining try/catch fail-safe defaults to production - init.ts: consistently call trackInitTemplate after scaffoldProject in both paths Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -500,8 +500,8 @@ export default defineCommand({
|
||||
const templateId: TemplateId = templateResult;
|
||||
|
||||
// 4. Copy template and patch
|
||||
trackInitTemplate(templateId);
|
||||
scaffoldProject(destDir, name, templateId, localVideoName);
|
||||
trackInitTemplate(templateId);
|
||||
|
||||
const files = readdirSync(destDir);
|
||||
clack.note(files.map((f) => c.accent(f)).join("\n"), c.success(`Created ${name}/`));
|
||||
|
||||
@@ -49,7 +49,7 @@ export function shouldTrack(): boolean {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Placeholder API key means it hasn't been configured yet
|
||||
// Safety check: ensure the API key has been configured (phc_ prefix = valid PostHog key)
|
||||
if (!POSTHOG_API_KEY.startsWith("phc_")) {
|
||||
telemetryEnabled = false;
|
||||
return false;
|
||||
@@ -91,6 +91,8 @@ export async function flush(): Promise<void> {
|
||||
const config = readConfig();
|
||||
const batch = eventQueue.map((e) => ({
|
||||
event: e.event,
|
||||
// $ip: null tells PostHog to not record the request IP for this event.
|
||||
// Server-side "Discard client IP data" is also enabled in project settings.
|
||||
properties: { ...e.properties, $ip: null },
|
||||
distinct_id: config.anonymousId,
|
||||
timestamp: e.timestamp,
|
||||
@@ -115,9 +117,9 @@ export async function flush(): Promise<void> {
|
||||
}
|
||||
|
||||
/**
|
||||
* Synchronous flush for use in the `exit` event handler (which doesn't support async).
|
||||
* Uses a synchronous XMLHttpRequest-style approach via child_process to ensure
|
||||
* events are sent even when process.exit() is called.
|
||||
* Fire-and-forget flush for use in the `exit` event handler.
|
||||
* Spawns a detached child process that sends the HTTP request independently,
|
||||
* so the parent process exits immediately without waiting.
|
||||
*/
|
||||
export function flushSync(): void {
|
||||
if (eventQueue.length === 0) {
|
||||
@@ -136,17 +138,17 @@ export function flushSync(): void {
|
||||
const payload = JSON.stringify({ api_key: POSTHOG_API_KEY, batch });
|
||||
|
||||
try {
|
||||
// Spawn a detached process to send the request so we don't block exit.
|
||||
// The subprocess inherits nothing and runs independently.
|
||||
const { execFileSync } = require("node:child_process") as typeof import("node:child_process");
|
||||
execFileSync(
|
||||
const { spawn } = require("node:child_process") as typeof import("node:child_process");
|
||||
const child = spawn(
|
||||
process.execPath,
|
||||
[
|
||||
"-e",
|
||||
`fetch(${JSON.stringify(`${POSTHOG_HOST}/batch/`)},{method:"POST",headers:{"Content-Type":"application/json"},body:${JSON.stringify(payload)},signal:AbortSignal.timeout(${FLUSH_TIMEOUT_MS})}).catch(()=>{})`,
|
||||
],
|
||||
{ stdio: "ignore", timeout: FLUSH_TIMEOUT_MS },
|
||||
{ detached: true, stdio: "ignore" },
|
||||
);
|
||||
// Let the parent exit without waiting for the child
|
||||
child.unref();
|
||||
} catch {
|
||||
// Silently ignore
|
||||
}
|
||||
@@ -154,7 +156,8 @@ export function flushSync(): void {
|
||||
|
||||
/**
|
||||
* Show the first-run telemetry notice if it hasn't been shown yet.
|
||||
* Returns true if the notice was shown (so callers can add spacing).
|
||||
* Must be called BEFORE any tracking calls so the user sees the disclosure
|
||||
* before any data is sent.
|
||||
*/
|
||||
export function showTelemetryNotice(): boolean {
|
||||
if (!shouldTrack()) return false;
|
||||
@@ -162,6 +165,11 @@ export function showTelemetryNotice(): boolean {
|
||||
const config = readConfig();
|
||||
if (config.telemetryNoticeShown) return false;
|
||||
|
||||
// Persist the notice flag first, before any tracking occurs,
|
||||
// so the user is never tracked without having seen the disclosure.
|
||||
config.telemetryNoticeShown = true;
|
||||
writeConfig(config);
|
||||
|
||||
console.log();
|
||||
console.log(` ${c.dim("Hyperframes collects anonymous usage data to improve the tool.")}`);
|
||||
console.log(` ${c.dim("No personal info, file paths, or content is collected.")}`);
|
||||
@@ -169,7 +177,5 @@ export function showTelemetryNotice(): boolean {
|
||||
console.log(` ${c.dim("Disable anytime:")} ${c.accent("hyperframes telemetry disable")}`);
|
||||
console.log();
|
||||
|
||||
config.telemetryNoticeShown = true;
|
||||
writeConfig(config);
|
||||
return true;
|
||||
}
|
||||
|
||||
@@ -69,7 +69,7 @@ export function readConfig(): HyperframesConfig {
|
||||
*/
|
||||
export function writeConfig(config: HyperframesConfig): void {
|
||||
try {
|
||||
mkdirSync(CONFIG_DIR, { recursive: true });
|
||||
mkdirSync(CONFIG_DIR, { recursive: true, mode: 0o700 });
|
||||
writeFileSync(CONFIG_FILE, JSON.stringify(config, null, 2) + "\n", { mode: 0o600 });
|
||||
cachedConfig = { ...config };
|
||||
} catch {
|
||||
|
||||
@@ -7,6 +7,8 @@ export function isDevMode(): boolean {
|
||||
const url = new URL(import.meta.url);
|
||||
return url.pathname.endsWith(".ts");
|
||||
} catch {
|
||||
// Fail-safe: if URL parsing fails for any reason, assume production.
|
||||
// This ensures telemetry is never accidentally disabled in production builds.
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user