Repository navigation
Conversation
There was a problem hiding this comment.
Review summary
Solid, well-tested PR. It extracts shared Unix dependency setup into scripts/install-dependencies.sh, adds a standalone scripts/install.sh entrypoint, and introduces CA-preservation logic with extensive unit + integration tests. Two genuine improvements stand out: replacing the global-trust apt-key add with a scoped signed-by= keyring, and the carefully-scoped CA preservation (fingerprint-matched, follows symlinks like the real updater, refuses non-CA / managed / disabled certs, never disables TLS). Path-with-spaces handling and errexit-in-conditional pitfalls are tested throughout.
Verified the new __tests__/activate.test.ts mock counts are consistent with src/install.ts: non-Windows source builds call llvm-config 3× then go build (asserted as the 4th call, 5 mocks total), Windows calls go build first (2 mocks). No inconsistency.
Inline findings below. None are blockers.
Minor items not inlined:
scripts/install-dependencies.sh:40(curl ... llvm-snapshot.gpg.key): no--retry, unlike other network paths in the project; a flaky runner fails on the first attempt. Consider--retry 3 --retry-connrefused.scripts/install-dependencies.sh(macOS branch,brew updateguard): the "Preserve runner-image formulae on Intel" comment sits directly above thearm64-only refresh, which can momentarily read as applying to the arm64 branch.THIRD_PARTY_NOTICES.mdcredits "Changjun Ji" whilescripts/preserve-extra-ca.shcredits "CarlJi" — same person (correct), but inconsistent form across sibling files.
There was a problem hiding this comment.
Review summary
Solid, well-tested PR. It extracts shared Unix dependency setup into scripts/install-dependencies.sh, adds a standalone scripts/install.sh entrypoint, and introduces CA-preservation logic with extensive unit + integration tests. Two genuine improvements stand out: replacing the global-trust apt-key add with a scoped signed-by= keyring, and the carefully-scoped CA preservation (fingerprint-matched, follows symlinks like the real updater, refuses non-CA / managed / disabled certs, never disables TLS). Path-with-spaces handling and errexit-in-conditional pitfalls are tested throughout.
Verified the new __tests__/activate.test.ts mock counts are consistent with src/install.ts: non-Windows source builds call llvm-config 3× then go build (asserted as the 4th call, 5 mocks total), Windows calls go build first (2 mocks). No inconsistency.
Inline findings below. None are blockers.
(Note: this review re-submits three inline findings whose titles were previously too long; it supersedes the earlier review on this PR.)
Minor items not inlined:
scripts/install-dependencies.sh:40(curl ... llvm-snapshot.gpg.key): no--retry, unlike other network paths in the project; a flaky runner fails on the first attempt. Consider--retry 3 --retry-connrefused.scripts/install-dependencies.sh(macOS branch,brew updateguard): the "Preserve runner-image formulae on Intel" comment sits directly above thearm64-only refresh, which can momentarily read as applying to the arm64 branch.THIRD_PARTY_NOTICES.mdcredits "Changjun Ji" whilescripts/preserve-extra-ca.shcredits "CarlJi" — same person (correct), but inconsistent form across sibling files.
Problem
llcppg replaced setup-llgo in #917 to provide an actionable local/agent installer, not because the action's Go selection was broken. Its #940 addresses a second issue: apt can rebuild the system CA bundle and drop proxy CAs that were appended only to that bundle.
Changes
Use the same LLGo ref resolver, checksum-verified release installer, source builder, and Unix dependencies in CI and locally:
The local entrypoint requires Node.js 20+ and an existing Go 1.21+ launcher. It owns unique installation directories, emits a safely quoted activation file, and does not reset user checkouts or edit profiles. Windows setup and the existing source-build command are unchanged. CA preservation adapts #940 with attribution, fixes symlink traversal, and never introduces a project-supplied CA.
Validation
66efbc5in CI and AGENTS.md. All four downstream jobs passed: action/standalone installation on Linux/macOS, full llcppg coverage tests and post-install HTTPS/Git checks. Standalone jobs start from Go 1.24 and activate the real Go 1.27.0 binary.