feat: add global URL-Test lazy and tolerance settings#53
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab9e9a14ef
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ...(urlTestLazy !== undefined ? { urlTestLazy } : {}), | ||
| ...(urlTestTolerance !== undefined ? { urlTestTolerance } : {}), |
There was a problem hiding this comment.
When editing an existing subscription that already has urlTestLazy/urlTestTolerance saved, disabling the new switch sets both store values to undefined, so these spreads omit them from the config payload. The local update path then merges the submitted config over currentSecrets.config (local/src/lib/subscription-service.ts:220 via normalizeSubscriptionConfigForPersistence's { ...existingConfig, ...submittedConfig }), so the old override values remain encrypted and server-side regeneration keeps using them. Send an explicit clearing value or otherwise delete the saved keys when the override is disabled.
Useful? React with 👍 / 👎.
| urlTestLazy: | ||
| typeof cfg.urlTestLazy === "boolean" ? (cfg.urlTestLazy as boolean) : state.urlTestLazy, |
There was a problem hiding this comment.
When loading a saved subscription whose config omits these new optional fields, such as any preexisting subscription or one with the override disabled, this falls back to the current draft's urlTestLazy/urlTestTolerance instead of the legacy undefined state. If the user previously enabled global URL-Test settings, opening such a subscription silently applies those settings to its generated YAML and subsequent save. Missing fields should hydrate as undefined rather than inheriting the previous screen state.
Useful? React with 👍 / 👎.
| urlTestLazy: | ||
| typeof config.urlTestLazy === "boolean" ? config.urlTestLazy : state.urlTestLazy, |
There was a problem hiding this comment.
When applying a built-in or legacy template that omits the new optional URL-Test fields, this preserves whatever override is currently in the editor instead of reverting to the template's legacy behavior. For example, after enabling urlTestLazy/urlTestTolerance, applying a default template still leaves those overrides in the generated YAML even though the template config does not specify them. Missing template fields should clear these optional overrides rather than inherit stale state.
Useful? React with 👍 / 👎.
| urlTestTolerance: | ||
| typeof config.urlTestTolerance === "number" | ||
| ? config.urlTestTolerance | ||
| : state.urlTestTolerance, |
There was a problem hiding this comment.
This accepts any numeric urlTestTolerance from a loaded template, while the template validator and other hydration paths require a non-negative integer. If a malformed or unvalidated template config contains -1 or 1.5, applying it stores that value and the generator emits an invalid tolerance into every URL-Test group. Keep this runtime guard consistent with validateSubBoostTemplateConfig/persistence by requiring an integer >= 0 before applying it.
Useful? React with 👍 / 👎.
1 participant
Summary
Testing