[codex] fix: sync provider environment rows#3524
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff7bac007e
ℹ️ 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".
ApprovabilityVerdict: Needs human review Introduces substantial new state synchronization logic (~245 lines) for merging local environment variable drafts with server-persisted data. The complex state management with multiple edge cases warrants human review despite comprehensive test coverage. You can customize Macroscope's approvability policy. Learn more. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6298d9fb9
ℹ️ 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".
Dismissing prior approval to re-evaluate 9787cea
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9787cea601
ℹ️ 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".
Dismissing prior approval to re-evaluate 72c7478
72c7478 to
f6f464b
Compare
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
3642597 to
8f7b869
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8f7b869. Configure here.
Co-authored-by: Codex <codex@openai.com>

Summary
Root cause
ProviderEnvironmentSectioninitialized local rows from props once and did not reconcile them when refreshed or reset provider environment props changed underneath the mounted card.Impact
Validation
PATH="$HOME/.vite-plus/bin:$PATH" vp test run apps/web/src/components/settings/ProviderInstanceCard.test.ts --project unitreports no configured project namedunitin this checkout before running tests.PATH="$HOME/.vite-plus/bin:$PATH" vp test run apps/web/src/components/settings/ProviderInstanceCard.test.tspasses: 1 file, 5 tests.PATH="$HOME/.vite-plus/bin:$PATH" vp checkpasses with existing warnings outside this change.PATH="$HOME/.vite-plus/bin:$PATH" vp run typecheckpasses.Note
Medium Risk
Non-trivial client state around sensitive environment variables; regressions could drop edits, resurrect deleted rows, or mishandle redacted secret UX, though server persistence is unchanged.
Overview
Fixes provider environment variable rows going stale when persisted settings refresh or reset while the settings card stays mounted, without treating new array references as real data changes.
ProviderEnvironmentSectionnow keys sync offgetProviderEnvironmentContentKey(serialized name/value/sensitive/redaction) and runsmergeEnvironmentDraftRowsForPersistedUpdatein auseEffectso incoming persisted snapshots merge with local drafts instead of replacing them blindly.The merge keeps in-progress edits and blank “add” rows, honors local deletions (including stale server echoes via a locally-deleted map), handles reordering, and treats redacted sensitive save echoes as acknowledged when they match what was just published. Edits clear redaction on value change; deletes record removed variables before publish.
Adds broad unit coverage for the content key and merge edge cases in
ProviderInstanceCard.test.ts.Reviewed by Cursor Bugbot for commit 1d42fee. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Fix provider environment variable sync to preserve local edits across persisted updates
mergeEnvironmentDraftRowsForPersistedUpdatein ProviderInstanceCard.tsx to merge incoming persisted environment updates with local draft rows, preserving in-progress edits, unpublished adds, and local deletions.getProviderEnvironmentContentKeyto detect when the persisted environment has actually changed content, gating the merge effect.Macroscope summarized 1d42fee.