Skip to content

fix(browser): unwrap optional page restoration URL - #16395

Closed
austinywang wants to merge 1 commit into
mainfrom
fix/browser-page-restoration-compile
Closed

austinywang wants to merge 1 commit into
mainfrom
fix/browser-page-restoration-compile

Conversation

@austinywang

@austinywang austinywang commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

main's nightly Release build failed during compile admission because the Memory Saver page-restoration path passed its optional documentURL into BrowserFormStateSnapshot.isSameDocument, which requires concrete URLs. The guard now unwraps the URL before comparing session state, while preserving the existing nil behavior for panes with no committed document.

The failure is recorded in main run 36823012853, job 110244894895, at Sources/Panels/BrowserPanel+PageRestoration.swift:87.

The focused hosted run on this head no longer reports that browser error. It remains blocked by the independent duplicate notification accent declaration in Sources/Update/UpdateTitlebarAccessory.swift; that updater/notification repair is tracked in #16388, and this PR intentionally does not touch updater files.

Testing

Changelog

Fixed: page restoration now compiles when a discarded browser pane has no committed URL.

@cursor

cursor Bot commented Oct 1, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@coderabbitai

coderabbitai Bot commented Oct 1, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 4 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 6f1a95fe-2fb2-4509-8745-9e24a391197b

📥 Commits

Reviewing files that changed from the base of the PR and between a20ed74 and 9ccd4f1.

📒 Files selected for processing (1)
  • Sources/Panels/BrowserPanel+PageRestoration.swift
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No issues found across 1 file

Re-trigger cubic

@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

CI failure attribution

CI failed on 9ccd4f1f46 (run 36825836794 attempt 1): 1 code.

Job Verdict Why
macos / macOS compile admission code a compile error
Matched log lines
macos / macOS compile admission: /tmp/cmux-ci/src/Sources/Update/UpdateTitlebarAccessory.swift:989:49: error: invalid redeclaration of 'cmuxAccent'

Not re-run automatically: macos / macOS compile admission is not a machine failure.

Written by scripts/ci/classify_failures.py (ci-failure-attribution.yml); signatures are its SIGNATURES table. A machine verdict is the runner's fault, not this PR's.

@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Dogfood tours of 9ccd4f1f

browser-notifications-tour at 9ccd4f1f: not run

skipped: CI left no app build for this head (its compile failed or was cancelled)

Tours are picked by the paths globs in dogfood/scenarios/*.json; a Dogfood-tours: a, b line in the description picks them instead (none turns this off). Look at every frame before merging: a green tour only means no step failed.

@austinywang

Copy link
Copy Markdown
Contributor Author

Hosted compile admission on this head no longer reports BrowserPanel+PageRestoration.swift:87; it is blocked only by the existing main notification defect: duplicate cmuxAccent in UpdateTitlebarAccessory.swift:986,989. That repair is intentionally tracked in #16388 because this PR excludes updater files.

@austinywang

Copy link
Copy Markdown
Contributor Author

Correctness review: the one-line let documentURL guard preserves the existing nil behavior and leaves non-nil restoration semantics unchanged. Existing BrowserDiscardRestoreStrategyTests cover nil as non-restorable, so a new WKWebView integration test would add cost without testing new behavior. The remaining red compile check is the unrelated duplicate cmuxAccent in #16388.

@teamleaderleo teamleaderleo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Superseded on main: 538aaf6 ("Normalize instance tags and fix browser restoration compile") added the same let documentURL, guard at Sources/Panels/BrowserPanel+PageRestoration.swift:84. Merging this onto current main produces an empty diff, so it can close.

@teamleaderleo

Copy link
Copy Markdown
Collaborator

Closing: covered on main by 538aaf6, which added the same documentURL unwrap.

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.

2 participants