mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-12 07:09:59 +00:00
feat(cli): persist + show friendly user identity; preserve unknown credential fields (#1741)
* feat(cli): persist + show friendly user identity; preserve unknown credential fields The `~/.heygen/credentials` file is SHARED with the Go `heygen` CLI. This is the hyperframes-side mirror of heygen-cli#197, which adds an optional `user` block to that file. Two CLIs writing one file must round-trip each other's data without loss. Load-bearing change: the credentials reader/writer now PRESERVES unknown fields on round-trip. Previously readStore/writeStore stripped any key this CLI didn't model, so writing the file back would silently drop the `user` block heygen-cli wrote (and any future key). Unrecognized top-level keys, and unknown keys inside `oauth` / `user`, are captured on a hidden symbol slot and re-emitted verbatim. Known fields stay strictly validated. Also mirrors heygen-cli#197's friendly-display feature: - New optional `user` block schema (email/first_name/last_name/username), all omitempty; legacy files without it parse fine. - After login (OAuth + api-key paths) probe /v3/users/me, persist the block, and show a friendly name (email > "first last" > username). Probe failure is non-fatal (login still succeeds); a stale block is cleared on probe failure so a wrong account can't surface. - `auth status` surfaces the persisted block (persisted_user in JSON, a cached Account row in human output) for file-sourced credentials; env-sourced credentials skip it (the on-disk block may belong to a different key). - Fixed the OAuth write path to carry the user block + unknown keys across a fresh login / refresh (it previously rebuilt a minimal record). Tests: preserve-unknown-fields round-trip (top-level, oauth, user), the exact cross-CLI `user`-block scenario, schema round-trip + omitempty, backwards-compat with legacy files, login persistence + graceful probe failure + stale-clear, and the `auth status` surface. Full CLI suite (1009 tests) green; oxlint + oxfmt + tsc clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cli): preserve unknown credential data in cleanup/rollback paths Addresses Magi's REQUEST_CHANGES on #1741. The credentials reader/writer already round-trips unknown/foreign keys (the cross-CLI forward-compat contract), but three destructive paths still deleted the whole file when no known api_key/oauth survived — even when the hidden Symbol-keyed unknown-field bag held a future credential another CLI owns. That clobbers exactly the data this PR preserves. - Add `hasPreservedUnknownData(record)` to store.ts (checks the top-level unknown bag + the oauth/user sub-object bags) and export it via the barrel. - `clearOAuth`, `clearUserInfo`, and the failed `auth login --api-key` rollback now write the credential-less remnant (carrying the unknown bag) instead of deleting the file when unknown/foreign data survives. They still delete when nothing worth preserving remains. - Regression tests: rollback path + both cleanup paths (clearOAuth, clearUserInfo) preserve a foreign top-level key; `hasPreservedUnknownData` unit tests at all three levels. Also addresses the review's minor items: - Add a refresh-path round-trip test (`refreshTokens`) proving an unknown key inside the oauth sub-object survives a no-rotation refresh — the most-frequent write path, previously only implicitly covered. - Clarify the `userDisplayName` / `combineName` docstrings: precedence is `email > "first last" > first-only > last-only > username`. - Replace the stale `expires_at` example date in store.ts with `<ISO-8601 UTC>`. Full CLI suite green (1020 tests); tsc, oxlint, oxfmt --check all clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
James Russo
co-authored by
Claude Opus 4.8
Claude Opus 4.8
parent
c9e8dd3862
commit
b9b5780396
@@ -3,7 +3,14 @@ import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
||||
import { isAuthError } from "./errors.js";
|
||||
import { clearOAuth, deleteStore, readStore, writeStore, type Credentials } from "./store.js";
|
||||
import {
|
||||
clearOAuth,
|
||||
deleteStore,
|
||||
hasPreservedUnknownData,
|
||||
readStore,
|
||||
writeStore,
|
||||
type Credentials,
|
||||
} from "./store.js";
|
||||
|
||||
async function makeTmpDir(): Promise<string> {
|
||||
return fs.mkdtemp(join(tmpdir(), "hf-auth-store-"));
|
||||
@@ -173,12 +180,143 @@ describe("auth/store", () => {
|
||||
await expect(readStore(path)).rejects.toSatisfy((err) => isAuthError(err));
|
||||
});
|
||||
|
||||
it("drops unknown top-level keys", async () => {
|
||||
it("exposes only the typed surface for known keys (unknown keys hidden from callers)", async () => {
|
||||
// The typed `credentials` view shows only the modelled keys —
|
||||
// unknown/foreign keys are captured in a hidden (symbol-keyed)
|
||||
// passthrough slot, not the enumerable surface, so callers can't
|
||||
// accidentally read them. Round-trip preservation is covered below.
|
||||
await fs.writeFile(path, JSON.stringify({ api_key: "hg_x", future_field: { stuff: 1 } }), {
|
||||
mode: 0o600,
|
||||
});
|
||||
const result = await readStore(path);
|
||||
expect(result.credentials).toEqual({ api_key: "hg_x" });
|
||||
expect(Object.keys(result.credentials)).toEqual(["api_key"]);
|
||||
expect(result.credentials.api_key).toBe("hg_x");
|
||||
});
|
||||
|
||||
// --- Cross-CLI forward compatibility: unknown-field preservation. ---
|
||||
// The credentials file is SHARED with the Go `heygen` CLI. If this CLI
|
||||
// strips keys it doesn't model when it writes the file back, it
|
||||
// silently destroys the other CLI's data (and vice versa). The writer
|
||||
// MUST round-trip unknown fields untouched.
|
||||
|
||||
it("preserves an unknown TOP-LEVEL key across a read → write round-trip", async () => {
|
||||
// Simulate a file another CLI version wrote with a future key.
|
||||
await fs.writeFile(
|
||||
path,
|
||||
JSON.stringify({ api_key: "hg_x", future_field: { nested: [1, 2], flag: true } }),
|
||||
{ mode: 0o600 },
|
||||
);
|
||||
// Read it, then write back a typed update (here: just the api_key it
|
||||
// surfaced). The unknown key must survive.
|
||||
const { credentials } = await readStore(path);
|
||||
await writeStore(credentials, path);
|
||||
|
||||
const onDisk = JSON.parse(await fs.readFile(path, "utf8"));
|
||||
expect(onDisk.api_key).toBe("hg_x");
|
||||
expect(onDisk.future_field).toEqual({ nested: [1, 2], flag: true });
|
||||
});
|
||||
|
||||
it("preserves the heygen-cli `user` block when this CLI rewrites only the credential", async () => {
|
||||
// The exact cross-CLI data-loss scenario: heygen-cli wrote a `user`
|
||||
// block; hyperframes-cli updates the api_key and must not drop it.
|
||||
await fs.writeFile(
|
||||
path,
|
||||
JSON.stringify({
|
||||
api_key: "hg_old",
|
||||
user: { email: "jane@example.com", first_name: "Jane", last_name: "Doe", username: "jdoe" },
|
||||
}),
|
||||
{ mode: 0o600 },
|
||||
);
|
||||
const { credentials } = await readStore(path);
|
||||
credentials.api_key = "hg_new";
|
||||
await writeStore(credentials, path);
|
||||
|
||||
const onDisk = JSON.parse(await fs.readFile(path, "utf8"));
|
||||
expect(onDisk.api_key).toBe("hg_new");
|
||||
expect(onDisk.user).toEqual({
|
||||
email: "jane@example.com",
|
||||
first_name: "Jane",
|
||||
last_name: "Doe",
|
||||
username: "jdoe",
|
||||
});
|
||||
});
|
||||
|
||||
it("preserves an unknown key INSIDE the oauth sub-object", async () => {
|
||||
await fs.writeFile(
|
||||
path,
|
||||
JSON.stringify({
|
||||
oauth: { access_token: "at_1", id_token: "future_id_token_value" },
|
||||
}),
|
||||
{ mode: 0o600 },
|
||||
);
|
||||
const { credentials } = await readStore(path);
|
||||
await writeStore(credentials, path);
|
||||
|
||||
const onDisk = JSON.parse(await fs.readFile(path, "utf8"));
|
||||
expect(onDisk.oauth.access_token).toBe("at_1");
|
||||
expect(onDisk.oauth.id_token).toBe("future_id_token_value");
|
||||
});
|
||||
|
||||
it("preserves an unknown key INSIDE the user sub-object", async () => {
|
||||
await fs.writeFile(
|
||||
path,
|
||||
JSON.stringify({
|
||||
api_key: "hg_x",
|
||||
user: { email: "u@example.com", avatar_url: "https://cdn/x.png" },
|
||||
}),
|
||||
{ mode: 0o600 },
|
||||
);
|
||||
const { credentials } = await readStore(path);
|
||||
await writeStore(credentials, path);
|
||||
|
||||
const onDisk = JSON.parse(await fs.readFile(path, "utf8"));
|
||||
expect(onDisk.user.email).toBe("u@example.com");
|
||||
expect(onDisk.user.avatar_url).toBe("https://cdn/x.png");
|
||||
});
|
||||
|
||||
it("round-trips the user block (schema + omitempty: empty fields are not written)", async () => {
|
||||
const creds: Credentials = {
|
||||
api_key: "hg_x",
|
||||
user: { email: "u@example.com", username: "u" },
|
||||
};
|
||||
await writeStore(creds, path);
|
||||
const result = await readStore(path);
|
||||
expect(result.credentials.user).toEqual({ email: "u@example.com", username: "u" });
|
||||
|
||||
// omitempty: only the populated fields appear on disk — no empty
|
||||
// first_name / last_name strings littering the file.
|
||||
const onDisk = JSON.parse(await fs.readFile(path, "utf8"));
|
||||
expect(onDisk.user).toEqual({ email: "u@example.com", username: "u" });
|
||||
expect(Object.keys(onDisk.user)).toEqual(["email", "username"]);
|
||||
});
|
||||
|
||||
it('omits an all-empty user block entirely (no `"user": {}` litter)', async () => {
|
||||
await writeStore({ api_key: "hg_x", user: {} }, path);
|
||||
const onDisk = JSON.parse(await fs.readFile(path, "utf8"));
|
||||
expect(onDisk.user).toBeUndefined();
|
||||
expect(onDisk.api_key).toBe("hg_x");
|
||||
});
|
||||
|
||||
it("backwards-compat: a legacy file WITHOUT a user block parses with user undefined", async () => {
|
||||
await fs.writeFile(path, JSON.stringify({ api_key: "hg_legacy" }), { mode: 0o600 });
|
||||
const result = await readStore(path);
|
||||
expect(result.source).toBe("file_json");
|
||||
expect(result.credentials.api_key).toBe("hg_legacy");
|
||||
expect(result.credentials.user).toBeUndefined();
|
||||
});
|
||||
|
||||
it("ignores a malformed user sub-field rather than rejecting the whole file", async () => {
|
||||
// The user block is additive metadata — a junk sub-field must never
|
||||
// block resolving a perfectly good api_key. Non-string fields are
|
||||
// dropped; the credential survives.
|
||||
await fs.writeFile(
|
||||
path,
|
||||
JSON.stringify({ api_key: "hg_x", user: { email: "u@example.com", first_name: 12345 } }),
|
||||
{ mode: 0o600 },
|
||||
);
|
||||
const result = await readStore(path);
|
||||
expect(result.credentials.api_key).toBe("hg_x");
|
||||
expect(result.credentials.user).toEqual({ email: "u@example.com" });
|
||||
});
|
||||
|
||||
it("deleteStore is idempotent", async () => {
|
||||
@@ -202,8 +340,119 @@ describe("auth/store", () => {
|
||||
await expect(fs.access(path)).rejects.toThrow();
|
||||
});
|
||||
|
||||
it("clearOAuth keeps the user block (and unknown keys) when an api_key survives", async () => {
|
||||
await fs.writeFile(
|
||||
path,
|
||||
JSON.stringify({
|
||||
api_key: "hg_keep",
|
||||
oauth: { access_token: "drop_me" },
|
||||
user: { email: "u@example.com" },
|
||||
future_field: 1,
|
||||
}),
|
||||
{ mode: 0o600 },
|
||||
);
|
||||
await clearOAuth(path);
|
||||
const onDisk = JSON.parse(await fs.readFile(path, "utf8"));
|
||||
expect(onDisk.oauth).toBeUndefined();
|
||||
expect(onDisk.api_key).toBe("hg_keep");
|
||||
expect(onDisk.user).toEqual({ email: "u@example.com" });
|
||||
expect(onDisk.future_field).toBe(1);
|
||||
});
|
||||
|
||||
it("clearOAuth is a no-op when file is absent", async () => {
|
||||
await clearOAuth(path);
|
||||
await expect(fs.access(path)).rejects.toThrow();
|
||||
});
|
||||
|
||||
// --- Destructive paths must not clobber preserved unknown data. ---
|
||||
// When clearing the only known credential would otherwise delete the
|
||||
// file, a surviving unknown/foreign top-level key (a future credential
|
||||
// another CLI owns) must keep the file alive — deleting would clobber
|
||||
// exactly the cross-CLI data this machinery exists to preserve.
|
||||
|
||||
it("clearOAuth keeps the file (writing the unknown bag) when no api_key but a foreign top-level key survives", async () => {
|
||||
await fs.writeFile(
|
||||
path,
|
||||
JSON.stringify({
|
||||
oauth: { access_token: "drop_me" },
|
||||
future_credential: { token: "owned_by_other_cli" },
|
||||
}),
|
||||
{ mode: 0o600 },
|
||||
);
|
||||
await clearOAuth(path);
|
||||
// File must still exist and carry the foreign key.
|
||||
const onDisk = JSON.parse(await fs.readFile(path, "utf8"));
|
||||
expect(onDisk.oauth).toBeUndefined();
|
||||
expect(onDisk.future_credential).toEqual({ token: "owned_by_other_cli" });
|
||||
});
|
||||
|
||||
it("clearOAuth keeps the file when no api_key but a foreign key survives inside the user block", async () => {
|
||||
// The user block has no known friendly fields, only a foreign sub-key
|
||||
// — the block itself survives the oauth clear, so its unknown data
|
||||
// must too.
|
||||
await fs.writeFile(
|
||||
path,
|
||||
JSON.stringify({
|
||||
oauth: { access_token: "drop_me" },
|
||||
user: { external_org_id: "org_123" },
|
||||
}),
|
||||
{ mode: 0o600 },
|
||||
);
|
||||
await clearOAuth(path);
|
||||
const onDisk = JSON.parse(await fs.readFile(path, "utf8"));
|
||||
expect(onDisk.oauth).toBeUndefined();
|
||||
expect(onDisk.user).toEqual({ external_org_id: "org_123" });
|
||||
});
|
||||
|
||||
it("clearOAuth still deletes the file when only a known (empty-after-clear) surface remains", async () => {
|
||||
// No api_key, no foreign data — just the oauth block being cleared.
|
||||
// Nothing worth preserving, so the file goes.
|
||||
await writeStore({ oauth: { access_token: "only" } }, path);
|
||||
await clearOAuth(path);
|
||||
await expect(fs.access(path)).rejects.toThrow();
|
||||
});
|
||||
|
||||
describe("hasPreservedUnknownData", () => {
|
||||
it("false for an empty record", () => {
|
||||
expect(hasPreservedUnknownData({})).toBe(false);
|
||||
});
|
||||
|
||||
it("false for a record with only known fields", () => {
|
||||
expect(
|
||||
hasPreservedUnknownData({
|
||||
api_key: "hg_x",
|
||||
oauth: { access_token: "at" },
|
||||
user: { email: "u@example.com" },
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("true when a top-level unknown key was captured at read time", async () => {
|
||||
await fs.writeFile(path, JSON.stringify({ api_key: "hg_x", future_field: 1 }), {
|
||||
mode: 0o600,
|
||||
});
|
||||
const { credentials } = await readStore(path);
|
||||
expect(hasPreservedUnknownData(credentials)).toBe(true);
|
||||
});
|
||||
|
||||
it("true when an unknown key was captured inside the oauth sub-object", async () => {
|
||||
await fs.writeFile(
|
||||
path,
|
||||
JSON.stringify({ oauth: { access_token: "at", id_token: "future" } }),
|
||||
{ mode: 0o600 },
|
||||
);
|
||||
const { credentials } = await readStore(path);
|
||||
expect(hasPreservedUnknownData(credentials)).toBe(true);
|
||||
});
|
||||
|
||||
it("true when an unknown key was captured inside the user sub-object", async () => {
|
||||
await fs.writeFile(
|
||||
path,
|
||||
JSON.stringify({ api_key: "hg_x", user: { email: "u@example.com", avatar_url: "x" } }),
|
||||
{ mode: 0o600 },
|
||||
);
|
||||
const { credentials } = await readStore(path);
|
||||
expect(hasPreservedUnknownData(credentials)).toBe(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user