Skip to content

Put the mobile diff viewer behind a remote feature flag, off by default - #8937

Merged
azooz2003-bit merged 2 commits into
mainfrom
feat-diffv-remote-flag
Jul 27, 2026
Merged

azooz2003-bit merged 2 commits into
mainfrom
feat-diffv-remote-flag

Conversation

@azooz2003-bit

@azooz2003-bit azooz2003-bit commented Jul 25, 2026 •

Copy link
Copy Markdown
Collaborator

The iOS Changes viewer (#8221) now ships dark behind mobile-workspace-changes-enabled-release, a PostHog-backed flag in the CmuxFeatureFlags registry, defaulting to off on Release builds.

The gate lives at the feature's single host-side choke point. When the flag is off, mobile.host.status stops advertising workspace.changes.v1, and the five mobile.workspace.changes.* RPCs answer capability_disabled through one shared router guard in TerminalController+MobileWorkspaceChanges.swift. Every iOS entry point (workspace-row chip, toolbar button, one-time hint, Changes sheet, and summary polling) already feature-detects on that capability, so the phone UI turns itself off with zero iOS changes, and a phone holding a stale cached capability list cannot call through.

Status payloads are served off the main actor, so CmuxFeatureFlags gains a lock-protected off-main snapshot published by the shared instance (OSAllocatedUnfairLock, the existing idiom); before the snapshot exists readers get the per-flag compile-time default, which fails closed on Release. DEBUG builds default the flag on for dogfood, matching the cloud-vm-ui-enabled-release and pro-upgrade-ui-enabled-release convention; the local override in the Feature Flags window works as usual and remote values remain authoritative.

Verified live against a paired simulator with the tagged difv build, same two dirty demo workspaces in both states. Flag on: chips render (+48 −10 and +6,215 −6,160) and the device log shows changes.summary ok requested=3 summaries=3 chips=2. Flag off (local override): the same rows render with no chips, the log shows no changes traffic at all (the phone never polls), and the advertised capability count drops by exactly one. scripts/lint-feature-flags.py passes; capability inclusion/exclusion and flag-resolution tests added to the already-wired MobileHostConnectionLifecycleTests.

One ops note: the flag does not exist in the PostHog dashboard yet (no personal API key on this machine). An absent remote flag resolves to the Release default (off), so nothing ships enabled; create mobile-workspace-changes-enabled-release in PostHog project 244066 when ready to roll out or to use it as a kill switch.

🤖 Generated with Claude Code


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


Summary by cubic

Gates the iOS mobile diff viewer behind a remote feature flag so we can roll out safely. Also merges main and keeps the CmuxFeatureFlags telemetry-gated remote loader alongside the off-main snapshot publisher.

  • New Features

    • Added mobile-workspace-changes-enabled-release to CmuxFeatureFlags (Release off, DEBUG on; remote via PostHog, local overrides respected).
    • mobile.host.status advertises workspace.changes.v1 only when enabled; all mobile.workspace.changes.* RPCs route through one guard and return capability_disabled when off.
    • Published an off-main effective-values snapshot from CmuxFeatureFlags.shared so status payloads can read flags off the main actor.
    • Added localized strings for the flag title/description and the capability-disabled error; tests cover capability inclusion and remote flag resolution.
  • Migration

    • Create mobile-workspace-changes-enabled-release in PostHog project 244066 when ready to roll out or to use as a kill switch.

Written for commit 2aea225. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features

    • Added localized English and Japanese labels and messages for mobile workspace changes.
    • Added support for viewing workspace change summaries, files, diffs, statistics, and file contents from mobile clients.
    • Workspace changes are now controlled by a feature flag and advertised only when enabled.
    • Added a clear capability-disabled message when the feature is unavailable.
  • Tests

    • Added coverage for feature-flag behavior and capability advertisement.

mobile-workspace-changes-enabled-release (PostHog, registry in
CmuxFeatureFlags) now gates the feature at its single host-side choke
point: when off, mobile.host.status omits workspace.changes.v1 and the
five mobile.workspace.changes.* RPCs answer capability_disabled through
one shared router guard. Every iOS entry point (workspace-row chip,
toolbar button, one-time hint, Changes sheet, summary polling) already
feature-detects on that capability, so the phone UI turns off with no
iOS change and a stale cached capability list cannot call through.

Status payloads are served off the main actor, so the flag publishes a
lock-protected off-main snapshot from the shared instance; readers fall
back to the per-flag compile-time default before it exists. Release
defaults off until the PostHog flag enables it; DEBUG defaults on for
dogfood, matching the cloud-vm-ui and pro-upgrade-ui convention.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 25, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a mobile workspace changes feature flag with off-main snapshots, gates capability advertisement, centralizes workspace changes RPC dispatch, adds localized messages, and tests default, remote, and capability behavior.

Changes

Mobile workspace changes

Layer / File(s) Summary
Feature flag and snapshot support
Sources/FeatureFlags.swift, Resources/Localizable.xcstrings
Registers the mobile workspace changes flag, provides build-specific defaults and localized metadata, and publishes effective values through an optional locked snapshot.
Capability advertisement and validation
Sources/Mobile/MobileHostService+Capabilities.swift, cmuxTests/MobileHostConnectionLifecycleTests.swift
Conditionally includes workspace.changes.v1 in mobile capabilities and tests feature-flag and remote-value behavior.
Centralized RPC dispatch
Sources/TerminalController.swift, Sources/TerminalController+MobileWorkspaceChanges.swift, Resources/Localizable.xcstrings
Routes workspace changes methods through a gated dispatcher, forwarding supported methods and returning localized disabled or unknown-method errors.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant MobileClient
  participant TerminalController
  participant CmuxFeatureFlags
  participant WorkspaceChangeHandlers
  MobileClient->>TerminalController: mobile.workspace.changes.* RPC
  TerminalController->>CmuxFeatureFlags: read feature flag
  alt enabled
    TerminalController->>WorkspaceChangeHandlers: dispatch supported method
    WorkspaceChangeHandlers-->>MobileClient: workspace changes result
  else disabled
    TerminalController-->>MobileClient: capability_disabled error
  end
Loading

Possibly related PRs

Suggested reviewers: austinywang


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (3 errors)

Check name Status Explanation Resolution
Cmux Swift Blocking Runtime ❌ Error FAIL: FeatureFlags.swift adds an OSAllocatedUnfairLock-guarded shared snapshot for mobileHostCapabilities; the rule flags new manual locks in production Swift. Replace the lock with actor/MainActor-owned snapshot publication or another explicit signal-based cache, and keep nonisolated status readers read-only.
Cmux Swift @Concurrent ❌ Error New @MainActor workspace-changes RPCs await file/git-heavy WorkspaceChangesService async methods that are nonisolated but lack @concurrent. Add @concurrent to the WorkspaceChangesService async entrypoints (summary/changedFiles/fileStat/fileFetch/fileDiff) or hop the RPC work off @MainActor.
Cmux Full Internationalization ❌ Error New Localizable.xcstrings entries are only translated in en/ja, but the catalog already supports many more locales; the new router also adds an unlocalized "Unknown mobile method" message. Add translated values for every locale already present in Resources/Localizable.xcstrings, and localize the new mobile.workspace.changes method_not_found message (or reuse an existing localized key).
✅ Passed checks (22 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Swift Actor Isolation ✅ Passed The diff adds a lock-protected off-main snapshot and main-actor routing; no new unsafe shared mutable Sendable state or background UI-store access appears.
Cmux Browser Automation Off-Main ✅ Passed Diff only adds mobile.workspace.changes gating/router code; no browser.* command was moved to mainActor or processV2Command, and worker routing stays unchanged.
Cmux Expensive Synchronous Load ✅ Passed PASS: Only feature-flag gating and mobile changes routing were added; no agent-history/JSONL loader was moved onto MainActor or an interactive path.
Cmux Cache Substitution Correctness ✅ Passed PASS: status uses an off-main flag snapshot with cold-default fallback and event-driven refresh; the RPC gate still checks the live shared flag, so stale status caches can’t bypass.
Cmux No Hacky Sleeps ✅ Passed PR changes are Swift and xcstrings only; no TS/JS/shell/build-runtime code introduces sleeps/timers/polling.
Cmux Algorithmic Complexity ✅ Passed PASS: The new scans are over tiny fixed/static collections (8 feature flags, fixed capability list, 1–64 requested workspaces), and no scalable hot path gained nested rescans.
Cmux Swift Concurrency ✅ Passed No banned legacy async patterns were introduced; the only new Task is a notification-callback hop, and the rest uses async/await plus tests.
Cmux Swift Package Boundaries ✅ Passed App-lifecycle glue only: feature flags, capability gating, and RPC routing wrap existing package logic; reusable domain code stays in CMuxMobileChanges/CmuxGit.
Cmux Swiftpm Lockfiles ✅ Passed Diff only touches Swift source and localization files; no .gitignore, workflow, package, or Package.resolved changes appear.
Cmux Swift Logging ✅ Passed No added print/debugPrint/NSLog/Logger usage in the touched production Swift files; the new logging-like output is limited to tests and comments.
Cmux User-Facing Error Privacy ✅ Passed The new user-facing error is generic and avoids prohibited provider, flag, secret, or raw upstream details.
Cmux Swiftui State Layout ✅ Passed No prohibited SwiftUI state/layout patterns were introduced; the diff is backend/flag routing and the one new observable type uses modern @Observable, not legacy SwiftUI ownership.
Cmux Architecture Rethink ✅ Passed The lock is a required off-main snapshot bridge for nonisolated status reads, and the RPC gate is consolidated into one shared router; no symptom-patch lifecycle hacks or split ownership.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS: The PR only changes feature flags, mobile capabilities, RPC routing, and tests; it adds no user-visible standalone window/controller code or cmux.* identifier changes.
Cmux Source Artifacts ✅ Passed All changed paths are intentional source/test/localization files; no logs, caches, build outputs, or scratch dirs appear in the diff.
Cmux No Test Or Debug Seam In Production Source ✅ Passed No new test/debug seam in Sources; the DEBUG guards gate real flag defaults, and the new accessors/router are used by production callers.
Cmux No Ambient Global State ✅ Passed No new file-scope funcs or ambient globals; changes stay inside existing types, and the shared singleton predated the PR.
Title check ✅ Passed The title clearly matches the main change: gating the mobile diff viewer behind a remote feature flag by default.
Description check ✅ Passed The description covers summary and testing well; the demo video, review trigger, and checklist sections are not fully completed.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat-diffv-remote-flag

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.

@greptile-apps

greptile-apps Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds a remotely controlled, release-default-off gate for the mobile workspace diff viewer.

  • Registers and localizes the new PostHog-backed feature flag.
  • Mirrors effective flag values into a lock-protected snapshot for off-main capability responses.
  • Removes workspace.changes.v1 from host capabilities while disabled.
  • Routes all five mobile workspace-changes RPCs through a shared fail-closed guard.
  • Adds coverage for capability filtering and feature-flag resolution.

Confidence Score: 5/5

The PR appears safe to merge, with capability advertisement and RPC execution consistently gated by the same effective feature-flag state.

The shared feature-flag instance synchronously mirrors each resolution update into the off-main snapshot, status responses read that snapshot live, and the only production dispatcher for all five workspace-changes methods now passes through the guarded router.

Important Files Changed

Filename Overview
Sources/FeatureFlags.swift Adds the mobile-diff flag and keeps its authoritative MainActor resolution synchronized with a lock-protected off-main snapshot.
Sources/Mobile/MobileHostService+Capabilities.swift Conditionally removes the workspace-changes capability while preserving the rest of the advertised capability list.
Sources/TerminalController+MobileWorkspaceChanges.swift Adds one localized, fail-closed guard shared by all five workspace-changes RPC handlers.
Sources/TerminalController.swift Rewires the mobile RPC dispatcher so every workspace-changes request passes through the shared guard.
Resources/Localizable.xcstrings Supplies English and Japanese strings for the flag UI and disabled-capability error.
cmuxTests/MobileHostConnectionLifecycleTests.swift Covers capability inclusion and exclusion plus default and remote feature-flag resolution.

Sequence Diagram

sequenceDiagram
    participant PostHog
    participant Flags as CmuxFeatureFlags
    participant Host as MobileHostService
    participant Phone as iOS Client
    participant Router as Workspace Changes Router
    PostHog-->>Flags: Remote flag value
    Flags->>Flags: Update MainActor state and off-main snapshot
    Phone->>Host: mobile.host.status
    Host-->>Phone: Capabilities filtered by snapshot
    Phone->>Router: "mobile.workspace.changes.*"
    alt Flag enabled
        Router-->>Phone: Workspace diff response
    else Flag disabled
        Router-->>Phone: capability_disabled
    end
Loading

Reviews (1): Last reviewed commit: "Put the mobile diff viewer behind a remo..." | Re-trigger Greptile

Union of the CmuxFeatureFlags init: main's telemetry-gated control-plane
remote loader parameters plus this branch's off-main snapshot publisher.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@azooz2003-bit
azooz2003-bit merged commit e0cf205 into main Jul 27, 2026
5 checks passed
@azooz2003-bit
azooz2003-bit deleted the feat-diffv-remote-flag branch July 27, 2026 19:02
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