mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-07 18:26:17 +00:00
fix(cli): persist authoring skill in hyperframes.json for durable render attribution (#2762)
* fix(cli): persist authoring skill in hyperframes.json for durable render attribution authoring_skill was stamped only on the first render through a workflow passing --skill, so re-renders, `npm run render`, --batch, existing-project renders, and general-video lost it — leaving 77-96% of real-human render volume un-attributed and the skills-penetration metric misleadingly low. Persist the owning skill in hyperframes.json: `init --skill` stamps it at creation, `render` resolves the flag then falls back to the stored value, and an explicit --skill seeds it (seed-once, never overwriting the creating workflow's identity). Activate all render-producing creation workflows to declare their skill at init. Forward-only: does not rewrite historical telemetry. * fix(cli): patch hyperframes.json in place when seeding the authoring skill seedProjectAuthoringSkill is the only writer that touches an already existing hyperframes.json — every other writeProjectConfig call site is guarded to write only when the file is absent, which made the whole-file overwrite safe by construction. Round-tripping the seed through normalizeConfig broke that: it rebuilds the object from a field whitelist with no rest-spread, so any key outside the schema was silently dropped, a media block was materialized in projects that never had one, and key order was rewritten. hyperframes.json is normally committed, so a render introduced a diff the user never asked for, and any field added to the schema later would be deleted by a render on an older CLI. Parse the raw JSON, set authoringSkill, write it back, reusing the file's own indentation. Unknown keys and formatting survive; the only delta is the key being added. A corrupt config is now left untouched instead of clobbered. Seed-once semantics are unchanged, still normalized so a hand-edited garbage slug neither reaches telemetry nor wedges the seed. Reported independently by both reviewers on #2762. * fix(cli): create the docker build context with mkdtempSync The `--docker` build context was created at a guessable path derived from `Date.now()` in the world-writable OS temp dir. Another local user can pre-create or symlink that path and have the build read a Dockerfile they control. mkdtempSync gets a random suffix and 0o700 from the kernel, and it creates the directory itself, so the separate mkdirSync goes away. Pre-existing on main (alert #432, 2026-06-04, packages/cli/src/commands/render.ts), surfaced against this branch only because the seed commit shifted line numbers in the same file. Fixed here to unblock the CodeQL gate on #2762 rather than left for a follow-up; the remaining 10 js/insecure-temporary-file alerts elsewhere in the repo are untouched and still want their own pass. * fix(cli): drop the check-then-use race when seeding the authoring skill The seed tested for the config with existsSync and then wrote, which is a check-then-use race: the file can be created or swapped between the check and the write (CodeQL js/file-system-race). Read once and branch on the failure reason instead. Only ENOENT creates a config from scratch; any other read failure (permissions, I/O) now leaves an existing file alone rather than overwriting it with a default, so this is also strictly safer than the version it replaces. Also replaces the `as Record<string, unknown>` assertion with an isJsonObject type guard, per the repo's no-assertion convention. Behaviour unchanged: all 4 seed regression tests still pass, and the create/preserve/seed-once/corrupt-untouched paths were re-verified end to end.
This commit is contained in:
WaterrrForever
@@ -9,6 +9,7 @@ import {
|
||||
projectConfigPath,
|
||||
readProjectConfig,
|
||||
resolveAutoProxy,
|
||||
seedProjectAuthoringSkill,
|
||||
writeProjectConfig,
|
||||
PROJECT_CONFIG_FILENAME,
|
||||
} from "./projectConfig.js";
|
||||
@@ -84,6 +85,18 @@ describe("projectConfig", () => {
|
||||
const result = normalizeConfig({ media: "nope" as unknown as never });
|
||||
expect(result.media).toEqual({ autoProxy: true });
|
||||
});
|
||||
|
||||
it("preserves a valid authoringSkill slug", () => {
|
||||
const result = normalizeConfig({ authoringSkill: "product-launch-video" });
|
||||
expect(result.authoringSkill).toBe("product-launch-video");
|
||||
});
|
||||
|
||||
it("drops an invalid authoringSkill (never reaches telemetry)", () => {
|
||||
const result = normalizeConfig({
|
||||
authoringSkill: "Not A Slug!" as unknown as never,
|
||||
});
|
||||
expect(result.authoringSkill).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("readProjectConfig", () => {
|
||||
@@ -228,4 +241,127 @@ describe("projectConfig", () => {
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("seedProjectAuthoringSkill", () => {
|
||||
it("stamps the owning skill into a fresh project (creates the config)", () => {
|
||||
const dir = tmp();
|
||||
try {
|
||||
seedProjectAuthoringSkill(dir, "faceless-explainer");
|
||||
expect(loadProjectConfig(dir).authoringSkill).toBe("faceless-explainer");
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("preserves other config fields when stamping an existing config", () => {
|
||||
const dir = tmp();
|
||||
try {
|
||||
writeProjectConfig(dir, {
|
||||
...DEFAULT_PROJECT_CONFIG,
|
||||
registry: "https://custom.example.com",
|
||||
});
|
||||
seedProjectAuthoringSkill(dir, "pr-to-video");
|
||||
const read = readProjectConfig(dir);
|
||||
expect(read?.authoringSkill).toBe("pr-to-video");
|
||||
expect(read?.registry).toBe("https://custom.example.com");
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("is seed-once: a later --skill never overwrites the project owner", () => {
|
||||
const dir = tmp();
|
||||
try {
|
||||
seedProjectAuthoringSkill(dir, "product-launch-video");
|
||||
seedProjectAuthoringSkill(dir, "motion-graphics");
|
||||
expect(loadProjectConfig(dir).authoringSkill).toBe("product-launch-video");
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("ignores an invalid slug and writes nothing", () => {
|
||||
const dir = tmp();
|
||||
try {
|
||||
seedProjectAuthoringSkill(dir, "Not A Slug!");
|
||||
expect(readProjectConfig(dir)).toBeUndefined();
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
// The seed is the only writer that touches an existing hyperframes.json,
|
||||
// which is normally committed — a render must not diff it beyond the one
|
||||
// key being added. Guards against round-tripping through normalizeConfig.
|
||||
it("preserves config keys outside the known schema", () => {
|
||||
const dir = tmp();
|
||||
try {
|
||||
writeFileSync(
|
||||
projectConfigPath(dir),
|
||||
JSON.stringify(
|
||||
{
|
||||
registry: "https://example.com/my-registry",
|
||||
myTeamSetting: { reviewer: "wenbo", keep: true },
|
||||
futureSchemaKey: 42,
|
||||
},
|
||||
null,
|
||||
2,
|
||||
),
|
||||
"utf-8",
|
||||
);
|
||||
seedProjectAuthoringSkill(dir, "product-launch-video");
|
||||
const raw = JSON.parse(readFileSync(projectConfigPath(dir), "utf-8"));
|
||||
expect(raw.authoringSkill).toBe("product-launch-video");
|
||||
expect(raw.myTeamSetting).toEqual({ reviewer: "wenbo", keep: true });
|
||||
expect(raw.futureSchemaKey).toBe(42);
|
||||
expect(raw.registry).toBe("https://example.com/my-registry");
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("does not materialize a media block the user never wrote", () => {
|
||||
const dir = tmp();
|
||||
try {
|
||||
writeFileSync(
|
||||
projectConfigPath(dir),
|
||||
JSON.stringify({ registry: "https://example.com/r" }, null, 2),
|
||||
"utf-8",
|
||||
);
|
||||
seedProjectAuthoringSkill(dir, "motion-graphics");
|
||||
const raw = JSON.parse(readFileSync(projectConfigPath(dir), "utf-8"));
|
||||
expect(raw.media).toBeUndefined();
|
||||
expect(raw.$schema).toBeUndefined();
|
||||
expect(Object.keys(raw)).toEqual(["registry", "authoringSkill"]);
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("reuses the file's own indentation", () => {
|
||||
const dir = tmp();
|
||||
try {
|
||||
writeFileSync(
|
||||
projectConfigPath(dir),
|
||||
JSON.stringify({ registry: "https://example.com/r" }, null, 4),
|
||||
"utf-8",
|
||||
);
|
||||
seedProjectAuthoringSkill(dir, "pr-to-video");
|
||||
expect(readFileSync(projectConfigPath(dir), "utf-8")).toContain('\n "registry"');
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("leaves a corrupt config untouched rather than clobbering it", () => {
|
||||
const dir = tmp();
|
||||
try {
|
||||
writeFileSync(projectConfigPath(dir), "{ not valid json", "utf-8");
|
||||
seedProjectAuthoringSkill(dir, "faceless-explainer");
|
||||
expect(readFileSync(projectConfigPath(dir), "utf-8")).toBe("{ not valid json");
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -10,6 +10,7 @@
|
||||
import { readFileSync, writeFileSync } from "node:fs";
|
||||
import { join, resolve } from "node:path";
|
||||
import { DEFAULT_REGISTRY_URL } from "../registry/index.js";
|
||||
import { normalizeSkillSlug } from "../telemetry/skill.js";
|
||||
|
||||
export const PROJECT_CONFIG_FILENAME = "hyperframes.json";
|
||||
const PROJECT_CONFIG_SCHEMA_URL = "https://hyperframes.heygen.com/schema/hyperframes.json";
|
||||
@@ -40,6 +41,14 @@ export interface ProjectConfig {
|
||||
paths: ProjectConfigPaths;
|
||||
/** Media handling options (e.g. auto-proxying of browser-hostile codecs). */
|
||||
media?: ProjectConfigMedia;
|
||||
/**
|
||||
* Owning authoring-workflow skill slug (e.g. "product-launch-video"). Stamped
|
||||
* by `hyperframes init --skill` or seeded from the first `hyperframes render
|
||||
* --skill`, then read back so every later render of this project — re-render,
|
||||
* `npm run render`, `--batch`, preview — is attributed to it on anonymous
|
||||
* telemetry without the caller re-passing the flag.
|
||||
*/
|
||||
authoringSkill?: string;
|
||||
}
|
||||
|
||||
export const DEFAULT_PROJECT_CONFIG: ProjectConfig = {
|
||||
@@ -92,6 +101,9 @@ export function normalizeConfig(partial: Partial<ProjectConfig>): ProjectConfig
|
||||
? partial.media.autoProxy
|
||||
: DEFAULT_PROJECT_CONFIG.media?.autoProxy,
|
||||
},
|
||||
// Slug-gate on read so a hand-edited or corrupt value never reaches the
|
||||
// telemetry stream; an invalid slug simply drops the attribution.
|
||||
authoringSkill: normalizeSkillSlug(partial.authoringSkill),
|
||||
};
|
||||
}
|
||||
|
||||
@@ -127,3 +139,74 @@ export function resolveAutoProxy(projectDir: string, flagValue: boolean | undefi
|
||||
}
|
||||
return loadProjectConfig(projectDir).media?.autoProxy ?? true;
|
||||
}
|
||||
|
||||
/** A parsed JSON value that can carry arbitrary keys — narrowed, not asserted. */
|
||||
function isJsonObject(value: unknown): value is Record<string, unknown> {
|
||||
return typeof value === "object" && value !== null && !Array.isArray(value);
|
||||
}
|
||||
|
||||
/** True when a thrown filesystem error reports the path as absent. */
|
||||
function isFileNotFound(error: unknown): boolean {
|
||||
return typeof error === "object" && error !== null && "code" in error && error.code === "ENOENT";
|
||||
}
|
||||
|
||||
/**
|
||||
* Persist the owning authoring-skill slug into `hyperframes.json` so every
|
||||
* later render of this project — re-render, `npm run render`, `--batch`,
|
||||
* preview — is attributed to the workflow that created it, without the caller
|
||||
* re-passing `--skill`.
|
||||
*
|
||||
* Seed-once: an existing stamp is never overwritten (the creating workflow owns
|
||||
* the identity; a one-off `--skill` on a later render still governs that
|
||||
* render's telemetry but does not rewrite the project's owner). An invalid or
|
||||
* empty slug is ignored. Best effort: a read-only or missing project directory
|
||||
* never fails the render it rode in on.
|
||||
*
|
||||
* This is the only writer that touches an ALREADY EXISTING `hyperframes.json`
|
||||
* (every other `writeProjectConfig` call site is guarded to write only when the
|
||||
* file is absent), so it must not round-trip through {@link normalizeConfig}:
|
||||
* that rebuilds the object from a field whitelist, which would drop keys it
|
||||
* does not know about and materialize defaults the user never wrote. The file
|
||||
* is normally committed, so a render must not introduce a diff beyond the one
|
||||
* key being added. Patch the parsed JSON in place instead, reusing the file's
|
||||
* own indentation.
|
||||
*/
|
||||
export function seedProjectAuthoringSkill(projectDir: string, rawSkill: unknown): void {
|
||||
const skill = normalizeSkillSlug(rawSkill);
|
||||
if (!skill) return;
|
||||
const path = projectConfigPath(projectDir);
|
||||
|
||||
// Read once and branch on why the read failed, rather than testing for the
|
||||
// file first: an `existsSync`-then-write pair is a check-then-use race, and
|
||||
// only a genuinely absent config may be created from scratch — any other
|
||||
// read failure (permissions, I/O) must leave an existing file alone instead
|
||||
// of overwriting it with a default.
|
||||
let text: string;
|
||||
try {
|
||||
text = readFileSync(path, "utf-8");
|
||||
} catch (error) {
|
||||
if (isFileNotFound(error)) {
|
||||
try {
|
||||
writeProjectConfig(projectDir, { ...DEFAULT_PROJECT_CONFIG, authoringSkill: skill });
|
||||
} catch {
|
||||
// Read-only or missing project directory — best effort.
|
||||
}
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
try {
|
||||
const parsed: unknown = JSON.parse(text);
|
||||
// A malformed config is left untouched rather than clobbered by a render.
|
||||
if (!isJsonObject(parsed)) return;
|
||||
// Seed-once. Normalized so a hand-edited garbage slug neither reaches
|
||||
// telemetry nor wedges the seed — the next `--skill` render heals it.
|
||||
if (normalizeSkillSlug(parsed.authoringSkill)) return;
|
||||
parsed.authoringSkill = skill;
|
||||
const indent = /\n([ \t]+)"/.exec(text)?.[1] ?? " ";
|
||||
writeFileSync(path, JSON.stringify(parsed, null, indent) + "\n", "utf-8");
|
||||
} catch {
|
||||
// Corrupt JSON, or a read-only file: attribution is best-effort telemetry,
|
||||
// never a render blocker.
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user