feat(hostd): replace per-price pin toggles with a single switch - #1539
Open
telestrial wants to merge 1 commit into
Open
feat(hostd): replace per-price pin toggles with a single switch#1539telestrial wants to merge 1 commit into
telestrial wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
4 Skipped Deployments
|
🦋 Changeset detectedLatest commit: b1baabe The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
telestrial
marked this pull request as ready for review
September 11, 2026 21:39
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
Claude Code Review is paused for this repository. To reconnect it, an admin of this repository's GitHub organization (or the account owner, for personal repositories) who can also manage your Claude organization's Code Review settings needs to re-link GitHub in Code Review settings. This is a one-time step.
Tip: disable this comment in your organization's Code Review settings.
There was a problem hiding this comment.
🟡 Changes recommended
Resolve mixed-state save behavior, pinning-disabled UI handling, effective-state validation, and Save availability for explicit unpinning.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR replaces hostd’s per-price pin toggles with a single all-or-nothing switch while preserving legacy mixed configurations.
Changes:
- Updates pinning transforms, UI, validation, and save-state handling.
- Adds mixed-pinning notices and interaction tracking.
- Adds unit/E2E coverage, fixtures, and a changeset.
File summaries
| File | Summary |
|---|---|
libs/e2e/src/fixtures/textInput.ts |
Adds visibility assertions. |
apps/hostd/contexts/config/usePinningTouched.tsx |
Tracks explicit pinning changes. |
apps/hostd/contexts/config/useOnValid.tsx |
Handles pinning save behavior. |
apps/hostd/contexts/config/types.ts |
Defines pinning state types. |
apps/hostd/contexts/config/transform.ts |
Transforms aggregate and legacy pinning states. |
apps/hostd/contexts/config/transform.spec.ts |
Tests transform behavior. |
apps/hostd/contexts/config/index.tsx |
Exposes legacy pinning state. |
apps/hostd/contexts/config/fields.tsx |
Updates pricing fields and validation. |
apps/hostd/components/Config/PricingSetting.tsx |
Adds shared pricing controls and notices. |
apps/hostd/components/Config/index.tsx |
Renders unified pricing controls. |
apps/hostd-e2e/src/specs/configLegacyPinning.spec.ts |
Tests legacy mixed configurations. |
apps/hostd-e2e/src/specs/config.spec.ts |
Updates pricing configuration tests. |
apps/hostd-e2e/src/fixtures/configSetMixedPinning.ts |
Provides mixed-pinning setup. |
apps/hostd-e2e/src/fixtures/configResetAllSettings.ts |
Updates configuration reset behavior. |
.changeset/pin-all-prices-or-none.md |
Documents the user-facing change. |
Review details
Suppressed comments (6)
apps/hostd/contexts/config/fields.tsx:264
- In the mixed legacy state, this fiat input is rendered for a pinned storage price while
values.shouldPinPricesis false. The short-circuit therefore accepts a negative pinned value andtransformUpSettingsPinnedcan send it in the preserved mixed payload; validate against the effective per-price pin state instead (and apply the same correction to the other pinned-price validators).
!values.shouldPinPrices ||
apps/hostd/contexts/config/fields.tsx:313
- In the mixed legacy state, this fiat input is rendered for a pinned egress price while
values.shouldPinPricesis false. The short-circuit therefore accepts a negative pinned value andtransformUpSettingsPinnedcan send it in the preserved mixed payload; validate against the effective per-price pin state instead.
!values.shouldPinPrices ||
apps/hostd/contexts/config/fields.tsx:361
- In the mixed legacy state, this fiat input is rendered for a pinned ingress price while
values.shouldPinPricesis false. The short-circuit therefore accepts a negative pinned value andtransformUpSettingsPinnedcan send it in the preserved mixed payload; validate against the effective per-price pin state instead.
!values.shouldPinPrices ||
apps/hostd/contexts/config/fields.tsx:426
- In the mixed legacy state, this fiat input is rendered for a pinned max-collateral value while
values.shouldPinPricesis false. The short-circuit therefore accepts a negative pinned value andtransformUpSettingsPinnedcan send it in the preserved mixed payload; validate against the effective per-price pin state instead.
!values.shouldPinPrices ||
apps/hostd/contexts/config/index.tsx:78
- This notice is derived from the pinned response without checking
pinningEnabled. Since the pinned resource is requested even when the explorer feature is disabled, a disabled host with a legacy mixed response can see an alert telling it to change pinning whileshouldPinPricesitself is hidden. IncludepinningEnabledin this condition so the notice is only shown when the host can act on it.
const showMixedPinningNotice = hasMixedPinning && !pinningTouched
apps/hostd/contexts/config/usePinningTouched.tsx:24
- Tracking touched state here is not enough to make the all-unpinned resolution savable.
ConfigActionsstill disables Save whenform.formState.isDirtyis false; toggling a legacy mixed switch false → true → false marks this field touched but restores its default value, soisDirtybecomes false. With no other edit, the host cannot submit the deliberate choice and the mixed configuration remains. ExposepinningTouchedthrough the config context and include it in the save-enabled condition (or otherwise mark this action dirty).
pinningTouched: !!touchedFields.shouldPinPrices,
- Files reviewed: 15/15 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
telestrial
force-pushed
the
09-11-feat_hostd_replace_per-price_pin_toggles_with_a_single_switch
branch
from
September 11, 2026 21:58
2a8f7b5 to
b1baabe
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
Mixed legacy pin validation and pinning-change tracking need correction before approval.
Review details
Suppressed comments (6)
apps/hostd/contexts/config/fields.tsx:264
- For a mixed legacy host,
transformDownsetsshouldPinPricesto false even when this storage fiat input is still shown as pinned. This condition therefore short-circuits the non-negative check, allowing a negative value to be submitted for an active pinned price. Validate against the effective per-price pin state, including the legacy state, rather than only the aggregate switch.
!values.shouldPinPrices ||
apps/hostd/contexts/config/fields.tsx:313
- For a mixed legacy host,
transformDownsetsshouldPinPricesto false even when this egress fiat input is still shown as pinned. This condition therefore short-circuits the non-negative check, allowing a negative value to be submitted for an active pinned price. Validate against the effective per-price pin state, including the legacy state, rather than only the aggregate switch.
!values.shouldPinPrices ||
apps/hostd/contexts/config/fields.tsx:361
- For a mixed legacy host,
transformDownsetsshouldPinPricesto false even when this ingress fiat input is still shown as pinned. This condition therefore short-circuits the non-negative check, allowing a negative value to be submitted for an active pinned price. Validate against the effective per-price pin state, including the legacy state, rather than only the aggregate switch.
!values.shouldPinPrices ||
apps/hostd/contexts/config/fields.tsx:426
- For a mixed legacy host,
transformDownsetsshouldPinPricesto false even when this max-collateral fiat input is still shown as pinned. This condition therefore short-circuits the non-negative check, allowing a negative value to be submitted for an active pinned price. Validate against the effective per-price pin state, including the legacy state, rather than only the aggregate switch.
!values.shouldPinPrices ||
apps/hostd/contexts/config/transform.ts:313
- For a mixed legacy host,
transformDowndeliberately setsshouldPinPricestofalse, but the pinned fiat inputs for the individually pinned prices are still displayed. The range validators infields.tsxnow key off this aggregate flag, so a negative value entered into a displayed legacy-pinned field passes validation and is sent back withpinned: true; validate against the effective per-price pin state instead of the aggregate switch.
shouldPinPrices:
settingsPinned.storage.pinned &&
settingsPinned.egress.pinned &&
settingsPinned.ingress.pinned &&
settingsPinned.maxCollateral.pinned,
apps/hostd/contexts/config/usePinningTouched.tsx:24
- This treats a field blur as an explicit pinning change.
FieldSwitchpasses itsonBlurinto React Hook Form, so a user who merely tabs through the switch setstouchedFields.shouldPinPricesand the next unrelated save resolves a mixed configuration even though the switch value never changed. Track an actual checked-value change separately while retaining the on/off/on case.
pinningTouched: !!touchedFields.shouldPinPrices,
- Files reviewed: 15/15 changed files
- Comments generated: 0 new
- Review effort level: Lite
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR changes hostd's configuration page to support either pinning all pinnable fields or none of them, while still respecting the legacy mixed configuration. After this PR, it will not be possible through the UI to set a new mixed configuration. I think it's still possible to set a mixed config through the API, though?
Single field pinning toggles are gone. If you're in that legacy state, the overall pin toggle is set to the off position. I wrestled with that and can see arguments for having it in the on position. I'm open to whatever we think is best.
I tested this on my own host without saving anything and used clusterd beyond that, so more manual testing on a real hostd daemon would be useful before we release this into the wild.