Repository navigation
fix(scripts): install manifest assets before linting registry items (#7279) - #7282
xunguangzlj-cloud wants to merge 2 commits into
Conversation
Re-verified the nine merged PRs in heygen-com#7266-heygen-com#7288 against fork file:line and reverse-apply checks; none needs a new port. The six open PRs (heygen-com#7266, heygen-com#7269, heygen-com#7281, heygen-com#7282, heygen-com#7285, heygen-com#7286) are pinned as monitored with revisit conditions; monitored 99 -> 105. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jrusso1020
left a comment
There was a problem hiding this comment.
Reviewed at 951cee8ca7279c0940df4436b431be99a0c68e9f.
Registry lint mounts manifest-declared assets at their target paths and reports spawn failure instead of treating it as a pass (scripts/lint-registry-items.mjs:113). Genuine missing assets remain findings.
CI integration prerequisite (non-blocking for code approval): add scripts/lint-registry-items.test.mjs to root package.json's explicit test:scripts list. node scripts/check-test-reachability.mjs currently fails with no CI runner selects this test; the main control passes. The runtime fix is clean, but this head cannot satisfy the normal merge gate until that small runner-wiring change lands.
Validation at this head: node --test scripts/lint-registry-items.test.mjs → 3 passed, including the real script on both reported items. Focused tests ran after a full build on current main; built shared parser dependencies were reused. No full render replay or native Windows run locally. git merge-tree --write-tree origin/main 951cee8ca7279c0940df4436b431be99a0c68e9f passed against main 539c70cee. No prior substantive reviews or inline comments were present.
Need: yes, requested starter issue · Size: ok · Reuse: existing lint/parser and manifest contracts · Simpler: already minimal · Structure: fine
Risk: low — bounded diagnostic or registry-lint change with focused coverage
— hf-review
Verdict: APPROVE
Reasoning: The requested behavior is implemented using the existing contracts, and the focused regression tests pass. The missing test-runner entry is a small integration fix; merge still needs normal green checks.
jrusso1020
left a comment
There was a problem hiding this comment.
Reviewed at 0faec0a1c0be90b2751a7c99d26a484e32cbf2e3 (delta from 951cee8ca7279c0940df4436b431be99a0c68e9f).
The prior test-runner integration concern is resolved: root package.json:57 explicitly selects the new test, and node scripts/check-test-reachability.mjs reports Test reachability verified: zero orphan tests. Manifest copies now check both source and target with the existing filesystem-aware containment helper before copying (scripts/lint-registry-items.mjs:116). No Important findings from the independent adversarial re-review.
Validation: a fresh-worktree bun install --frozen-lockfile --ignore-scripts and full bun run build passed. node --test scripts/lint-registry-items.test.mjs scripts/registry-target-paths.test.mjs passed 17/17, including the real CLI lint on both reported items and filesystem symlink cases. Both new escaping-entry tests fail with the previous reviewed script (0/2 pass), confirming they detect this guard change. Separate real-script probes verified that an escaping source symlink and unavailable Bun fail closed. Changed-file oxfmt/oxlint and merge-tree against main f004622a6 passed. No native Windows run or full registry/render replay locally.
CI remains in progress; merge still requires the normal green checks.
Need: yes, fixes requested registry lint behavior · Size: ok · Reuse: existing registry containment and CLI lint contracts · Simpler: already minimal · Structure: fine
Risk: low — bounded developer lint script with focused integration and containment coverage
— hf-review
Verdict: APPROVE
Reasoning: The manifest assets are mounted under the existing containment contract, and the focused regression suite now participates in CI. The earlier runner concern is fixed and the tested escape cases are rejected.
Closes #7279
What this changes
Registry lint mounted the composition HTML alone. Items such as
yt-lower-thirdandtexture-mask-textfailed even though their manifests listed the missing assets.test:scriptsso CI runs it.Test
Prepared and revised with AI assistance.