Skip to content

feat(install): add observable release proxy - #969

Open
benvinegar wants to merge 2 commits into
mainfrom
feat/release-proxy-analytics
Open

feat(install): add observable release proxy#969
benvinegar wants to merge 2 commits into
mainfrom
feat/release-proxy-analytics

Conversation

@benvinegar

Copy link
Copy Markdown
Member

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

  • add a stateless Cloudflare Worker at updates.hunk.dev/v1/curl/latest that caches and normalizes GitHub's latest stable release
  • log only allowlisted request source and current-version dimensions, without a database or stable installation identifier
  • route curl installer, startup, and explicit update checks through the Worker with bounded direct-GitHub fallback
  • honor HUNK_DISABLE_ANALYTICS=1 and DO_NOT_TRACK=1
  • add a separate GitHub Actions workflow that checks PRs and deploys Worker changes from main
  • keep release archives on GitHub and document Cloudflare's infrastructure-level logging caveat

Non-goals

  • no archive proxying
  • no install-success beacon
  • no persistent client identity
  • no analytics for npm or Homebrew release checks in this change

Verification

  • npm ci && npm test && npm run typecheck && npx wrangler deploy --dry-run in workers/release-proxy
  • bun 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.ts
  • sh -n install.sh
  • bun run typecheck
  • bun run deps:check
  • bun run lint
  • bun run changeset:status

The 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_TOKEN in the protected release-proxy GitHub 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

@vercel

vercel Bot commented Sep 1, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
hunk-web Ignored Ignored Preview Sep 1, 2026 9:04pm UTC

Request Review

@socket-security

socket-security Bot commented Sep 1, 2026

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedwrangler@​4.128.0981009296100

View full report

@greptile-apps

greptile-apps Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The 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.

  • Adds the cached Worker endpoint, deployment workflow, tests, and operational documentation.
  • Adds proxy-first release discovery to the shell installer, startup notices, and explicit self-update checks.
  • Sends allowlisted request-source and current-version dimensions while preserving direct GitHub archive downloads.

Confidence Score: 4/5

The 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

Filename Overview
workers/release-proxy/src/index.ts Adds a bounded, cached release-normalization endpoint with allowlisted analytics dimensions and non-cacheable failures.
src/core/install/latestRelease.ts Adds proxy-first curl release lookup and fallback, but introduces a direct process.env access contrary to the repository environment convention.
install.sh Adds opt-out-aware proxy resolution, validated response parsing, bounded downloader attempts, and direct-GitHub fallback.
.github/workflows/release-proxy.yml Adds lockfile-based Worker checks and protected main-branch deployment.
workers/release-proxy/src/index.test.ts Covers normalization, caching, malformed upstream responses, timeouts, routing, and analytics allowlisting.

Sequence Diagram

sequenceDiagram
    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
Loading
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

Comment thread src/core/install/latestRelease.ts Outdated
}

/** Return whether release analytics are disabled by either supported environment convention. */
function releaseAnalyticsDisabled(env: NodeJS.ProcessEnv = process.env) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant