fix(store): keep an unreadable proxy row from bricking every request

[kevin9327]

The bug

8942287 ("refactor(proxy): change proxy mode from system to default") renamed the enum variant:

 pub enum ProxyMode {
     #[default]
-    System,
+    Default,
     Custom,
 }

ProxyMode is #[serde(rename_all = "snake_case")], so the persisted wire value changed from "system" to "default". Nothing migrates the existing row, and there is no alias, so any install that ever pressed Save on the proxy card before this build holds a service_settings row that this build cannot read:

Error("unknown variant `system`, expected `default` or `custom`", line: 1, column: 16)

ProxyMode::System has existed since the initial schema commit, so this is every upgrading install with saved proxy settings — including those that only ever left it on the default.

Why it is worse than a bad proxy

proxy_settings_secret propagated the error, and it is the first statement of every outbound client factory (network.rs:97, :112), which provider_client / cursor_client / default_client all go through. On upgrade the read failure takes out model calls, plugin installs (plugin/installation.rs:65), search (search/fetch.rs:127, search/search_provider.rs:98) and the Cursor upstream proxy (api/cursor/proxy.rs).

And it is unrecoverable from the UI, which is the part that made this worth a fix rather than a note:

• proxy_settings() reads the same row, so the settings page cannot render the proxy card.
• set_proxy_settings() reads the existing row before it writes (it needs the old password when password: None), so the Save that would replace the bad value fails on the bad value.

Error::Json maps to a 4xx, so the user sees a request error on a settings page and has no way forward short of editing SQLite by hand.

The fix

Read the row through a fallback that logs and returns the default:

fn read_proxy_settings(value: &str) -> ProxySettingsSecret {
    serde_json::from_str(value).unwrap_or_else(|error| {
        tracing::warn!(%error, "ignoring unreadable outbound proxy settings");
        ProxySettingsSecret::default()
    })
}

The rows this actually affects are the ones whose mode meant "no outbound proxy" — which is precisely what ProxyMode::Default is — so nothing is silently changed for them. A genuinely corrupt row costs the user a re-entered address instead of a dead install, and the warning says why.

Deliberately not #[serde(alias = "system")]. 8942287 added default_proxy_mode_uses_the_default_wire_value, which asserts serde_json::from_str::<ProxyMode>("\"system\"").is_err(). This keeps that assertion true. If you would rather the old value keep its meaning exactly, the alias is a one-line change and I am happy to switch — that is your call on intent, not mine.

Verification

cargo +1.95 test --package cursor-server --lib store::settings

Before (fallback replaced with .unwrap(), tests kept):

---- store::settings::tests::an_unreadable_proxy_row_reads_as_no_proxy stdout ----
called `Result::unwrap()` on an `Err` value:
  Error("unknown variant `system`, expected `default` or `custom`", line: 1, column: 16)

---- store::settings::tests::a_proxy_row_from_an_older_build_stays_replaceable stdout ----
called `Result::unwrap()` on an `Err` value:
  Error("unknown variant `system`, expected `default` or `custom`", line: 1, column: 16)

test result: FAILED. 3 passed; 2 failed

After:

test store::settings::tests::an_unreadable_proxy_row_reads_as_no_proxy ... ok
test store::settings::tests::a_proxy_row_from_an_older_build_stays_replaceable ... ok
test store::settings::tests::default_proxy_mode_uses_the_default_wire_value ... ok
test store::settings::tests::default_commit_prompt_follows_its_saved_locale ... ok
test store::settings::tests::custom_commit_prompt_does_not_change_with_locale ... ok

test result: ok. 5 passed; 0 failed

The second test is the one that matters: it writes the pre-rename row into a real store, then asserts both that proxy_settings() still reads and that set_proxy_settings() can overwrite it — i.e. that the settings page is reachable again.

Gates (make check, Rust half):

• cargo fmt --all -- --check — clean
• cargo clippy --workspace --all-targets -- -D warnings — clean on CI's stable; see note
• cargo test --workspace --all-targets — all suites green

Toolchain note: this machine's stable is broken, so the gates ran with +1.95. Clippy 1.95 reports a collapsible_match error at server/src/cursor/compile/model.rs:109 on unmodified main; that is a 1.95-only false positive (its own suggestion, "fast" if parse_bool(parameter)? =>, does not compile — ? is not allowed in a match guard) and CI's 1.98.1 does not emit it, so main is green. Clippy here therefore ran with -A clippy::collapsible_match; this diff is unaffected either way.

Related

This is plausibly behind #404 ("changing network environment ... BYOK link drops, custom model cannot be selected, connectivity test also errors"), where the reporter's trigger is toggling proxy software — but I have not reproduced their setup, so treat that as a lead rather than a claim.