feat(install): add observable release proxy - #969
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Greptile SummaryThe PR adds a privacy-bounded Cloudflare release proxy and routes curl-managed installation and update checks through it with analytics opt-outs and direct-GitHub fallback.
Confidence Score: 4/5The PR appears safe to merge, with one non-blocking configuration-boundary cleanup in the release lookup helper. The proxy, fallback, timeout, caching, and analytics paths are covered without a concrete blocking failure; the remaining issue is an implicit direct process.env dependency. Files Needing Attention: src/core/install/latestRelease.ts Important Files Changed
Sequence DiagramsequenceDiagram
participant C as Curl-managed client
participant P as updates.hunk.dev Worker
participant G as GitHub Releases
C->>P: GET latest + bounded metadata
alt Cached stable release
P-->>C: "{version}"
else Cache miss
P->>G: Fetch latest release
G-->>P: GitHub release metadata
P-->>C: "Normalized {version}"
else Opt-out or proxy failure
C->>G: Fetch latest release directly
G-->>C: GitHub release metadata
end
C->>G: Download selected release archive
Prompt To Fix All With AI### Issue 1
src/core/install/latestRelease.ts:128
**Avoid implicit environment access**
Defaulting `releaseAnalyticsDisabled` to `process.env` introduces an unvalidated configuration dependency when callers omit `deps.env`, making release-lookup behavior harder to test and reason about consistently. Pass the environment explicitly from the application boundary instead.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "feat(install): add observable release pr..." | Re-trigger Greptile |
| } | ||
|
|
||
| /** Return whether release analytics are disabled by either supported environment convention. */ | ||
| function releaseAnalyticsDisabled(env: NodeJS.ProcessEnv = process.env) { |
There was a problem hiding this comment.
Avoid implicit environment access
Defaulting releaseAnalyticsDisabled to process.env introduces an unvalidated configuration dependency when callers omit deps.env, making release-lookup behavior harder to test and reason about consistently. Pass the environment explicitly from the application boundary instead.
Context Used: guidelines.mdc Cursor rule (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/core/install/latestRelease.ts
Line: 128
Comment:
**Avoid implicit environment access**
Defaulting `releaseAnalyticsDisabled` to `process.env` introduces an unvalidated configuration dependency when callers omit `deps.env`, making release-lookup behavior harder to test and reason about consistently. Pass the environment explicitly from the application boundary instead.
**Context Used:** guidelines.mdc Cursor rule ([source](https://github.com/modem-dev/modem/blob/main/.cursor/rules/guidelines.mdc))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Removed the implicit process.env fallback. Application boundaries already pass their environment explicitly; lower-level release lookup now only evaluates the injected environment. Focused tests, typecheck, lint, and the TTY smoke suite pass.
Responded by Pi using gpt-5.6-sol.
This comment was generated by Pi using gpt-5.6-sol
Problem
Hunk's curl installer and curl-managed update checks ask GitHub directly for the latest release. That traffic cannot provide Hunk-owned aggregate release-check analytics, and adding a separate telemetry request would create another network path.
Approach
updates.hunk.dev/v1/curl/latestthat caches and normalizes GitHub's latest stable releaseHUNK_DISABLE_ANALYTICS=1andDO_NOT_TRACK=1mainNon-goals
Verification
npm ci && npm test && npm run typecheck && npx wrangler deploy --dry-runinworkers/release-proxybun test src/core/install/latestRelease.test.ts src/core/process/updateNotice.test.ts src/core/install/selfUpdate.test.ts scripts/install-sh.test.ts test/cli/install-vm/prepare-fixtures.test.tssh -n install.shbun run typecheckbun run deps:checkbun run lintbun run changeset:statusThe implementation received an independent subagent review. Follow-up findings around complete response-body timeouts, wget retry bounds, error caching, analytics allowlists, workflow ref safety, and clean Worker dependency installation were fixed; the final review approved the diff with no high- or medium-severity findings.
Deployment
No live deployment was performed. The repository must configure
CLOUDFLARE_API_TOKENin the protectedrelease-proxyGitHub environment. The client and installer fall back directly to GitHub, so rollout ordering does not affect update availability.This PR description was generated by Pi using gpt-5.6-sol