[mstomar125]

Closes #3320

An app crate built on core can now decide where the update check looks and what the update banner's Download button opens, through AppExtensions::update_source(UpdateSource) (ADR-082 (proposed) §3.6). With AppExtensions::none() every build behaves exactly as before.

What changed

app-lib/src/update_check.rs

• New public types UpdateSource { latest: LatestVersionSource, download_url: Option<&'static str> } and LatestVersionSource::{GitHubLatestRelease { repo }, VersionEndpoint { url }} (Clone, Debug, PartialEq, Eq, with rustdoc), plus UpdateSource::community() (const fn).
• builtin_update_source_for(is_pro) holds today's behaviour in one place: false is community() (the core repo's GitHub latest release, downloaded from the releases page), true is the in-core version endpoint with no download URL, verbatim from Carry the download URL in the update-check payload instead of a hardcoded releases page #3319.
• UpdateSourceState (managed state), update_source_for_app(app) (the managed source, or the built-in one when none is managed), and check_for_update_for_app(app), which reads the version from app.package_info(), fetches from the source, and calls status_from(current, latest, source.download_url).
• Generic fetchers: github_latest_release_url(repo), fetch_github_latest_release(repo), fetch_version_endpoint(url), version_from_endpoint_json(body) (renamed from the edition-specific parser) and fetch_latest_version(&LatestVersionSource).
• Removed check_for_update, LATEST_RELEASE_URL, fetch_latest_tag, fetch_pro_latest_version and the old parser. check_for_update_command delegates to check_for_update_for_app. The module doc names the update source generically.

app-lib/src/extensions/mod.rs

• AppExtensions::update_source(source); the last call wins. Its rustdoc states the contract: the source is fixed when the app is built, the running version comes from the bundle config, a VersionEndpoint must answer {"version": "<semver or v-tag>"}, and download_url: None hides Download.
• assemble manages UpdateSourceState(extension's source, or builtin_update_source()), as its own statement before the existing ExtensionHooks manage, so the PRs that add more extension points after this one don't collide with it.
• UpdateSource and LatestVersionSource are re-exported from extensions and from the crate root.

app-lib/src/lib.rs: the startup spawn calls update_check::check_for_update_for_app(&update_app_handle); the current_version local is gone.

Deviations from the issue

• The constant is RELEASES_PAGE_URL, not COMMUNITY_RELEASES_PAGE_URL. That's the name Carry the download URL in the update-check payload instead of a hardcoded releases page #3319 actually merged with, so community() uses it. Its repo is the new constant CORE_REPO.
• assemble doesn't read the edition itself. The issue has assemble call builtin_update_source_for(is_pro_build()). Instead, a no-argument builtin_update_source() in update_check.rs makes that call, and both assemble and the unmanaged fallback in update_source_for_app use it. This keeps the extensions module free of edition reads, and leaves the builtin_update_source_for argument as the file's only is_pro_build() read, as the acceptance criterion asks.
• BASELINES are not lowered. The issue's step 5 says to lower them; the standing instruction for this drain is not to, because parallel PRs collide on those lines and Drive every Pro-boundary ratchet count to zero and make the check a hard ban #3371 tightens them all at once.
• Extra tests beyond the issue's list: builtin_source_with_the_edition_flag_is_the_in_core_endpoint_without_a_download (pins acceptance criterion 2; Delete the Pro arm of core's built-in update source and its worker parsing #3323 deletes it with the true arm, and it won't compile once the argument goes), update_source_falls_back_to_the_builtin_without_extension_state, and an_extension_update_source_decides_the_version_and_the_download. The last one drives check_for_update_for_app end to end against a one-shot HTTP server on 127.0.0.1, so it never touches the internet.
• builtin_source_is_community_outside_pro_builds is named builtin_source_without_the_edition_flag_is_community, so new code carries no product name, as the issue's last acceptance criterion asks. The new name also mirrors its sibling test.
• The source-text test covers both callers from lib.rs. both_update_checks_read_the_app_update_source pins the startup spawn in run's source and check_for_update_command's body in update_check.rs. The command takes a concrete tauri::AppHandle (Wry) and can't run on the mock runtime.

Pro-boundary ratchet

bun run scripts/check-pro-boundary.ts passes. BASELINES are untouched.

marker	before	after	baseline
proCommands	271	261	275
editionBranching	96	98	113
cloudWording	50	49	72
proWording	243	238	281

Every other marker is unchanged.

editionBranching rises by 2 (96 → 98), class 3: a planned move of Pro code already in core that adds no behaviour. The in-core edition branch moves out of check_for_update (one hit removed) into builtin_update_source_for, which the issue specifies with an is_pro: bool parameter. The added lines:

update_check.rs:117: builtin_update_source_for(crate::daemon_setup::is_pro_build())
update_check.rs:123: pub(crate) fn builtin_update_source_for(is_pro: bool) -> UpdateSource {
update_check.rs:124: if is_pro {

#3323 deletes the true arm and the parameter, which removes all three. The count stays below its baseline, so nothing is raised.

Tests

Run locally:

• cargo test --lib -p nodespace-app-lib -- update_check extensions:: run_wiring --test-threads=2: 50 passed.
• bun run test:changed (rust and scripts tiers): passed; the app-lib unit binary passed 290 of 290.
• cargo clippy -p nodespace-app-lib --all-targets -- -D warnings -D clippy::unused-async: clean.
• NODESPACE_PRO_SUPABASE_URL=https://example.invalid NODESPACE_PRO_ANON_KEY=dummy cargo check -p nodespace-app: passes.
• bun run scripts/check-pro-boundary.ts: passes (see above).

New and renamed tests. Each was mutation-checked: I broke the code it covers, confirmed it fails, then restored.

test	mutation that fails it
update_check::community_source_targets_the_public_release_endpoint	github_latest_release_url drops /latest
update_check::community_source_downloads_from_the_releases_page	community() has download_url: None
update_check::builtin_source_without_the_edition_flag_is_community	edition arms swapped
update_check::builtin_source_with_the_edition_flag_is_the_in_core_endpoint_without_a_download	edition arms swapped
update_check::version_endpoint_body_parsed (renamed)	parser reads tag_name instead of version
update_check::version_endpoint_error_body_yields_none (renamed)	parser keeps an empty version
extensions::fixture_tests::none_manages_the_builtin_update_source	assemble doesn't manage the source
extensions::fixture_tests::an_extension_update_source_replaces_the_builtin	update_source() ignores its argument
extensions::fixture_tests::the_last_update_source_wins	first call wins (get_or_insert)
extensions::fixture_tests::update_source_falls_back_to_the_builtin_without_extension_state	unmanaged fallback returns another source
extensions::fixture_tests::an_extension_update_source_decides_the_version_and_the_download	check_for_update_for_app passes None as the download URL
run_wiring_tests::both_update_checks_read_the_app_update_source	startup spawn stops calling check_for_update_for_app (fails on the startup assertion); the command builds its status from the built-in source (fails on the command assertion)

The existing update-check tests, including #3319's, pass unchanged.

Note for #3323

After this PR, #3323 deletes only PRO_LATEST_URL and its doc, the true arm and is_pro parameter of builtin_update_source_for, and the test pinning that arm. The version-endpoint parser and fetcher are now the generic VersionEndpoint helpers, and they stay.

🤖 Generated with Claude Code

[mstomar125]

Pragmatic code review (round 1)

Review Type: Initial Review

Reviewed at c22582bb against origin/main. I read the code and didn't rebuild, because the machine is shared. I did re-run bun run scripts/check-pro-boundary.ts (and --changed). gh pr checks reports no CI checks on this branch.

Requirements Check:

• ✅ AppExtensions::none() on a community build checks https://api.github.com/repos/NodeSpaceAI/nodespace-core/releases/latest, and Download opens the releases page. UpdateSource::community() is GitHubLatestRelease { repo: CORE_REPO } with download_url: Some(RELEASES_PAGE_URL). community_source_targets_the_public_release_endpoint pins the full URL literal and community_source_downloads_from_the_releases_page pins the download URL. The constant is named RELEASES_PAGE_URL, not the issue's COMMUNITY_RELEASES_PAGE_URL, because that's the name Carry the download URL in the update-check payload instead of a hardcoded releases page #3319 merged with. That deviation is correct.
• ✅ A Pro-mode build still checks the in-core endpoint with no Download URL. builtin_update_source_for(true) is verbatim VersionEndpoint { url: PRO_LATEST_URL } with download_url: None, and a dedicated test pins it. The is_pro_build() → true-arm wiring can't be exercised at runtime in a community test binary; see finding 4.
• ✅ An extension's UpdateSource decides the version source and the Download URL, for both the startup event and check_for_update_command.
  • Both callers now go through check_for_update_for_app → update_source_for_app → managed UpdateSourceState.
  • The fixture tests pin replacement and last-wins, and run an end-to-end check against a local endpoint.
  • both_update_checks_read_the_app_update_source pins both call sites.
• ✅ check_for_update is gone, and the file's only is_pro_build() read is the argument to builtin_update_source_for.
  • git grep -w check_for_update finds nothing anywhere.
  • update_check.rs:117 is the only is_pro_build() read, and extensions/ has none.
  • The issue had assemble call builtin_update_source_for(is_pro_build()). The PR moves that call into a no-argument builtin_update_source() instead. This is the better design: the extensions module stays free of edition reads, and assemble and the unmanaged fallback share one default.
• ✅ Every existing update-check test passes, and the two parser tests pass under their new names.
  • All of Carry the download URL in the update-check payload instead of a hardcoded releases page #3319's tests (status_from, serialization) are byte-identical.
  • version_endpoint_body_parsed and version_endpoint_error_body_yields_none exist with product-neutral fixture bodies.
  • The pass counts come from the author's run (50 targeted, 290/290 unit); I didn't re-run them.
• ✅ check-pro-boundary.ts passes, with one authorized deviation: BASELINES are not lowered.
  • My re-run matches the PR table: proCommands 261, editionBranching 98, cloudWording 49, proWording 238. The check passes with no count above baseline.
  • The +2 on editionBranching is the issue-specified is_pro: bool parameter. It's declared as class 3 with the three added lines quoted and Delete the Pro arm of core's built-in update source and its worker parsing #3323 named as the deleting issue, as CLAUDE.md requires.
  • Leaving BASELINES alone follows the standing drain instruction.
• ✅ The Pro-mode compile check passes. This is per the author's cargo check -p nodespace-app with the edition env vars; I didn't re-run it.
• ✅ No product names or issue numbers in new code or docs.
  • The edition-word hits in added lines are is_pro_build(), is_pro, PRO_LATEST_URL and one test name. The issue mandates every one of them, and Delete the Pro arm of core's built-in update source and its worker parsing #3323 deletes them.
  • There are no #NNNN references in code comments and no #[allow(dead_code)].

Code Review Findings:

What's good:

• The source has its own managed type, owned by update_check. UpdateSourceState lives in update_check instead of growing ExtensionHooks. That keeps the type next to its only reader, and it means the queued AppExtensions additions (Have the app library add core's window capability at startup #3308, Let an app built on core supply its DaemonProfile via AppExtensions::daemon_profile #3347) won't collide on one struct.
• A response can't change where Download goes. The Download URL is never taken from the network: status_from uses the source's static download_url. So a compromised or non-TLS version endpoint can at worst show a false banner. It can't point Download somewhere else.
• Timeouts and async safety are intact. Both fetchers still use the 5 s REQUEST_TIMEOUT. No State guard is held across an .await: update_source_for_app clones and drops it.
• The docs carry the contract. The rustdoc on AppExtensions::update_source covers all four points the issue asked for:
  • the source is fixed for the process;
  • the running version comes from the bundle config;
  • a VersionEndpoint answers {"version": …};
  • download_url: None hides Download.

1. 🟢 Suggestion: the docs say the default is always UpdateSource::community, which isn't true in an edition-flag build until Delete the Pro arm of core's built-in update source and its worker parsing #3323.

   • 📁 packages/desktop-app/app-lib/src/update_check.rs:15-16, :64-66; packages/desktop-app/app-lib/src/extensions/mod.rs:251-254
   • Three docs say that an app supplying no source "uses the built-in source, [UpdateSource::community]". In a build with the edition flag the built-in source is the version endpoint.
   • A product-neutral qualifier keeps the docs accurate in both builds. For example: "core's built-in source ([UpdateSource::community] in a default build)". The module doc could say: "In a default build, the built-in source reads…".
   • Principle: rustdoc is the extension contract, and it should be accurate in every build.
   • Optional. This is transitional, and Delete the Pro arm of core's built-in update source and its worker parsing #3323 makes the current text true.

2. 🟢 Suggestion: say in the docs that download_url must be an http(s) URL.

   • 📁 packages/desktop-app/app-lib/src/update_check.rs:74-76; packages/desktop-app/app-lib/src/extensions/mod.rs:268-270
   • The banner opens the URL through openUrl under opener:default. Its scope (tauri-plugin-opener 2.5.4 allow-default-urls) allows only http://*, https://*, mailto:* and tel:*.
   • An app crate that supplies, say, an app-store or custom-scheme URL gets a Download button that does nothing when clicked. The only sign is a log.warn in update-status.svelte.ts:117-125.
   • One sentence in the field doc closes this hole in the contract. For example: "An http(s) URL; the default opener scope refuses other schemes."
   • Principle: state the constraints of an extension API where the extension author will read them.

3. 🟢 Suggestion: consider #[non_exhaustive] on LatestVersionSource.

   • 📁 packages/desktop-app/app-lib/src/update_check.rs:84-100
   • EXTENSION_API_VERSION's doc says an additive change bumps the minor version. But adding a variant to an exhaustive pub enum breaks any app crate that matches on it.
   • A future source kind, such as a signed manifest, is plausible. With an enum-level #[non_exhaustive], app crates can still construct every variant, and a new variant becomes a minor change.
   • Leave UpdateSource as it is: marking the struct would block the struct-literal construction the API relies on.
   • Principle: forward-compatible public API.

4. 🟢 Informational: AC2's runtime wiring is covered by reading the code, not by a test.

   • 📁 packages/desktop-app/app-lib/src/update_check.rs:116-118; packages/desktop-app/app-lib/src/extensions/mod.rs:343-347
   • is_pro_build() is an option_env! compile-time constant, so test binaries are always community builds. Two mutations would therefore pass every test:
     • builtin_update_source() → builtin_update_source_for(false);
     • assemble → .unwrap_or_else(UpdateSource::community).
   • The Pro-mode cargo check catches neither.
   • This isn't a regression: the old if is_pro_build() branch in check_for_update was untested too. Delete the Pro arm of core's built-in update source and its worker parsing #3323 deletes this code soon, so no action is needed. A one-line source-text pin on builtin_update_source_for(crate::daemon_setup::is_pro_build()) is the cheap option if you want it.

5. 🟢 Nit: the localhost end-to-end test is low flake risk; a clearer failure message would help.

   • 📁 packages/desktop-app/app-lib/src/extensions/fixture_tests.rs:652-704
   • I checked the risks concretely:
     • Connection reset: none. The server reads the whole request head before replying, so it closes with no unread bytes and sends a FIN, not a RST. It also sends connection: close and content-length.
     • One-shot server: sufficient. reqwest makes a single HTTP/1.1 connection to an http:// URL.
     • Hang: impossible. PROMPTLY (10 s) exceeds REQUEST_TIMEOUT (5 s), so a failure is an assertion, not a hang.
     • Port binding: 127.0.0.1:0 matches seven existing bind sites in the repo's tests, including packages/agent/.../model_manager.rs, which also points reqwest 0.12 at a local listener.
   • Proxies are the one environmental dependency. Only nodespace-agent and nodespace-app-lib depend on reqwest 0.12.28, both with default-features = false, so system-proxy is off and macOS proxy settings are ignored.
   • However, hyper-util 0.1.20's matcher honors HTTP_PROXY and ALL_PROXY with no implicit loopback bypass. On a machine with one of those set and NO_PROXY not covering 127.0.0.1, the test fails every time, showing only left: None. That's the same exposure the agent tests already have.
   • Optional: add a message to the status.latest assertion, e.g. "no answer from the local endpoint (is HTTP_PROXY set without NO_PROXY=127.0.0.1?)".

6. 🟢 Nit: a misconfigured source fails silently.

   • 📁 packages/desktop-app/app-lib/src/update_check.rs:237-310
   • A typo in an extension's repo or url, such as a missing scheme, means "no update known" forever, with nothing logged. Swallowing errors is the right behaviour for users; this is about diagnosis.
   • A tracing::debug! on the fetchers' error paths, or one warn when a VersionEndpoint URL doesn't parse, would make extension misconfiguration findable.
   • This behaviour predates the PR, but an extension point makes it more likely to matter. Optional.

7. 🟢 Nit: small wording and consistency fixes.

   • 📁 packages/desktop-app/app-lib/src/update_check.rs:15, :424
   • Module-doc line 15 is 97 characters, while the paragraph around it wraps at about 80.
   • The issue-mandated test name builtin_source_is_community_outside_pro_builds could mirror its sibling, builtin_source_with_the_edition_flag_…. For example: builtin_source_without_the_edition_flag_is_community. Purely cosmetic, and Delete the Pro arm of core's built-in update source and its worker parsing #3323 rewrites both tests anyway.

Recommendation: APPROVE

No blockers or important issues. The design follows the existing extension-point pattern and keeps the extensions module free of edition reads. Both callers converge on one source-reading path, and every new test was mutation-checked. All seven findings are optional polish.

[mstomar125]

Automated pragmatic-code-review: APPROVE (posted as a comment — GitHub blocks self-approval). Full findings in the review comment above.

[mstomar125]

Address review (round 1)

Review verdict: APPROVE, with 7 optional findings and no blockers. Addressed in 30a34da7.

✅ Addressed (5)

1. Built-in source docs. The module doc, UpdateSource's doc and AppExtensions::update_source now say the built-in source is UpdateSource::community in a default build, so they're accurate in an edition-flag build too.
2. download_url scheme. UpdateSource::download_url and AppExtensions::update_source now say the URL must be http or https: the banner opens it through the opener plugin, whose default scope refuses other schemes.
3. E2E failure message. The status.latest assertion in an_extension_update_source_decides_the_version_and_the_download now names HTTP_PROXY/ALL_PROXY and NO_PROXY.
4. Silent misconfiguration. check_for_update_for_app logs once at debug level, with the source, when it finds no latest version. That's enough to diagnose a mistyped extension source; the fetchers still swallow errors, as before.
5. Wording.
   • The 97-character module-doc line is rewrapped.
   • builtin_source_is_community_outside_pro_builds is renamed builtin_source_without_the_edition_flag_is_community. It now mirrors its sibling, and new code carries no product name (the issue's last acceptance criterion).

⏭️ Skipped (2)
3. #[non_exhaustive] on LatestVersionSource.

• The extension API's other public data types (DaemonReady, with public fields) don't use it either.
• The project is pre-release with a no-compatibility-shims policy, and EXTENSION_API_VERSION's bump rule already covers adding a variant.
• Revisit if the extension API is frozen for external app crates.

4. Source-text pin on the is_pro_build() wiring.
   • A pin would put an is_pro_build needle in this file, adding an editionBranching ratchet hit for code Delete the Pro arm of core's built-in update source and its worker parsing #3323 deletes.
   • The if is_pro_build() branch it replaces was untested too.
   • The true arm itself is pinned by builtin_source_with_the_edition_flag_is_the_in_core_endpoint_without_a_download.

Checks after the fixes

• cargo test --lib -p nodespace-app-lib -- update_check extensions:: run_wiring: 50 passed.
• cargo clippy -p nodespace-app-lib --all-targets -- -D warnings -D clippy::unused-async: clean.
• check-pro-boundary: counts unchanged (proCommands 261, editionBranching 98, cloudWording 49, proWording 238). The push check passed.

Re-Review Decision

Decision: NO RE-REVIEW NEEDED

Rationale: Every change is documentation, a test name, a test assertion message, or one debug log line that doesn't change control flow or return values. None touches logic the review approved, and the tests and clippy pass.