chore(skills): package Codex plugin upload#2668
Conversation
vanceingalls
left a comment
There was a problem hiding this comment.
Read at head d0156ed1b4f2b655d1ac239938763f60ef755e6a. Packaging spec is clean and I verified every referenced path exists at HEAD, but one line in the claude → code-editorial rename swept a functional identifier that will regress PR-video credits. Details below.
P1 — pr-to-video/scripts/ingest.mjs BOT_DENYLIST rename is a functional regression
skills/pr-to-video/scripts/ingest.mjs line 120: "claude" was replaced with "code-editorial".
const BOT_DENYLIST = new Set(
[
"code-editorial", // ← was "claude"
"graphite-app",
"dependabot",
"github-actions",
"codecov",
"codecov-commenter",
"coderabbitai",
"sonarcloud",
…
]
);The set filters GitHub bot logins out of contributors before the credits close — every other entry is a real bot username (graphite-app, dependabot, github-actions, codecov, coderabbitai, sonarcloud, vercel, netlify, renovate, snyk-bot, mergify, …). "claude" matched the Anthropic reviewer account; "code-editorial" is a visual-preset name, not a GitHub login — no one has that username.
Effect: in Claude Code environments (which still ship these skills via the .claude/skills/ mirror), any PR reviewed or authored by the claude bot will now appear in the video's credits close as if it were a human contributor. Codex environments are unaffected either way, so the rename is pure downside.
Fix: revert this one line to "claude" (independent of the visual-preset rename). Suggestion:
- "code-editorial",
+ "claude",If you're intentionally decoupling from Anthropic bot filtering, delete the entry entirely — but don't leave "code-editorial" in a bot-login list.
Non-blockers
hyperframes-core/references/subagent-dispatch.mdlost the concrete harness table. The Primitive-map trading concrete Claude Code / Codex / OpenClaw / Hermes rows for portable prose is coherent with the PR intent, but it also dropped the Codex-specific "user must explicitly grant delegation before spawn — ask one line at the first user pause" policy-gate warning. That warning is load-bearing for Codex; agents running Codex will silently degrade to inline serial without it. Consider keeping that one Codex callout even in the portable framing.hyperframes-cli/references/upgrade-info-misc.mdflattened theagent_runtimetelemetry enum. Downstream consumers grep for the concrete values (claude_code / codex / cursor / copilot_agent / jules / replit / devin / aider / gemini_cli / hermes / openclaw / null). Keeping the list documented is cheap.docs/superpowers/specs/2026-07-13-media-use-sfx-cli-advisory-design.mdstill contains thecurl -fsSL https://static.heygen.ai/cli/install.sh | bashexample — inconsistent with the PR's shell-pipe cleanup. Historical spec doc, so may be intentional; a quick grep sweep ofdocs/superpowers/would confirm nothing else was missed.
Positive spot-checks
scripts/package-codex-plugin.mjs— clean 37 lines,git archive HEADfor determinism,--prefix=hyperframes/matches the spec, 100 MB cap enforced. All four pathspecs verified to exist at HEAD (.codex-plugin/plugin.json,assets/icon.png,assets/logo.png,skills/). CI's 11.7 MB result leaves a healthy margin.frame-presets/claude→code-editorialrename is consistent — no strayframe-presets/claudereferences at HEAD across the 16 doc callers I checked (pr-to-video/SKILL.md,references/*.md,frame-contract.test.mjs,design-spec.md,frame-worker.md).nightcitytheme +coverwordsetpiece +cyberpunk-widths.json+CyberpunkReplica.ttfremovals are complete — dispatch entry, preference map, and function body all deleted frommake-theme.cjs;CATALOG.md,PORTING.md,README.md, and the CJK routing line all updated.figma/SKILL.md— removing "have user paste FIGMA_TOKEN" and generalizing MCP → connector is a real security improvement without breaking the flow (preflight still checks env / .env before erroring; themotionContextToDocsand validation-via-verify-motion.mjs steps are unchanged).media-use/setup-providers.md— bothcurl -fsSL … | bashsites in the file (code fence + table row) are removed consistently.
Recommendation: fix the BOT_DENYLIST line, then this is good to merge. Happy to re-verify.
Review by Via
james-russo-rames-d-jusso
left a comment
There was a problem hiding this comment.
Reviewed at d0156ed.
Fanned out four parallel adversarial sub-agents — one per surface — to answer your regression-risk question directly. Summary posture: no blockers, low regression risk from the changes as shipped. The concerns worth surfacing are all about future-proofing (drift risk) rather than defects in this diff.
Rename (claude → code-editorial) — complete. Tests (frame-contract.test.mjs), SKILL.md, ingest.mjs, and all touched docs consistently use code-editorial. Residual claude matches worktree-wide are all legitimate Anthropic-product references (Claude Code CLI, CLAUDE.md, docs nav slugs) — none are stale preset keys. Zero residue.
CP2077 theme removal — one orphan (see below), otherwise clean. nightcity, cyberpunk, CDPR, fankit sweep across *.{md,json,cjs,mjs,js,ts,tsx,txt} shows the removal is byte-thorough. skills-manifest.json clean. make-theme.cjs post-deletion has zero orphan refs. The one orphan is pre-existing (outside this PR's diff so I can't inline-anchor): skills/embedded-captions/.gitignore has three stale lines referencing assets/brand/Cyberpunk2077.svg with a comment claiming cyberpunk-widths.json is "consumed at runtime" — but the entire assets/brand/ directory was deleted, so both the ignored file and the referenced runtime asset no longer exist. Cleanup opportunity — could fold into this PR as a follow-up commit.
Install-flow tightening — clean. Zero curl … | bash user-facing patterns. Secret-paste guidance replaced with explicit "never ask them to paste the token into the conversation" in figma/SKILL.md:28. Self-update (skillsUpdateCheck.ts, HYPERFRAMES_NO_UPDATE_CHECK) and consent-gated telemetry (telemetry/client.ts, HYPERFRAMES_NO_TELEMETRY) both retained end-to-end.
Figma conditional — clean. Gate is documentation-level (SKILL.md instructs the agent to check connector availability and fall back to native-export requests); downstream code paths (packages/core/src/figma/*) are pure transforms over agent-provided JSON, so no crash surface if the connector is absent.
Package script (scripts/package-codex-plugin.mjs) — sane baseline. git archive HEAD sidesteps symlink/path-traversal issues, gives byte-deterministic output, and enforces the single hyperframes/ root via --prefix. Two future-proofing concerns + one Codex-limit-precision nit + one working-tree foot-gun inline below.
No blockers. Regression risk on the four content surfaces is low — the packaging script's manifest-drift and CI-enforcement gaps are the ones worth thinking about before this becomes routine release plumbing.
| const REPO_ROOT = join(import.meta.dirname, ".."); | ||
| const OUTPUT = join(REPO_ROOT, "dist", "hyperframes-plugin.zip"); | ||
| const MAX_UPLOAD_BYTES = 100 * 1_000_000; | ||
| const PLUGIN_PATHS = [".codex-plugin", "assets/icon.png", "assets/logo.png", "skills"]; |
There was a problem hiding this comment.
🟠 Concern — hardcoded asset list drifts from .codex-plugin/plugin.json. The manifest is the source-of-truth for which brand assets Codex will resolve at runtime, but this script hardcodes assets/icon.png + assets/logo.png alongside it. If a future PR adds e.g. assets/dark-logo.png to plugin.json without editing this script, the archive ships a manifest with an unresolvable asset reference — git archive won't fail (the missing path isn't listed here), the size check is far below 100 MB, and there's no CI alarm. Suggest parsing .codex-plugin/plugin.json and deriving asset paths from the manifest's interface.composerIcon / interface.logo (and any future asset keys), then failing loud if a referenced file is missing at HEAD. Non-blocking today because the manifest is simple; the drift risk shows up the first time an asset gets added or renamed. — Rames D Jusso
| ); | ||
|
|
||
| const bytes = statSync(OUTPUT).size; | ||
| if (bytes > MAX_UPLOAD_BYTES) { |
There was a problem hiding this comment.
🟠 Concern — 100 MB guard has no CI enforcement, only manual invocation. The throw fires only when someone runs bun run package:codex-plugin locally. If a future skill lands a big fixture (HEVC test asset, PNG-sequence render sample, model weights) that pushes the archive past 100 MB, the regression is invisible until the next manual upload — which might be days or weeks after merge, at which point bisecting to the guilty PR is annoying. Suggest wiring a lightweight CI step (in ci.yml or a nightly job) that runs bun run package:codex-plugin and asserts the size ceiling. Even a PR-time invocation would give a signal. Non-blocking because the current archive is 11.7 MB — plenty of headroom — but the guard's value is proportional to how often it runs. — Rames D Jusso
|
|
||
| const REPO_ROOT = join(import.meta.dirname, ".."); | ||
| const OUTPUT = join(REPO_ROOT, "dist", "hyperframes-plugin.zip"); | ||
| const MAX_UPLOAD_BYTES = 100 * 1_000_000; |
There was a problem hiding this comment.
🟡 Nit — decimal MB vs binary MiB. 100 * 1_000_000 = 100,000,000 bytes (decimal MB). If Codex actually enforces 100 MiB (104,857,600 bytes = binary), this script rejects at ~95.4 MiB, which is a ~4.9 MB conservative margin. If Codex enforces exactly 100 MB (decimal) then the script is precise. Suggest confirming the exact byte value from Codex docs and adding a comment linking the source — a future contributor shouldn't have to re-derive this. — Rames D Jusso
| "--prefix=hyperframes/", | ||
| "--output", | ||
| OUTPUT, | ||
| "HEAD", |
There was a problem hiding this comment.
❓ Question — git archive HEAD intentionally ignores the working tree; how does this compose with scripts/set-version.ts? README notes the archive is built from committed HEAD, which is the correct default for a release-artifact packager. But if the release flow ever pairs this with scripts/set-version.ts (bump version → package → upload) and someone forgets to commit between the bump and the pack, the uploaded archive carries the previous version. A one-line dirty-tree warning (git diff --quiet HEAD -- .codex-plugin skills assets, non-fatal) would make the failure mode loud without changing the intentional HEAD semantic. Fine either way — just wanted to make sure the composition is intentional. — Rames D Jusso
|
|
||
| ## Setup — install heygen first (free-usage path) | ||
|
|
||
| Install the HeyGen CLI through its verified release instructions, then run: |
There was a problem hiding this comment.
🟡 Nit — «verified HeyGen release instructions» has no URL. The prior text was a concrete curl … | bash command; the replacement points at instructions without saying where they live. Users following this doc will need to guess where the verified instructions actually are (docs.heygen.com? GitHub Releases? Codex marketplace?). Linking the canonical location — even inline — would close the discoverability gap that the security tightening opened. Same nit applies at :65 (tools table row). Not a security concern, just a UX affordance. — Rames D Jusso
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
…tcity removal The nightcity theme removal left SKILL.md claiming 36 identities in four places, including the frontmatter description the router reads. The catalog now has 35 entries (10 classic + 25 themed).
…ble-plugin # Conflicts: # skills-manifest.json
james-russo-rames-d-jusso
left a comment
There was a problem hiding this comment.
R1 LGTM stands. Delta since (d0156ed1b → 046f88d88): Miao's c1eb12876 catalog-count 36 → 35 (nightcity removal follow-through, benign) + a merge from main to resolve the skills-manifest.json conflict. Stamping on Miao's ask.
Summary
bun run package:codex-pluginto build an upload-ready Codex plugin ZIP, including only the manifest, referenced brand assets, andskills/under onehyperframes/root.claudetocode-editorial.Verification
bun run package:codex-plugin(11.7 MB archive)node --test skills/pr-to-video/scripts/frame-contract.test.mjsnode --test skills/hyperframes-animation/scripts/package-loader.test.mjs skills/hyperframes-creative/scripts/package-loader.test.mjs