fix: move Twilio phone number clear to success path of config response#7421
Merged
Conversation
Co-Authored-By: Claude <noreply@anthropic.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![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: 9a1c7e3a14
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+354
to
+357
| if !response.hasCredentials { | ||
| // Credentials were confirmed removed — clear derived state | ||
| self.twilioPhoneNumber = nil | ||
| self.twilioNumbers = [] |
There was a problem hiding this comment.
onTwilioConfigResponse now clears twilioPhoneNumber whenever response.hasCredentials is false, which drops the phone number returned by successful get responses for accounts that have an assigned number but no active credentials. The daemon explicitly preserves and reports that number (assistant/src/daemon/handlers/config.ts, comments around clear_credentials and get response fields) so users can re-enter credentials without reassigning; after this change, the macOS store forgets it and the UI shows "Not assigned" until a later manual reassignment/refresh flow repopulates it.
Useful? React with 👍 / 👎.
7 tasks
ashleeradka
added a commit
that referenced
this pull request
May 20, 2026
) * fix(web): port homepage UI tweaks from platform #7421 (LUM-1726) Drift port of vellum-assistant-platform#7421 (commit f216413). - Suggestion pill icon size: 9px → 18px - Hide feed filter bar when only one category is present (<=1) - Filter bar → feed sections spacing: lg → sm (8px) - Unread indicator dot: right → left of feed row icon - Detail panel default: defaultLeftWidth={600} → defaultLeftPercent={50} resolved via useLayoutEffect to prevent flash from pixel fallback - Derive effectiveFilter from activeFilter + presentCategories so a stale category filter automatically falls back to "All" instead of stranding the feed in an empty state with no visible filter controls Adds defaultLeftPercent prop to ResizablePanel (design library) with useLayoutEffect-based resolution that respects existing localStorage preferences. https://claude.ai/code/session_013dBXRbLF218UhdLq7FEAvv * fix(web): address review feedback on homepage UI tweaks Two findings flagged in review, both fixed properly rather than faithful-port mirroring of platform — this repo doesn't carry forward upstream tech debt. 1. ResizablePanel (Codex P2): malformed localStorage values made the useState initializer fall back to defaultLeftWidth while the new useLayoutEffect skipped percent resolution, leaving a persistent unintended width. Extracted a readStoredWidth() helper that does shape + finiteness validation in one place and is used by both the initializer and the percent-resolution effect. 2. HomeFeedList (Devin): the previous diff resolved a stale activeFilter via the derived effectiveFilter for rendering, but never reset the underlying state. If the selected category disappeared and later reappeared, the filter would silently re-activate. Use React's adjust-state-during-render pattern to keep activeFilter in sync with effectiveFilter — React bails out on same-value setState so this avoids a synchronization Effect. https://react.dev/reference/react/useState#storing-information-from-previous-renders https://claude.ai/code/session_013dBXRbLF218UhdLq7FEAvv * docs(web): document "don't carry upstream tech debt" migration policy Add explicit guidance to apps/web/AGENTS.md so this rule persists across agent sessions and to anyone (human or AI) working on the migration: - A faithful port preserves behavior and feature parity, not every implementation choice. The stack changed; the code should change with it. - This is an OSS repo — align with React, React Router, and major OSS players' recommended patterns rather than mirroring the platform repo. - When a bot review flags a real issue in newly-ported code, fix it rather than dismissing as "matches upstream." The platform will be deprecated; we don't need to mirror its bugs. - Small refactors belong in the port PR; large ones get a separate Linear issue. - PR descriptions should call out divergences from the platform implementation so reviewers understand the deltas. https://claude.ai/code/session_013dBXRbLF218UhdLq7FEAvv * docs(web): revert migration-policy section from AGENTS.md OSS contributors working in this repo have no context on the platform repo and shouldn't need to. The "don't carry over upstream tech debt" policy belongs in the platform repo's web docs (alongside the existing migration content), not here. Reverting the section added in 845ebf0. Equivalent guidance moves to vellum-assistant-platform/web/CLAUDE.md. https://claude.ai/code/session_013dBXRbLF218UhdLq7FEAvv --------- Co-authored-by: Claude <noreply@anthropic.com>
Merged
5 tasks
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
1 participant
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.
Addresses review feedback on #7220. Moves optimistic UI updates (twilioPhoneNumber = nil, twilioNumbers = [], etc.) from the button action into the onTwilioConfigResponse success handler, so they only apply after the backend confirms credentials were cleared.