Skip to content

fix(settings): prevent false unsaved prompts and preserve drafts - #8704

Merged
waleedlatif1 merged 6 commits into
stagingfrom
codex/settings-unsaved-changes
Oct 7, 2026
Merged

waleedlatif1 merged 6 commits into
stagingfrom
codex/settings-unsaved-changes

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Prevent false unsaved prompts from loading, defaults, refetches, and edits reverted to saved values.
  • Preserve authored drafts and failed saves across workspace and organization settings, including inline creation forms and hidden SSO/provisioning drafts; share link, tab, history, refresh, and pending-request protection.
  • Remove duplicate dialogs/listeners, keep untouched credential fields current, and reconcile secret rows atomically so later edits cannot leave extra keys or lose existing values. Apply fresh secret snapshots after reverting or saving, and preserve same-page hash drafts across native and programmatic history. Preserve structured-clone history state and expire superseded traversal confirmations while retaining metadata-only updates. Confirmed history traversal discards drafts only when the browser reports an actual route change; no-ops and hashes retain edits, and pending saves still win.

Browser support boundary

Native Back/Forward uses Navigation API indexes or entries tracked by the app. Without the Navigation API, unindexed native entries cannot be cancelled reliably because popstate exposes no direction; those traversals pass through without guessing or rewriting the history stack. Programmatic traversal and beforeunload remain guarded. Unknown classic cross-document traversal may require native confirmation after the shared dialog; permission never suppresses later unload protection.

Type of Change

  • Bug fix

Testing

  • 196 focused settings and sibling tests; regressions demonstrated red on pre-fix or guard-removal controls.
  • 22 real Next.js App Router browser checks for clean links, Keep editing/discard, native Back/Forward, managed replace links, pending requests, same-page hashes, out-of-range traversal, converted deltas, and cross-origin draft retention.
  • Root test suite, all-workspace type checking, lint, all 58 audits, docs manifest, and block registry validation.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing (new tests pass the test-audit authoring gate)
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercel Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
docs Ready Ready Preview Oct 7, 2026 3:41am UTC

Request Review

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 3 files

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Turn on auto-fix | Re-trigger cubic

Comment thread apps/sim/ee/custom-blocks/components/custom-block-detail.tsx Outdated
@greptile-apps

greptile-apps Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium risk] Refactors settings unsaved-changes handling across many pages.

This PR appears safe to merge; no new blocking issue was found.

What we checked:

  • Back without a destination: requestLeave skips discard at confirmation. Drafts are discarded only after the browser reports departure, so a no-op keeps them.

Summary

This PR shares settings navigation protection and keeps authored drafts separate from saved values.

  • The latest changes wait for an actual history traversal before discarding drafts.
  • Back or Forward with no destination leaves drafts intact.
  • A save that starts before traversal keeps the user on the edited page.
  • All five supplied previous threads are unnumbered. Their original issues are addressed: typed whitespace remains, secret rows reconcile together, saved empty secrets remain valid, row matching uses maps, and same-page history preserves drafts.
  • No new actionable issues found.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Back or Forward] --> B{Same page?}
  B -->|Yes| C[Traverse and keep draft]
  B -->|No| D{Save pending?}
  D -->|Yes| E[Stay on edited page]
  D -->|No, draft exists| F[Ask before leaving]
  F -->|Keep editing| E
  F -->|Discard| G[Authorize traversal]
  G --> H{Browser reports departure?}
  H -->|No| I[Keep draft]
  H -->|Yes| J[Discard and leave]
Loading

Reviews (6) · Last reviewed commit: "fix(settings): retain drafts until histo..." · Reviewed by Greptile

Comment thread apps/sim/ee/custom-blocks/components/custom-block-detail.tsx Outdated
@waleedlatif1
waleedlatif1 force-pushed the codex/settings-unsaved-changes branch from 045b060 to 22fe83b Compare October 7, 2026 01:26
@waleedlatif1 waleedlatif1 changed the title fix(settings): prevent false custom block unsaved changes fix(settings): preserve drafts and guard settings navigation Oct 7, 2026
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 52 files

Turn on auto-fix | Re-trigger cubic

Comment thread apps/sim/components/settings/use-settings-browser-navigation.ts Outdated
Comment thread apps/sim/ee/scim/components/scim-section.tsx Outdated
Comment thread .cursor/rules/sim-settings-pages.mdc Outdated
Comment thread apps/sim/components/secrets/secrets-editor.tsx Outdated
Comment thread apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.ts
Comment thread apps/sim/components/secrets/secrets-editor.tsx Outdated
Comment thread apps/sim/components/secrets/secrets-editor.tsx Outdated
Comment thread apps/sim/components/secrets/secrets-editor.tsx Outdated
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 53 files

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Turn on auto-fix | Re-trigger cubic

Comment thread apps/sim/components/secrets/secrets-editor.tsx Outdated
Comment thread apps/sim/ee/scim/components/scim-section.tsx Outdated
Comment thread .agents/skills/add-settings-page/SKILL.md Outdated
Comment thread .claude/rules/sim-settings-pages.md Outdated
@waleedlatif1 waleedlatif1 changed the title fix(settings): preserve drafts and guard settings navigation fix(settings): prevent false unsaved prompts and preserve drafts Oct 7, 2026
Comment thread apps/sim/components/settings/use-settings-browser-navigation.ts Outdated
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 53 files

You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.

Fix all with cubic | Turn on auto-fix | Re-trigger cubic

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/sim/components/settings/use-settings-browser-navigation.ts">

<violation number="1" location="apps/sim/components/settings/use-settings-browser-navigation.ts:50">
P2: This stamp also rewrites non-plain structured-clone state: spreading a `Map` drops its entries, and spreading a `Date` drops its value. Stamp only plain-object state and leave other state types intact.</violation>

<violation number="2" location="apps/sim/components/settings/use-settings-browser-navigation.ts:101">
P2: A confirmed traversal beyond the history boundary emits no `popstate`, leaving `allowTraversal` set; a later Back with the same delta then bypasses the dirty-state check and can lose a new draft. Clear this authorization when a traversal is superseded or produces no matching event.</violation>
</file>

Comment thread apps/sim/components/settings/use-settings-browser-navigation.ts Outdated
Comment thread apps/sim/components/settings/use-settings-browser-navigation.ts Outdated
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 53 files

You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.

Fix all with cubic | Turn on auto-fix | Re-trigger cubic

Comment thread apps/sim/components/settings/use-settings-browser-navigation.ts Outdated
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 53 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.

Turn on auto-fix | Re-trigger cubic

@waleedlatif1
waleedlatif1 merged commit c5d07cc into staging Oct 7, 2026
36 of 37 checks passed
@waleedlatif1
waleedlatif1 deleted the codex/settings-unsaved-changes branch October 7, 2026 05:48

This branch was successfully deployed

1 active deployment
Preview — f6fe47b0 Deployed Oct 7, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant