Repair Linux bring-up and add cross-platform CI - #12
Conversation
The repository had never been exercised on Linux and had no continuous integration. Running the full pipeline on x86-64 Linux found four defects, all of which predated this change and reached main directly. scripts/pi/bootstrap.sh aborted at the native host step. The crate gained a second binary in c324fe8 without declaring default-run, so every documented `cargo run -p vcg-host -- ...` invocation was ambiguous. Eleven call sites across README.md, native/vcg-host/README.md, and docs/WINDOWS_QUALIFICATION.md were affected; one manifest key repairs all of them. The day-one Pi script had never completed. The 720p launcher overlapped critical text on Linux. .home-view gave its heading row a fixed 180px minimum, which overrides the content-derived minimum and lets the row be sized smaller than the text it holds, spilling the heading's trailing line into the destinations. The defect existed on both platforms; only Linux font metrics wrapped .destination small to a second line, taking the remaining slack and turning a latent spill into a visible collision. The row minimum is now content-derived and the TV vertical rhythm scales with viewport height. Seven clippy findings under the workspace's opted-in pedantic lints are fixed by refactoring rather than suppression. One, large_enum_variant on InstalledRuntime, was visible only on Windows, where PathBuf is 32 bytes against 24 on Linux, pushing the variant gap past the 200-byte threshold. Four more were masked entirely, because the library failed first and the second binary never compiled. Adds scripts/validate-source-bindings.mjs. Each per-plan validator stops at its first mismatch, so a single drifted document hid every later drift, including drift in unrelated plans. Fourteen bindings had drifted undetected. Re-registers zero-result plan source bindings. Seventeen had drifted directly: fourteen undetected, plus apps/console-lab/src/styles.css (two plans) and native/vcg-host/src/installed_catalog.rs (one plan), which this change touches. Because plans bind other plans, that propagated to 63 bindings over 10 passes before reaching a fixed point. All 551 bindings across 59 plans are current. No physical result, threshold, gate, selection, authorization, or qualification field changed. The frozen runtime-payload-scorecard-desk-baseline-v1.json is deliberately untouched: no script reads it, and it is a historical record. Regenerates the browser-measured TV conformance, launcher surface, launcher search, and OCR-A fallback evidence on Windows with Chrome 150.0.7871.187, matching the platform those artifacts declare, and advances the frozen evidence date to 2026-08-02. Verification is desk and WSL2 only. Nothing here is a hardware qualification result. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The repository had no continuous integration. Both defective commits that this branch repairs reached main directly, with no pull request and no automated check. Every job exists because something it checks reached main unnoticed. Linux is the product target (Raspberry Pi 5 and ordinary x86-64 Linux); Windows is the development host. Both are matrixed because defects have been found that are invisible on the other: a clippy lint that only fires where PathBuf is 32 bytes, and a launcher layout collision that only appears under Linux font metrics. - node: install, prepare, a git diff --exit-code gate proving the generators are deterministic and their output committed, typecheck, the full test suite including the new source-binding check, build, and manifest validation. - native: fmt, clippy with -D warnings, tests, and an explicit host doctor step that guards the default-run manifest key. - e2e: real Google Chrome, as playwright.config.ts pins channel chrome, with trace upload on failure. - pi-bringup: runs scripts/pi/bootstrap.sh itself rather than reimplementing its steps, so the operator-facing script cannot rot while its individual commands keep passing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe pull request adds CI coverage, source-binding validation, native packaging refactors, responsive console layout changes, and refreshed benchmark and TV conformance evidence. ChangesRepository verification
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 191-192: Update the CI step named “scripts/pi/bootstrap.sh” to
invoke the script explicitly through bash rather than executing its path
directly, while preserving the existing script path and workflow behavior.
- Around line 24-26: Add a top-level permissions block in the workflow alongside
the global env configuration, setting the GitHub Actions token scope to
read-only for repository contents. Do not grant write access or add job-specific
permissions.
- Around line 51-55: Add shell: bash to the multi-line “Prepare pinned assets
and generated sources” run step, and apply the same shell setting to the native
job’s “Show toolchain” step and the e2e job’s corresponding preparation step so
failures propagate correctly on Windows.
- Around line 59-65: Update the “Generated output is deterministic and
committed” workflow step to use a porcelain Git status check that detects both
modified tracked files and newly generated untracked files. Preserve the
existing error message and nonzero exit behavior when the working tree is not
clean.
- Line 36: Update every actions/checkout step in the workflow, including the
steps near lines 36, 90, 129, and 171, to set persist-credentials to false while
preserving their existing checkout configuration.
In
`@benchmarks/tv-conformance/windows-x64-chrome-150-launcher-search-tv-conformance-v1.json`:
- Around line 621-622: Regenerate the six remote-web screenshots so each is
captured after its corresponding outcome transition in exerciseInteraction(page,
state), including READY and OFFLINE, then update the artifact metadata in
benchmarks/tv-conformance/windows-x64-chrome-150-launcher-search-tv-conformance-v1.json
at lines 621-622 and the validator metadata in
scripts/validate-launcher-search-tv-evidence.mjs at lines 321-329 to match the
regenerated evidence.
In `@native/vcg-host/src/bin/vcg-development-package.rs`:
- Around line 577-579: Replace the positional tuple returned by
write_package_manifest with a named ManifestDigests struct containing manifest,
frontend_sha256, and core_sha256 fields. Update write_signed_catalog to accept
&ManifestDigests and access digests by field name, then adjust callers to pass
the struct while preserving the existing artifact-to-digest mapping.
- Around line 716-738: Update write_signed_descriptor to derive the archive,
release descriptor, and signatures filenames from its generation parameter
instead of hardcoding “1”. Also update stage_release_tree to derive the
release-source directory name from generation, keeping artifact names and
recorded generation consistent for every generation.
In `@native/vcg-host/src/retroarch.rs`:
- Around line 378-393: Update the caller around plan_frontend_artifacts to
construct the session path before invoking it, then pass that session path into
plan_frontend_artifacts instead of passing only request and install_root. Adjust
plan_frontend_artifacts to use the supplied session when building the frontend
artifact location, removing its duplicate runtime_root/profile/game path
construction.
In `@package.json`:
- Line 31: Update the package.json test script to run all test files in grouped
tsx --test and node --test invocations rather than chaining each suite with &&,
so one failure does not prevent later tests from running. Preserve the existing
test file coverage and subsequent validation commands, including
validate:source-bindings, validate:research-evidence, and the recursive test
command.
In `@scripts/validate-source-bindings.mjs`:
- Around line 100-116: Update scripts/validate-source-bindings.mjs at lines
100-116 to add a problems entry whenever a binding’s path or sha256 is not a
string, and to report an unreadable target for every affected plan rather than
only the initial digests cache miss; preserve the cache while ensuring each plan
records its own failure. At lines 73-80, catch JSON.parse failures for files
under benchmarks/ or compliance/ and add a problem instead of silently skipping
the file.
- Around line 190-206: Update the reporting flow after the early-success check
in the validation script so the drift summary and re-registration advice are
emitted only when drift is non-empty. Preserve the existing problems reporting
and exitCode behavior for problems-only runs, avoiding any drift message when
the drift array is empty.
- Around line 54-66: Update the entry sorting in walkJson to use a byte-stable
raw string comparison instead of localeCompare, ensuring identical traversal
order across runtimes while preserving the existing recursive JSON-file
discovery behavior.
- Around line 155-173: Update the rewrite loop around pattern and text.replace
to deduplicate entries by file and entry.target before processing, then use a
global match so every path/sha256 occurrence for that target is replaced. Reset
the global RegExp state before each test call to avoid lastIndex affecting
subsequent matches, while preserving the existing missing-binding error and
rewritten tracking behavior.
- Around line 209-212: Update the entry-point guard around main to convert
process.argv[1] with pathToFileURL and compare its href exactly with
import.meta.url, removing the filename-based endsWith fallback; preserve
invoking main only when this specific module is executed directly.
In `@scripts/validate-source-bindings.test.mjs`:
- Around line 41-56: Export the reregister function and add scratch-repository
coverage for its --write path: create a reviewed plan, modify the bound source,
run reregister, and assert the plan changes only in the digest field while
preserving all other bytes. Exercise the updated registration result so the
regex rewrite and fixed-point behavior are covered without changing unrelated
plan content.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 18958132-943d-4887-b668-004f2daa6b9c
⛔ Files ignored due to path filters (27)
benchmarks/tv-conformance/windows-x64-chrome-150-launcher-home-1080p.pngis excluded by!**/*.pngbenchmarks/tv-conformance/windows-x64-chrome-150-launcher-home-4k.pngis excluded by!**/*.pngbenchmarks/tv-conformance/windows-x64-chrome-150-launcher-home-720p.pngis excluded by!**/*.pngbenchmarks/tv-conformance/windows-x64-chrome-150-launcher-search-destructive-settings-denial-1080p.pngis excluded by!**/*.pngbenchmarks/tv-conformance/windows-x64-chrome-150-launcher-search-destructive-settings-denial-4k.pngis excluded by!**/*.pngbenchmarks/tv-conformance/windows-x64-chrome-150-launcher-search-destructive-settings-denial-720p.pngis excluded by!**/*.pngbenchmarks/tv-conformance/windows-x64-chrome-150-launcher-search-empty-query-scroll-activation-1080p.pngis excluded by!**/*.pngbenchmarks/tv-conformance/windows-x64-chrome-150-launcher-search-empty-query-scroll-activation-4k.pngis excluded by!**/*.pngbenchmarks/tv-conformance/windows-x64-chrome-150-launcher-search-empty-query-scroll-activation-720p.pngis excluded by!**/*.pngbenchmarks/tv-conformance/windows-x64-chrome-150-launcher-search-motion-results-1080p.pngis excluded by!**/*.pngbenchmarks/tv-conformance/windows-x64-chrome-150-launcher-search-motion-results-4k.pngis excluded by!**/*.pngbenchmarks/tv-conformance/windows-x64-chrome-150-launcher-search-motion-results-720p.pngis excluded by!**/*.pngbenchmarks/tv-conformance/windows-x64-chrome-150-launcher-search-no-results-1080p.pngis excluded by!**/*.pngbenchmarks/tv-conformance/windows-x64-chrome-150-launcher-search-no-results-4k.pngis excluded by!**/*.pngbenchmarks/tv-conformance/windows-x64-chrome-150-launcher-search-no-results-720p.pngis excluded by!**/*.pngbenchmarks/tv-conformance/windows-x64-chrome-150-launcher-search-offline-package-activation-1080p.pngis excluded by!**/*.pngbenchmarks/tv-conformance/windows-x64-chrome-150-launcher-search-offline-package-activation-4k.pngis excluded by!**/*.pngbenchmarks/tv-conformance/windows-x64-chrome-150-launcher-search-offline-package-activation-720p.pngis excluded by!**/*.pngbenchmarks/tv-conformance/windows-x64-chrome-150-launcher-search-remote-web-offline-failure-1080p.pngis excluded by!**/*.pngbenchmarks/tv-conformance/windows-x64-chrome-150-launcher-search-remote-web-offline-failure-4k.pngis excluded by!**/*.pngbenchmarks/tv-conformance/windows-x64-chrome-150-launcher-search-remote-web-offline-failure-720p.pngis excluded by!**/*.pngbenchmarks/tv-conformance/windows-x64-chrome-150-launcher-search-remote-web-ready-denial-1080p.pngis excluded by!**/*.pngbenchmarks/tv-conformance/windows-x64-chrome-150-launcher-search-remote-web-ready-denial-4k.pngis excluded by!**/*.pngbenchmarks/tv-conformance/windows-x64-chrome-150-launcher-search-remote-web-ready-denial-720p.pngis excluded by!**/*.pngbenchmarks/tv-conformance/windows-x64-chrome-150-launcher-search-unavailable-package-denial-1080p.pngis excluded by!**/*.pngbenchmarks/tv-conformance/windows-x64-chrome-150-launcher-search-unavailable-package-denial-4k.pngis excluded by!**/*.pngbenchmarks/tv-conformance/windows-x64-chrome-150-launcher-search-unavailable-package-denial-720p.pngis excluded by!**/*.png
📒 Files selected for processing (57)
.github/workflows/ci.ymlapps/console-lab/src/styles.cssbenchmarks/camera-angle/fixed-angle-escalation-evidence-plan-v1.jsonbenchmarks/camera-cabling/cross-tier-camera-cable-plan-v1.jsonbenchmarks/camera-geometry/cross-tier-camera-placement-geometry-plan-v1.jsonbenchmarks/camera-service/camera-replacement-recalibration-plan-v1.jsonbenchmarks/camera-state/physical-shutter-camera-state-experience-plan-v1.jsonbenchmarks/capture-inference-mode/shared-camera-capture-inference-mode-plan-v1.jsonbenchmarks/controller-only-usability/cross-tier-controller-only-usability-plan-v1.jsonbenchmarks/cross-tier-reference/pi5-x86-product-contract-plan-v1.jsonbenchmarks/csi-fallback/pi5-camera-module-3-wide-csi-fallback-plan-v1.jsonbenchmarks/depth-comparison/oak-d-pro-w-rgb-depth-comparison-plan-v1.jsonbenchmarks/depth-interference/cross-device-ir-interference-safety-plan-v1.jsonbenchmarks/enclosure/pi5-integrated-enclosure-reference-build-plan-v1.jsonbenchmarks/failure-critical-substitutes/cross-tier-failure-critical-substitute-plan-v1.jsonbenchmarks/first-party-motion-adaptation/first-party-motion-adaptation-ranking-plan-v1.jsonbenchmarks/font-coverage/ocra-font-structural-evidence-v1.jsonbenchmarks/font-coverage/windows-x64-chrome-150-ocra-platform-fallback-v1.jsonbenchmarks/idle-energy/cross-tier-idle-energy-plan-v1.jsonbenchmarks/kiosk-compositor/cross-tier-kiosk-compositor-plan-v1.jsonbenchmarks/lens-calibration/cross-tier-lens-distortion-rectification-plan-v1.jsonbenchmarks/libretro/supervised-libretro-frontend-qualification-plan-v1.jsonbenchmarks/orbbec/gemini-335l-linux-target-plan-v1.jsonbenchmarks/pi5-hdmi-cec/pi5-hdmi-audio-cec-plan-v1.jsonbenchmarks/realsense/d455-linux-target-plan-v1.jsonbenchmarks/reserved-home/reserved-home-action-plan-v1.jsonbenchmarks/room-survey/living-room-play-zone-plan-v1.jsonbenchmarks/shutdown-reserve/pi5-shutdown-reserve-comparison-plan-v1.jsonbenchmarks/signed-local-package/runtime-neutral-signed-local-package-plan-v1.jsonbenchmarks/steam-input/steamos-steam-input-action-plan-v1.jsonbenchmarks/steam-machine-accountless/steam-machine-accountless-core-plan-v1.jsonbenchmarks/steam-machine-dual-boot/steam-machine-windows-dual-boot-status-v1.jsonbenchmarks/steam-machine-os-comparison/steam-machine-steamos-windows-comparison-plan-v1.jsonbenchmarks/steamos-camera/steamos-uvc-permission-plan-v1.jsonbenchmarks/steamos-content/steamos-update-safe-content-plan-v1.jsonbenchmarks/steamos-shell/steamos-outer-shell-lifecycle-plan-v1.jsonbenchmarks/steamos-workload/steamos-pose-game-workload-plan-v1.jsonbenchmarks/system-economics/complete-system-economics-plan-v1.jsonbenchmarks/tv-appliance/cross-tier-tv-appliance-plan-v1.jsonbenchmarks/tv-conformance/windows-x64-chrome-150-launcher-home-tv-conformance-v1.jsonbenchmarks/tv-conformance/windows-x64-chrome-150-launcher-representative-surfaces-tv-conformance-v1.jsonbenchmarks/tv-conformance/windows-x64-chrome-150-launcher-search-tv-conformance-v1.jsonbenchmarks/tv-conformance/windows-x64-chrome-150-tv-conformance-v1.jsonbenchmarks/tv-visual-tokens/physical-tv-visual-token-plan-v1.jsonbenchmarks/visual-robustness/cross-tier-visual-robustness-plan-v1.jsondocs/IMPLEMENTATION_LOG.mdnative/vcg-host/Cargo.tomlnative/vcg-host/src/bin/vcg-development-package.rsnative/vcg-host/src/installed_catalog.rsnative/vcg-host/src/retroarch.rspackage.jsonscripts/generate-tv-conformance-evidence.mjsscripts/validate-launcher-search-tv-evidence.mjsscripts/validate-launcher-tv-conformance-evidence.mjsscripts/validate-ocra-platform-fallback-evidence.mjsscripts/validate-source-bindings.mjsscripts/validate-source-bindings.test.mjs
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: e2e / windows-latest
- GitHub Check: node / windows-latest
- GitHub Check: node / ubuntu-latest
- GitHub Check: e2e / ubuntu-latest
🧰 Additional context used
🪛 zizmor (1.28.0)
.github/workflows/ci.yml
[warning] 36-36: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 90-92: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 129-129: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 171-171: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 1-193: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[warning] 28-80: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[warning] 82-119: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[warning] 121-165: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[error] 36-36: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 38-38: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 90-90: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 99-99: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 129-129: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 131-131: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 160-160: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 171-171: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 173-173: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 180-180: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🔇 Additional comments (61)
apps/console-lab/src/styles.css (1)
486-491: LGTM!Also applies to: 4900-4916, 4958-4958
benchmarks/font-coverage/ocra-font-structural-evidence-v1.json (1)
728-728: LGTM!Also applies to: 1157-1158, 1373-1373
benchmarks/font-coverage/windows-x64-chrome-150-ocra-platform-fallback-v1.json (1)
70-71: LGTM!Also applies to: 97-105, 123-123
benchmarks/tv-conformance/windows-x64-chrome-150-launcher-home-tv-conformance-v1.json (1)
3-6: LGTM!Also applies to: 17-17, 79-80, 117-118, 155-156, 165-166, 208-208, 220-224, 242-242
benchmarks/tv-conformance/windows-x64-chrome-150-launcher-representative-surfaces-tv-conformance-v1.json (1)
3-9: LGTM!Also applies to: 393-394, 439-439, 451-451, 475-475
benchmarks/tv-conformance/windows-x64-chrome-150-tv-conformance-v1.json (1)
3-6: LGTM!Also applies to: 114-114, 157-157, 206-206
benchmarks/tv-visual-tokens/physical-tv-visual-token-plan-v1.json (1)
35-45: LGTM!scripts/generate-tv-conformance-evidence.mjs (1)
16-16: LGTM!benchmarks/tv-appliance/cross-tier-tv-appliance-plan-v1.json (1)
15-15: LGTM!benchmarks/visual-robustness/cross-tier-visual-robustness-plan-v1.json (1)
55-55: LGTM!scripts/validate-launcher-tv-conformance-evidence.mjs (2)
72-88: 🗄️ Data Integrity & IntegrationFrozen screenshot commitments match the tracked PNG files.
expectedLauncherTvScreenshotsvalidates the recorded byte counts and SHA-256 values correctly.
128-129: 🗄️ Data Integrity & IntegrationNo changes needed.
Both validators and all four relevant tracked evidence artifacts use the same asset names.
scripts/validate-launcher-search-tv-evidence.mjsvalidates hashed asset patterns and does not require fixed names.benchmarks/camera-angle/fixed-angle-escalation-evidence-plan-v1.json (1)
30-30: 🗄️ Data Integrity & IntegrationNo digest updates are required. The repository-wide check reports zero drift, zero missing targets, and 551 validated bindings.
> Likely an incorrect or invalid review comment.benchmarks/camera-cabling/cross-tier-camera-cable-plan-v1.json (1)
35-35: LGTM!benchmarks/camera-geometry/cross-tier-camera-placement-geometry-plan-v1.json (1)
32-32: LGTM!benchmarks/camera-service/camera-replacement-recalibration-plan-v1.json (1)
30-30: LGTM!Also applies to: 45-55
benchmarks/camera-state/physical-shutter-camera-state-experience-plan-v1.json (1)
46-46: LGTM!Also applies to: 61-71
benchmarks/capture-inference-mode/shared-camera-capture-inference-mode-plan-v1.json (1)
22-22: LGTM!benchmarks/orbbec/gemini-335l-linux-target-plan-v1.json (1)
41-41: LGTM!benchmarks/pi5-hdmi-cec/pi5-hdmi-audio-cec-plan-v1.json (1)
26-26: LGTM!benchmarks/realsense/d455-linux-target-plan-v1.json (1)
41-41: LGTM!benchmarks/reserved-home/reserved-home-action-plan-v1.json (1)
45-55: LGTM!benchmarks/room-survey/living-room-play-zone-plan-v1.json (1)
31-31: LGTM!benchmarks/shutdown-reserve/pi5-shutdown-reserve-comparison-plan-v1.json (1)
57-62: LGTM!benchmarks/signed-local-package/runtime-neutral-signed-local-package-plan-v1.json (1)
40-40: LGTM!benchmarks/controller-only-usability/cross-tier-controller-only-usability-plan-v1.json (1)
38-58: LGTM!benchmarks/cross-tier-reference/pi5-x86-product-contract-plan-v1.json (1)
55-55: LGTM!Also applies to: 80-85, 110-110
benchmarks/csi-fallback/pi5-camera-module-3-wide-csi-fallback-plan-v1.json (1)
39-49: LGTM!Also applies to: 69-69
benchmarks/depth-comparison/oak-d-pro-w-rgb-depth-comparison-plan-v1.json (1)
40-45: LGTM!benchmarks/depth-interference/cross-device-ir-interference-safety-plan-v1.json (1)
22-32: LGTM!benchmarks/enclosure/pi5-integrated-enclosure-reference-build-plan-v1.json (1)
33-33: LGTM!Also applies to: 43-48, 63-88
benchmarks/failure-critical-substitutes/cross-tier-failure-critical-substitute-plan-v1.json (1)
35-40: LGTM!Also applies to: 70-75
benchmarks/first-party-motion-adaptation/first-party-motion-adaptation-ranking-plan-v1.json (1)
43-43: LGTM!Also applies to: 73-73
benchmarks/idle-energy/cross-tier-idle-energy-plan-v1.json (1)
25-25: LGTM!benchmarks/kiosk-compositor/cross-tier-kiosk-compositor-plan-v1.json (1)
37-37: LGTM!Also applies to: 47-47
benchmarks/lens-calibration/cross-tier-lens-distortion-rectification-plan-v1.json (1)
40-40: LGTM!benchmarks/libretro/supervised-libretro-frontend-qualification-plan-v1.json (1)
18-18: LGTM!Also applies to: 53-58, 78-78
scripts/validate-source-bindings.mjs (2)
31-52: LGTM!
1-29: LGTM!scripts/validate-source-bindings.test.mjs (3)
1-39: LGTM!
95-128: LGTM!
130-160: LGTM!package.json (1)
113-113: LGTM!docs/IMPLEMENTATION_LOG.md (1)
3-35: LGTM!native/vcg-host/src/bin/vcg-development-package.rs (5)
43-44: LGTM!Also applies to: 53-53, 148-148
470-507: LGTM!
561-572: LGTM!Also applies to: 580-583, 627-630
633-705: LGTM!
844-846: LGTM!native/vcg-host/src/installed_catalog.rs (5)
543-549: LGTM!
664-737: LGTM!
739-762: LGTM!Also applies to: 769-783
785-829: LGTM!
763-768: 🩺 Stability & AvailabilityNo issue found.
validate_relative_filerequires at least oneComponent::Normaland rejects roots, prefixes,.and... Thefile_name()expectis unreachable for parsed catalog paths.> Likely an incorrect or invalid review comment.native/vcg-host/src/retroarch.rs (4)
394-443: LGTM!
445-510: LGTM!
512-531: LGTM!
533-555: LGTM!.github/workflows/ci.yml (2)
158-165: 🩺 Stability & AvailabilityKeep the trace upload path unchanged.
outputDiris../../test-results/console-lab, which resolves totest-results/console-labat the repository root. The upload path is valid.> Likely an incorrect or invalid review comment.
92-112: 🩺 Stability & AvailabilityNo
working-directorychange is required. The repository root contains the Cargo workspace andrust-toolchain.toml.> Likely an incorrect or invalid review comment.native/vcg-host/Cargo.toml (1)
4-7: 📐 Maintainability & Code QualityNo change required.
native/vcg-host/src/main.rsprovides the implicitvcg-hostbinary, sodefault-run = "vcg-host"is valid and resolves the default binary.> Likely an incorrect or invalid review comment.
| env: | ||
| CI: "true" | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Add a top-level least-privilege permissions block.
The workflow declares no permissions, so every job inherits the repository default token scope. No job needs write access. Set read-only at the top level.
🔒 Proposed fix
+permissions:
+ contents: read
+
env:
CI: "true"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| env: | |
| CI: "true" | |
| permissions: | |
| contents: read | |
| env: | |
| CI: "true" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 24 - 26, Add a top-level permissions
block in the workflow alongside the global env configuration, setting the GitHub
Actions token scope to read-only for repository contents. Do not grant write
access or add job-specific permissions.
Source: Linters/SAST tools
| matrix: | ||
| os: [ubuntu-latest, windows-latest] | ||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
Disable credential persistence on every actions/checkout step.
actions/checkout writes the job token into .git/config by default. No job in this workflow performs a git operation that needs the token. Set persist-credentials: false on Lines 36, 90, 129, and 171.
🔒 Proposed fix
- - uses: actions/checkout@v4
+ - uses: actions/checkout@v4
+ with:
+ persist-credentials: false🧰 Tools
🪛 zizmor (1.28.0)
[warning] 36-36: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 36-36: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml at line 36, Update every actions/checkout step in
the workflow, including the steps near lines 36, 90, 129, and 171, to set
persist-credentials to false while preserving their existing checkout
configuration.
Source: Linters/SAST tools
| - name: Prepare pinned assets and generated sources | ||
| run: | | ||
| pnpm prepare:assets | ||
| pnpm prepare:catalog | ||
| pnpm prepare:schemas |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Set shell: bash on multi-line run steps.
On windows-latest, GitHub Actions runs run blocks with PowerShell Core by default. PowerShell Core does not stop the script when a native command returns a non-zero exit code. Only the exit code of the last command reaches the runner. If pnpm prepare:assets fails here, pnpm prepare:schemas still runs and the step reports success.
Add shell: bash to this step. The same applies to the native job "Show toolchain" step (Lines 93-97) and the e2e job "Prepare pinned assets and generated sources" step (Lines 141-145).
🔧 Proposed fix
- name: Prepare pinned assets and generated sources
+ shell: bash
run: |
pnpm prepare:assets
pnpm prepare:catalog
pnpm prepare:schemas📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Prepare pinned assets and generated sources | |
| run: | | |
| pnpm prepare:assets | |
| pnpm prepare:catalog | |
| pnpm prepare:schemas | |
| - name: Prepare pinned assets and generated sources | |
| shell: bash | |
| run: | | |
| pnpm prepare:assets | |
| pnpm prepare:catalog | |
| pnpm prepare:schemas |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 51 - 55, Add shell: bash to the
multi-line “Prepare pinned assets and generated sources” run step, and apply the
same shell setting to the native job’s “Show toolchain” step and the e2e job’s
corresponding preparation step so failures propagate correctly on Windows.
| - name: Generated output is deterministic and committed | ||
| shell: bash | ||
| run: | | ||
| if ! git diff --exit-code; then | ||
| echo "::error::Generated output differs from what is committed. Run the prepare scripts and commit the result." | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
git diff --exit-code does not detect new untracked files.
The step guarantees that generated output is committed. git diff --exit-code only compares tracked files. If a generator emits a new file that was never committed, the tree stays clean by this check and the defect passes. Use a porcelain status check instead.
🔧 Proposed fix
- name: Generated output is deterministic and committed
shell: bash
run: |
- if ! git diff --exit-code; then
+ git add --intent-to-add --all
+ if ! git diff --exit-code; then
echo "::error::Generated output differs from what is committed. Run the prepare scripts and commit the result."
exit 1
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Generated output is deterministic and committed | |
| shell: bash | |
| run: | | |
| if ! git diff --exit-code; then | |
| echo "::error::Generated output differs from what is committed. Run the prepare scripts and commit the result." | |
| exit 1 | |
| fi | |
| - name: Generated output is deterministic and committed | |
| shell: bash | |
| run: | | |
| git add --intent-to-add --all | |
| if ! git diff --exit-code; then | |
| echo "::error::Generated output differs from what is committed. Run the prepare scripts and commit the result." | |
| exit 1 | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 59 - 65, Update the “Generated output
is deterministic and committed” workflow step to use a porcelain Git status
check that detects both modified tracked files and newly generated untracked
files. Preserve the existing error message and nonzero exit behavior when the
working tree is not clean.
| - name: scripts/pi/bootstrap.sh | ||
| run: scripts/pi/bootstrap.sh |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
Invoke the script through bash so the run does not depend on the stored executable bit.
Direct invocation fails with "Permission denied" if scripts/pi/bootstrap.sh is not committed with mode 100755. Windows-based commits frequently drop that bit.
🔧 Proposed fix
- name: scripts/pi/bootstrap.sh
- run: scripts/pi/bootstrap.sh
+ run: bash scripts/pi/bootstrap.sh📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: scripts/pi/bootstrap.sh | |
| run: scripts/pi/bootstrap.sh | |
| - name: scripts/pi/bootstrap.sh | |
| run: bash scripts/pi/bootstrap.sh |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 191 - 192, Update the CI step named
“scripts/pi/bootstrap.sh” to invoke the script explicitly through bash rather
than executing its path directly, while preserving the existing script path and
workflow behavior.
| const target = binding?.path; | ||
| const recorded = binding?.sha256; | ||
| if (typeof target !== "string" || typeof recorded !== "string") continue; | ||
| if (!SHA256.test(recorded)) { | ||
| problems.push(`${plan.relativePath}: sourceBindings[${index}].sha256 is not a SHA-256`); | ||
| continue; | ||
| } | ||
| if (!digests.has(target)) { | ||
| try { | ||
| const bytes = await readFile(resolve(repositoryRoot, target)); | ||
| digests.set(target, normalizedSha256(bytes, target)); | ||
| } catch (error) { | ||
| digests.set(target, null); | ||
| problems.push(`${plan.relativePath}: cannot digest ${target} (${error.message})`); | ||
| } | ||
| } | ||
| const actual = digests.get(target); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
The validator silently drops inputs it cannot interpret. The file header states that the script walks every declared binding in one pass and reports the complete picture, because per-plan validators stop at the first mismatch. Three code paths break that guarantee by discarding an input with no entry in problems, so CI reports success for bindings that were never checked.
scripts/validate-source-bindings.mjs#L100-L116: record a problem whenpathorsha256is not a string, and record a problem for every plan that binds an unreadable target rather than only the first plan that reaches thedigestscache miss.scripts/validate-source-bindings.mjs#L73-L80: record a problem whenJSON.parsefails for a file insidebenchmarks/orcompliance/, instead of skipping the file.
📍 Affects 1 file
scripts/validate-source-bindings.mjs#L100-L116(this comment)scripts/validate-source-bindings.mjs#L73-L80
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/validate-source-bindings.mjs` around lines 100 - 116, Update
scripts/validate-source-bindings.mjs at lines 100-116 to add a problems entry
whenever a binding’s path or sha256 is not a string, and to report an unreadable
target for every affected plan rather than only the initial digests cache miss;
preserve the cache while ensuring each plan records its own failure. At lines
73-80, catch JSON.parse failures for files under benchmarks/ or compliance/ and
add a problem instead of silently skipping the file.
| for (const entry of entries) { | ||
| // Rewrite the digest that sits beside this exact path, leaving all | ||
| // other formatting byte-identical. JSON round-tripping would reflow | ||
| // the document and change bytes no reviewer approved. | ||
| const escaped = entry.target.replaceAll(/[.*+?^${}()|[\]\\]/gu, "\\$&"); | ||
| const pattern = new RegExp( | ||
| `("path"\\s*:\\s*"${escaped}"\\s*,\\s*"sha256"\\s*:\\s*")[0-9a-f]{64}(")`, | ||
| "u", | ||
| ); | ||
| if (!pattern.test(text)) { | ||
| throw new Error( | ||
| `${entry.plan}: could not locate the sha256 beside ${entry.target}`, | ||
| ); | ||
| } | ||
| text = text.replace(pattern, `$1${entry.actual}$2`); | ||
| const key = `${entry.plan} -> ${entry.target}`; | ||
| if (!rewritten.has(key)) rewritten.set(key, entry); | ||
| } | ||
| await writeFile(file, text); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
A path bound twice in one plan makes --write fail to converge.
pattern has no g flag, so String.prototype.replace rewrites only the first "path"/"sha256" pair for that target. If one plan declares the same path under two roles, both drift entries carry the same entry.target and the same entry.actual. The first entry updates occurrence one. The second entry matches occurrence one again, which already holds the new digest, and writes the identical value. Occurrence two stays stale.
The next pass detects the same drift, and the loop repeats until Line 176 throws "source bindings did not converge after 16 passes". The message does not describe the real cause.
Deduplicate the entries per file and target, then replace every occurrence.
🔧 Proposed fix
for (const [file, entries] of byFile) {
let text = await readFile(file, "utf8");
+ const seen = new Set();
for (const entry of entries) {
+ if (seen.has(entry.target)) continue;
+ seen.add(entry.target);
// Rewrite the digest that sits beside this exact path, leaving all
// other formatting byte-identical. JSON round-tripping would reflow
// the document and change bytes no reviewer approved.
const escaped = entry.target.replaceAll(/[.*+?^${}()|[\]\\]/gu, "\\$&");
const pattern = new RegExp(
`("path"\\s*:\\s*"${escaped}"\\s*,\\s*"sha256"\\s*:\\s*")[0-9a-f]{64}(")`,
- "u",
+ "gu",
);
if (!pattern.test(text)) {
throw new Error(
`${entry.plan}: could not locate the sha256 beside ${entry.target}`,
);
}
+ pattern.lastIndex = 0;
text = text.replace(pattern, `$1${entry.actual}$2`);Note: a global RegExp keeps lastIndex between test calls, so the reset above is required.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (const entry of entries) { | |
| // Rewrite the digest that sits beside this exact path, leaving all | |
| // other formatting byte-identical. JSON round-tripping would reflow | |
| // the document and change bytes no reviewer approved. | |
| const escaped = entry.target.replaceAll(/[.*+?^${}()|[\]\\]/gu, "\\$&"); | |
| const pattern = new RegExp( | |
| `("path"\\s*:\\s*"${escaped}"\\s*,\\s*"sha256"\\s*:\\s*")[0-9a-f]{64}(")`, | |
| "u", | |
| ); | |
| if (!pattern.test(text)) { | |
| throw new Error( | |
| `${entry.plan}: could not locate the sha256 beside ${entry.target}`, | |
| ); | |
| } | |
| text = text.replace(pattern, `$1${entry.actual}$2`); | |
| const key = `${entry.plan} -> ${entry.target}`; | |
| if (!rewritten.has(key)) rewritten.set(key, entry); | |
| } | |
| await writeFile(file, text); | |
| for (const [file, entries] of byFile) { | |
| let text = await readFile(file, "utf8"); | |
| const seen = new Set(); | |
| for (const entry of entries) { | |
| if (seen.has(entry.target)) continue; | |
| seen.add(entry.target); | |
| // Rewrite the digest that sits beside this exact path, leaving all | |
| // other formatting byte-identical. JSON round-tripping would reflow | |
| // the document and change bytes no reviewer approved. | |
| const escaped = entry.target.replaceAll(/[.*+?^${}()|[\]\\]/gu, "\\$&"); | |
| const pattern = new RegExp( | |
| `("path"\\s*:\\s*"${escaped}"\\s*,\\s*"sha256"\\s*:\\s*")[0-9a-f]{64}(")`, | |
| "gu", | |
| ); | |
| if (!pattern.test(text)) { | |
| throw new Error( | |
| `${entry.plan}: could not locate the sha256 beside ${entry.target}`, | |
| ); | |
| } | |
| pattern.lastIndex = 0; | |
| text = text.replace(pattern, `$1${entry.actual}$2`); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/validate-source-bindings.mjs` around lines 155 - 173, Update the
rewrite loop around pattern and text.replace to deduplicate entries by file and
entry.target before processing, then use a global match so every path/sha256
occurrence for that target is replaced. Reset the global RegExp state before
each test call to avoid lastIndex affecting subsequent matches, while preserving
the existing missing-binding error and rewritten tracking behavior.
| if (drift.length === 0 && problems.length === 0) { | ||
| const bindings = plans.reduce((total, plan) => total + plan.bindings.length, 0); | ||
| console.log(`${bindings} source bindings across ${plans.length} plans are current.`); | ||
| return; | ||
| } | ||
| for (const entry of drift) { | ||
| console.error( | ||
| `${entry.plan}\n -> ${entry.target}\n recorded ${entry.recorded}\n actual ${entry.actual}`, | ||
| ); | ||
| } | ||
| console.error( | ||
| `\n${drift.length} drifted source binding(s). A drifted binding means a` | ||
| + " pre-registered plan no longer describes the bytes it was reviewed" | ||
| + " against. Re-register deliberately with:\n" | ||
| + " node scripts/validate-source-bindings.mjs --write", | ||
| ); | ||
| process.exitCode = 1; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
A problems-only run prints a misleading "0 drifted source binding(s)" summary.
If problems is non-empty and drift is empty, the early return at Line 193 is skipped, the loop at Line 195 emits nothing, and Line 201 prints "0 drifted source binding(s)" followed by re-registration advice. Re-registration does not repair an unreadable source. The exit code is correct; the text is not.
Print the drift summary only when drift exists.
🔧 Proposed fix
for (const entry of drift) {
console.error(
`${entry.plan}\n -> ${entry.target}\n recorded ${entry.recorded}\n actual ${entry.actual}`,
);
}
- console.error(
- `\n${drift.length} drifted source binding(s). A drifted binding means a`
- + " pre-registered plan no longer describes the bytes it was reviewed"
- + " against. Re-register deliberately with:\n"
- + " node scripts/validate-source-bindings.mjs --write",
- );
+ if (drift.length > 0) {
+ console.error(
+ `\n${drift.length} drifted source binding(s). A drifted binding means a`
+ + " pre-registered plan no longer describes the bytes it was reviewed"
+ + " against. Re-register deliberately with:\n"
+ + " node scripts/validate-source-bindings.mjs --write",
+ );
+ }
+ if (problems.length > 0) {
+ console.error(`\n${problems.length} unreadable or malformed source binding(s).`);
+ }
process.exitCode = 1;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (drift.length === 0 && problems.length === 0) { | |
| const bindings = plans.reduce((total, plan) => total + plan.bindings.length, 0); | |
| console.log(`${bindings} source bindings across ${plans.length} plans are current.`); | |
| return; | |
| } | |
| for (const entry of drift) { | |
| console.error( | |
| `${entry.plan}\n -> ${entry.target}\n recorded ${entry.recorded}\n actual ${entry.actual}`, | |
| ); | |
| } | |
| console.error( | |
| `\n${drift.length} drifted source binding(s). A drifted binding means a` | |
| + " pre-registered plan no longer describes the bytes it was reviewed" | |
| + " against. Re-register deliberately with:\n" | |
| + " node scripts/validate-source-bindings.mjs --write", | |
| ); | |
| process.exitCode = 1; | |
| if (drift.length === 0 && problems.length === 0) { | |
| const bindings = plans.reduce((total, plan) => total + plan.bindings.length, 0); | |
| console.log(`${bindings} source bindings across ${plans.length} plans are current.`); | |
| return; | |
| } | |
| for (const entry of drift) { | |
| console.error( | |
| `${entry.plan}\n -> ${entry.target}\n recorded ${entry.recorded}\n actual ${entry.actual}`, | |
| ); | |
| } | |
| if (drift.length > 0) { | |
| console.error( | |
| `\n${drift.length} drifted source binding(s). A drifted binding means a` | |
| " pre-registered plan no longer describes the bytes it was reviewed" | |
| " against. Re-register deliberately with:\n" | |
| " node scripts/validate-source-bindings.mjs --write", | |
| ); | |
| } | |
| if (problems.length > 0) { | |
| console.error(`\n${problems.length} unreadable or malformed source binding(s).`); | |
| } | |
| process.exitCode = 1; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/validate-source-bindings.mjs` around lines 190 - 206, Update the
reporting flow after the early-success check in the validation script so the
drift summary and re-registration advice are emitted only when drift is
non-empty. Preserve the existing problems reporting and exitCode behavior for
problems-only runs, avoiding any drift message when the drift array is empty.
| if (import.meta.url === `file://${process.argv[1]}`.replace(/\\/gu, "/") | ||
| || process.argv[1]?.endsWith("validate-source-bindings.mjs")) { | ||
| await main(); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Node.js recommended way to detect if an ES module is the main entry point pathToFileURL
💡 Result:
As of August 2026, the recommended way to detect if an ECMAScript module (ESM) is the main entry point in Node.js depends on your target Node.js version. For modern versions of Node.js (v24 and later), you should use the built-in import.meta.main property [1][2]. This property provides a direct, native mechanism to check if the current module is the entry point of the process [3][2]: if (import.meta.main) { // Module is being run directly as the entry point } else { // Module is being imported by another file } If you must support older versions of Node.js, the community-standard approach is to compare the module's URL with the path of the entry file [4][5][6]. Because process.argv[3] returns a filesystem path and import.meta.url is a file URL, you must convert the filesystem path to a file URL for a reliable cross-platform comparison [6][7][8]: import { pathToFileURL } from 'node:url'; if (import.meta.url === pathToFileURL(process.argv[3]).href) { // Module is being run directly as the entry point } Important Considerations: 1. Cross-Platform Reliability: Using pathToFileURL is critical for cross-platform support [7]. Simply prepending file:// to process.argv[3] often fails on Windows due to differences in path separators (backslashes vs. forward slashes) and drive letter formatting [7][8]. 2. Symlinks: If your environment involves symlinks, process.argv[3] might point to the symlink path rather than the resolved file path [6][9]. To handle this more robustly, you can use fs.realpath to resolve the paths before comparison, though this typically requires an asynchronous operation (top-level await) [5][9]: import { fileURLToPath, pathToFileURL } from 'node:url'; import { realpath } from 'node:fs/promises'; const modulePath = await realpath(fileURLToPath(import.meta.url)); if (await realpath(process.argv[3]) === modulePath) { // Module is being run directly (handles symlinks) }
Citations:
- 1: https://stackoverflow.com/questions/57838022/detect-whether-es-module-is-run-from-command-line-in-node
- 2: esm: implement import.meta.main nodejs/node#32223
- 3: https://nodejs.org/api/esm.html
- 4: How to determine whether a file has been run directly inside ES module nodejs/help#2065
- 5: https://www.lukinotes.com/2023/02/detecting-main-module-in-esm-package.html
- 6: https://www.developerload.com/detect-whether-es-module-is-run-from-command-line-in-node
- 7: fix(scripts): use pathToFileURL for ESM entrypoint check on Windows neilsonnn/image-blaster#7
- 8: fix(windows): detect worker ESM main via pathToFileURL thedotmack/claude-mem#3318
- 9: https://mbork.pl/2026-02-09_Node_modules_working_as_command-line_scripts_revisited
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
fd -i 'validate-source-bindings' .
printf '%s\n' '--- package/runtime declarations ---'
for f in package.json .nvmrc .node-version; do
if [ -f "$f" ]; then
echo "### $f"
cat -n "$f" | sed -n '1,180p'
fi
done
printf '%s\n' '--- source outline ---'
ast-grep outline scripts/validate-source-bindings.mjs --match main --view expanded 2>/dev/null || true
printf '%s\n' '--- source guard and imports ---'
cat -n scripts/validate-source-bindings.mjs | sed -n '1,28p;190,220p'
printf '%s\n' '--- related tests and invocations ---'
rg -n --glob '!node_modules' 'validate-source-bindings|import\.meta\.main|pathToFileURL|process\.argv\[1\]' .Repository: Randroids-Dojo/VCG-Console
Length of output: 50382
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- runtime ---'
node --version
printf '%s\n' '--- URL conversion behavior ---'
node --input-type=module - <<'JS'
import { pathToFileURL } from "node:url";
const inputs = [
"scripts/validate-source-bindings.mjs",
"/workspace/scripts/validate-source-bindings.mjs",
"C:\\workspace\\scripts\\validate-source-bindings.mjs",
"\\\\server\\share\\scripts\\validate-source-bindings.mjs",
"/tmp/a file/validate-source-bindings.mjs",
"/tmp/a%20file/validate-source-bindings.mjs",
];
for (const input of inputs) {
console.log(JSON.stringify({ input, href: pathToFileURL(input).href }));
}
JS
printf '%s\n' '--- import.meta.main availability ---'
node --input-type=module - <<'JS'
console.log(JSON.stringify({
node: process.version,
hasImportMetaMain: Object.hasOwn(import.meta, "main"),
importMetaMain: import.meta.main,
}));
JS
printf '%s\n' '--- test module import shape ---'
cat -n scripts/validate-source-bindings.test.mjs | sed -n '1,35p;120,155p'Repository: Randroids-Dojo/VCG-Console
Length of output: 4286
🌐 Web query:
site:nodejs.org/api/esm.html import.meta.main added version Node.js
💡 Result:
The import.meta.main property was added to Node.js in versions v24.2.0 and v22.18.0 [1]. This API allows developers to determine if the current ECMAScript module is the entry point of the process, serving as the equivalent to require.main === module in CommonJS and functioning similarly to Python's name == "main" [1]. [1]
Citations:
Use pathToFileURL for the entry-point guard.
The current comparison fails for Windows drive-letter URLs, relative paths, and paths that require percent-encoding. The endsWith fallback can also run main() when another copy with the same filename is the entry point.
Replace the fallback with an exact URL comparison using pathToFileURL.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/validate-source-bindings.mjs` around lines 209 - 212, Update the
entry-point guard around main to convert process.argv[1] with pathToFileURL and
compare its href exactly with import.meta.url, removing the filename-based
endsWith fallback; preserve invoking main only when this specific module is
executed directly.
| test("accepts a plan whose bindings match their sources", async () => { | ||
| const dir = await scratchRepository(); | ||
| try { | ||
| const body = "# Demo\n\nExact reviewed content.\n"; | ||
| await writeFile(join(dir, "docs", "DEMO.md"), body); | ||
| await writePlan(dir, [ | ||
| { role: "demo-boundary", path: "docs/DEMO.md", sha256: digestOf(body) }, | ||
| ]); | ||
|
|
||
| const { drift, problems } = await findDrift(dir); | ||
| assert.deepEqual(problems, []); | ||
| assert.deepEqual(drift, []); | ||
| } finally { | ||
| await rm(dir, { recursive: true, force: true }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Add coverage for the --write re-registration path.
The tests exercise collectPlans and findDrift only. reregister is untested, and it is the only function that mutates reviewed plan bytes. Its regex rewrite, its fixed-point loop, and its 16-pass ceiling all carry real failure modes.
Export reregister and add a scratch-repository test that drifts a source, runs re-registration, and asserts that the plan bytes change only in the digest field.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/validate-source-bindings.test.mjs` around lines 41 - 56, Export the
reregister function and add scratch-repository coverage for its --write path:
create a reviewed plan, modify the bound source, run reregister, and assert the
plan changes only in the digest field while preserving all other bytes. Exercise
the updated registration result so the regex rewrite and fixed-point behavior
are covered without changing unrelated plan content.
1 participant
The repository had never been exercised on Linux and had no continuous integration. Running the complete pipeline on x86-64 Linux (WSL2, Ubuntu 26.04) found four defects. All of them predated this branch, and both commits that introduced them reached
maindirectly with no pull request and no automated check.Defects repaired
scripts/pi/bootstrap.shaborted at the native host step (exit 101). The crate gained a second binary inc324fe8without declaringdefault-run, making every documentedcargo run -p vcg-host -- ...invocation ambiguous. Eleven call sites acrossREADME.md,native/vcg-host/README.md, anddocs/WINDOWS_QUALIFICATION.mdwere affected; one manifest key repairs all of them. The day-one Raspberry Pi script had never completed.720p launcher overlapped critical text on Linux.
.home-viewgave its heading row a fixed180pxminimum, which overrides the content-derived minimum and lets the row be sized smaller than the text it holds. The defect existed on both platforms — only Linux font metrics wrapped.destination smallto a second line, consuming the slack that hid it. Measured on both: the heading needs 222.31px, Windows gave the row 215.84px (absorbed by padding), Linux gave it 190.66px (visible collision).Seven
clippyfindings under the workspace's opted-inpedanticlints, fixed by refactoring rather than suppression. One,large_enum_variantonInstalledRuntime, was visible only on Windows:PathBufis 32 bytes there against 24 on Linux, pushing the variant gap past the 200-byte threshold. Four more were masked entirely, because the library failed first and the second binary never compiled.Fourteen source bindings had drifted undetected. Each per-plan validator stops at its first mismatch, so one drifted document hid every later drift, including drift in unrelated plans.
Source-binding re-registration
scripts/validate-source-bindings.mjs(new, with tests) walks all 551 bindings in one pass and implements the declaredsourceDigestContractexactly: SHA-256 over strict UTF-8 after CRLF-to-LF normalization, rejecting a BOM and bare carriage returns. It has a deliberate--writere-registration mode that iterates to a fixed point.Seventeen bindings had drifted directly: fourteen undetected, plus
styles.css(two plans) andinstalled_catalog.rs(one plan), which this branch touches. Because plans bind other plans, re-registering propagated to 63 bindings over 10 passes. No physical result, threshold, gate, selection, authorization, or qualification field changed.runtime-payload-scorecard-desk-baseline-v1.jsonis deliberately left untouched: no script reads it, so its stale pins are a historical record that "fixing" would falsify.Browser-measured TV conformance, launcher surface, launcher search, and OCR-A fallback evidence were regenerated on Windows with Chrome 150.0.7871.187 — matching the platform those artifacts declare — with the frozen evidence date advanced to 2026-08-02.
Continuous integration
Four jobs, Linux and Windows matrixed, because this work found defects invisible on each.
nodegit diff --exit-codedeterminism gate, typecheck, full suite incl. source bindings, build, manifestsnative-D warnings, tests, and an explicitdoctorstep guardingdefault-rune2eplaywright.config.tspinschannel: "chrome"), traces uploaded on failurepi-bringupscripts/pi/bootstrap.shitself, so the operator script cannot rot while its individual commands passVerification
Every job's commands were run locally on both platforms before this PR.
pnpm testcargo fmt/clippy -D warnings/cargo testcargo run -p vcg-host -- doctorpnpm test:e2escripts/pi/bootstrap.shvalidate:source-bindingsLimits
Verification is desk and WSL2 only. Nothing here is a hardware qualification result. A Logitech C920 was enumerated under V4L2 through usbip and produced real frames, but its frame rate over that transport is a property of USB-over-TCP, not of the camera, Linux, or the Raspberry Pi.
The workflows have never executed on GitHub. This PR is their first run, and runner-specific details such as rustup toolchain provisioning may need adjustment.
🤖 Generated with Claude Code