Skip to content

Revert "fix(mothership): fix url keeping markdown hash on resource sw…#3980

Closed
TheodoreSpeaks wants to merge 2 commits intomainfrom
revert-3968-fix/resource-hashtag-url
Closed

Revert "fix(mothership): fix url keeping markdown hash on resource sw…#3980
TheodoreSpeaks wants to merge 2 commits intomainfrom
revert-3968-fix/resource-hashtag-url

Conversation

@TheodoreSpeaks
Copy link
Copy Markdown
Collaborator

…itch (#3…"

This reverts commit fa51f94.

Summary

I merged directly into main. Reverting this for commit history's sake

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation
  • Other: ___________

Testing

How has this been tested? What should reviewers focus on?

Checklist

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

Screenshots/Videos

@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 6, 2026

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Apr 6, 2026 4:50pm

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented Apr 6, 2026

PR Summary

Low Risk
Low risk: changes a single replaceState URL update to preserve the current hash fragment, which could affect navigation/anchor behavior but doesn’t touch data or auth logic.

Overview
Reverts the prior behavior that cleared window.location.hash when updating the resource query param, so switching the active resource now preserves any existing URL fragment while still replaceState-updating the query string.

Reviewed by Cursor Bugbot for commit a7e4f23. Bugbot is set up for automated code reviews on this repo. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 6, 2026

Greptile Summary

This PR reverts commit fa51f94, which fixed a bug where markdown heading anchor hashes were incorrectly preserved in the URL when switching between resources in the home view. The revert is explicitly described as intentional — the author merged directly to main and is reverting to maintain proper commit history before re-landing the fix via a formal PR.

  • Reintroduced bug: The useEffect syncing activeResourceId to the URL query param uses url.toString(), which includes the URL hash fragment. A user who clicks a markdown heading anchor (e.g., #section-title) and then switches to a different resource will find the stale hash appended to the new resource's URL, causing confusing or broken anchor navigation
  • Intentional but risky: Main will regress with this bug until the follow-up fix PR is merged; there is no linked follow-up PR referenced in this description
  • No code quality issues: Outside the intentional regression, the file follows project conventions (absolute imports, createLogger, correct Tailwind usage, proper TypeScript)

Confidence Score: 3/5

This PR intentionally reintroduces a known UX bug and should only be merged if a follow-up fix PR is ready to land immediately after

Score of 3 reflects that while the revert is intentional and acknowledged, it re-introduces a confirmed regression (URL hash persistence on resource switch) into main with no linked follow-up PR to restore the fix. The bug is not a security or data-loss issue but causes broken navigation UX.

apps/sim/app/workspace/[workspaceId]/home/home.tsx — the URL sync useEffect at lines 191-199 needs the hash-stripping logic restored

Important Files Changed

Filename Overview
apps/sim/app/workspace/[workspaceId]/home/home.tsx Revert removes URL hash-stripping fix, reintroducing bug where markdown heading anchors persist in the URL across resource switches

Sequence Diagram

sequenceDiagram
    participant User
    participant Browser
    participant home.tsx

    User->>Browser: View markdown resource A, clicks heading anchor
    Browser->>Browser: URL becomes /workspace/ws1/home?resource=A#heading
    User->>home.tsx: Switches to resource B (setActiveResourceId)
    home.tsx->>Browser: new URL(window.location.href) includes #heading
    home.tsx->>Browser: url.searchParams.set('resource', 'B')
    home.tsx->>Browser: replaceState(null, '', url.toString())
    Note over Browser: URL = /workspace/ws1/home?resource=B#heading ❌
    Note over Browser: Hash from resource A incorrectly persists
Loading

Comments Outside Diff (1)

  1. apps/sim/app/workspace/[workspaceId]/home/home.tsx, line 191-199 (link)

    P1 Reintroduced URL hash persistence bug

    This revert restores a known bug: url.toString() includes the full URL hash fragment (#...), so when a user has navigated to a markdown heading anchor in one resource and then switches to a different resource, the hash from the previous resource persists in the URL via window.history.replaceState. The reverted commit (fa51f94) fixed this by stripping the hash before calling replaceState.

    While this revert is intentional per the PR description ("for commit history's sake"), is there a follow-up PR queued to re-apply the fix? Merging this means main temporarily regresses with this bug.

Reviews (1): Last reviewed commit: "Revert "fix(mothership): fix url keeping..." | Re-trigger Greptile

@waleedlatif1 waleedlatif1 deleted the revert-3968-fix/resource-hashtag-url branch April 7, 2026 20:01
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