Skip to content

cmux NEXT bookmarks: the home daemon is the only writer (drop the bookmarks.json fallback) - #31832

Closed
lawrencecchen wants to merge 1 commit into
feat-cmux-nextfrom
rust-migration/bookmarks-filestore
Closed

lawrencecchen wants to merge 1 commit into
feat-cmux-nextfrom
rust-migration/bookmarks-filestore

Conversation

@lawrencecchen

Copy link
Copy Markdown
Contributor

The app had two bookmark writers. Besides the home daemon (bookmarks-v1), BookmarkService wrote <Application Support>/<bundle id>/BrowserProfiles/bookmarks.json and showed its trees whenever the daemon did not serve bookmarks: in the connect window at launch, while the daemon was down, and after a downgrade. Edits made there only partly reached the daemon later: the one-time copy replays creates by id, so a move, rename or delete made in that window was lost. The daemon has served bookmarks-v1 since the same day the file store landed (30b7258, 2026-09-30), so the fallback only ever covered the connect window and a broken daemon.

Now the daemon is the only writer:

  • An edit before the daemon serves bookmarks is refused with DaemonError.notConnected (the existing localized "Bookmark error: ..." text; no new strings). touch (last use) stays silent, as before.
  • When the daemon stops serving, the trees stay as they were instead of switching to a file copy.
  • isLoaded is true only for a tree the daemon sent, so the Chromium bookmark sync and bookmark.list's loaded field wait for the daemon instead of trusting an empty file.
  • migrateLegacyImports (earlier browser imports into bookmarks) ran at launch and wrote its done-marker even when its edits failed, so without the file it would have failed in the connect window and never run again. It now runs once the daemon serves bookmarks and the trees are fetched.
  • BookmarkFileStore keeps only load and remove; save is gone.

What happens to existing bookmarks.json files: nothing new. As before, when the daemon serves bookmarks-v1 the app copies the file into it node by node with the same ids (an interrupted copy resumes) and then deletes the file. No build writes the file any more, so it is not created again. This PR adds no delete; the existing one is unchanged.

Behavior change: a bookmark edit in the first moments after launch, before the home daemon connects, or while the daemon is unavailable, is refused with an error instead of saved to the local file.

Why now: layer-ownership.md V-11 (bead cx-e59e). The Swift half of the shared bookmark reducer (#31818 is the Rust half) shows pending edits over the daemon's confirmed tree and has no local file mode to serve.

Observed / Not run

Observed:

  • cmux-ci compile-check --app --package CmuxNext --recipe-version 2 --ref e10faeb324ad8897cbc638e2b4a9efa59d110d5a: job ff20deb8cbb2d857ab21d97e, RESULT_PENDING.
  • scripts/cmux-next/prepush-gate.py: no new violations (arch-ratchet app_lines -36).

Not run: no regression test. The CmuxNext unit suites were deleted (36f2381) and this has no Rust part; the change is a removal checked by compile and review. Not dogfooded in a tagged build.

Changelog

Changed: Bookmark edits made before the home daemon connects are refused instead of saved to a local file that the daemon only partly picked up.

…kmarks.json fallback

BookmarkService wrote <Application Support>/<bundle id>/BrowserProfiles/
bookmarks.json and showed its trees whenever the home daemon did not serve
bookmarks-v1: in the connect window at launch, while the daemon was down,
and after a downgrade. That made a second writer whose edits only partly
reached the daemon (the one-time copy replays creates, so moves, renames and
deletes made in that window were lost).

Now an edit before the daemon serves bookmarks is refused
(DaemonError.notConnected, the existing "Bookmark error" text), and the trees
stay as they were when the daemon goes. A file left by an older build is
still copied into the daemon once (same ids) and then deleted, as before.
BookmarkFileStore keeps only load and remove.

migrateLegacyImports (earlier browser imports into bookmarks) ran at launch
and wrote its done-marker even when its edits failed; it now runs once the
daemon serves bookmarks and the trees are fetched, so it cannot fail on the
connect window and never run again.

Layer ownership V-11 (bead cx-e59e): this goes before the Swift half of the
shared bookmark reducer, which has no local file mode to serve.
@lawrencecchen lawrencecchen added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Nov 2, 2026
@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

Review e10faeb: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr31832 -f ref=rust-migration/bookmarks-filestore -f platform=macos -f backend_mode=local

@blacksmith-sh

blacksmith-sh Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Found 6 test failures on Blacksmith runners:

Failures

Test View Logs
main.DeveloperIdAppTests.test_download_prints_the_active_profile_only/
test_download_prints_the_active_profile_only
View Logs
stubs: /System/Library/Frameworks/Security.framework/Versions/A/Security has no stub/
Security has no stub
View Logs
stubs: CoreFoundation.tbd: install-name /System/Library/Frameworks/
CoreFoundation.framework/Versions/A/CoreFoundation, missing 1: ['_CFRetain']/
CoreFoundation, missing 1: ['_CFRetain']
View Logs
stubs: CoreFoundation.tbd: install-name /System/Library/Frameworks/
CoreFoundation.framework/Versions/A/CoreFoundation, missing 1: ['_log2']/
CoreFoundation, missing 1: ['_log2']
View Logs
test_every_compile_job_names_its_warm_sources (main.ThisTree.test_every_compile_job
_names_its_warm_sources)/
test_every_compile_job_names_its_warm_sources (main.ThisTree.test_every_compile_job
_names_its_warm_sources)
View Logs
test_the_repository_highlights_validate (main.DigestTests.test_the_repository_highl
ights_validate)/
test_the_repository_highlights_validate (main.DigestTests.test_the_repository_highl
ights_validate)
View Logs

Fix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Replaced by #31842: the same fix, with the regression test committed before it (no force push on this branch).

This branch was previously deployed

1 inactive deployment
ci-routing-agent — e10faeb3 Deployed Nov 2, 2026 by lawrencecchen via wait for same-key tree publisher #99343
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant