fix(web): gate Sync All settings hop on uiMode=dev (PR #488 follow-up)#489
Merged
fix(web): gate Sync All settings hop on uiMode=dev (PR #488 follow-up)#489
Conversation
…Mode=dev Follow-up to #488 (graduating the Context Gateway tabs to prod). The "Sync All" button on the Artifact Sync overview also fans out to ``/api/context/settings/sync`` (settings-hook merge), and that endpoint lives on the ``settings_sync`` router which intentionally stays dev-only. In prod the sync hop returned 404 and the whole button toasted "Settings sync failed" *after* successfully syncing skills/commands/agents — making the prod surface look broken even though the artifact fanout completed. Same shape on the overview's 4th card: it labels Settings and links into the ``hooks-sync`` section, which is hidden in prod, so the card navigated to a dead tab. Both code paths now self-gate on ``STATE.uiMode === 'dev'`` (matching the Home dashboard pattern in app.js:657-690): the settings card and the settings sync hop only appear/run when the user is in dev mode. Prod users get a clean 3-card overview and a Sync All that completes silently for the artifacts they actually have a UI for. Pin test added so the gate doesn't silently disappear in a future JS refactor — same source-grep style as the existing app.js pins. Verified end-to-end via Playwright + isolated ``mm web`` (HOME override, fixture project under /tmp): overview renders 3 cards in prod / 4 in dev; Sync All toasts "동기화 완료" with no error in prod; filesystem outcome correct (``.claude/``, ``.codex/agents/*.toml`` project-scope per #483, ``.gemini/``); no HOME pollution. Co-Authored-By: Claude <[email protected]>
Per review feedback on #489: the original ``STATE.uiMode === 'dev'`` substring assertion would still pass if a future refactor removed one of the two gates (overview-card push, Sync All settings hop) while leaving the other. Switch to a count >= 2 assertion so both call sites must survive together. Comment names the two sites for the person who hits the failure later. Co-Authored-By: Claude <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Regression caught during functional testing of #488. The Context Gateway tabs ship in the prod surface now, but two code paths in
context-gateway.jsreach into the still-dev-onlysettings_syncrouter:context-gateway.js:124) — after fanning out skills/commands/agents successfully, itPOSTs/api/context/settings/sync. That endpoint lives on the dev-only router, so prod returns 404 and the button toasts"Settings sync failed"even though the artifact fanout already completed. The user sees a red error after a successful sync.hooks-syncsection.hooks-syncis hidden in prod (still dev-only by design), so clicking the card navigates to a tab that isn't rendered.Both paths now self-gate on
STATE.uiMode === 'dev', matching the established pattern inapp.js:657-690for the Home dashboard's namespaces/sessions/scratch fetches. Prod users get a clean 3-card overview (skills/commands/agents) and a Sync All button that completes silently against the artifacts they have a UI for.Test plan
uv run ruff check packages/memtomem/src+ruff format --check— cleanuv run pytest packages/memtomem/tests/test_web_mode.py— 31 passed, including new pintest_app_js_pins_ui_mode_default_and_toast_copynow also assertscontext-gateway.jscontains theSTATE.uiMode === 'dev'gate. Same source-grep style as the existing app.js pins (no JS runtime in CI).mm web(HOME override + fixture project under/tmp):Settings sync failederror..claude/skills/,.claude/commands/,.claude/agents/,.codex/agents/*.toml(project-scope per feat(context)!: codex_agents project-scope default #483),.gemini/agents/. No HOME pollution.Related
feedback_tier2_web_gating_deferred.mdkeepssettings_syncin_DEV_ONLY_ROUTERSintentionally; this fix is the cheap UX patch rather than promoting another router.🤖 Generated with Claude Code