Skip to content

feat: add dev/release side-by-side app identity#922

Open
karkarl wants to merge 1 commit into
openclaw:mainfrom
karkarl:karkarl-dev-release-side-by-side-identity
Open

feat: add dev/release side-by-side app identity#922
karkarl wants to merge 1 commit into
openclaw:mainfrom
karkarl:karkarl-dev-release-side-by-side-identity

Conversation

@karkarl

@karkarl karkarl commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Motivation

Currently, dev and release builds of OpenClaw Companion cannot coexist on the same machine -- they share the same MSIX identity, data directory, registry keys, mutex, and protocol handler. This makes it impossible to test dev builds without disrupting a working release install (similar to how WinUI Gallery ships separate Dev and Release variants).

Approach

Introduces a compile-time AppIdentity class that switches all identity-sensitive constants based on a DEV_BUILD define. Debug builds automatically get DEV_BUILD set (or it can be forced via /p:DevBuild=true), giving them fully isolated:

  • Display name: "OpenClaw Companion (Dev)" / "OpenClaw Tray (Dev)"
  • MSIX package identity: OpenClaw.Companion.Dev
  • Data directory: %LOCALAPPDATA%\OpenClawTray-Dev
  • Auto-start registry key: OpenClawTray-Dev
  • Single-instance mutex: OpenClawTray-Dev
  • Protocol handler: openclaw-dev://
  • Installer AppId and install path: separate from release

Key design decisions

  • Used #if DEV_BUILD conditional compilation rather than runtime config so the identity is baked in at compile time and cannot drift.
  • Dev builds accept both openclaw-dev:// and openclaw:// deep links for compatibility, but only register openclaw-dev:// as their protocol handler (so they don't steal activation from release).
  • The installer uses Inno Setup preprocessor (/DDevBuild=1) to produce a completely separate installer with its own AppId, directory, mutex, and protocol registration.
  • Existing CI Alpha/Release manifest patching is unaffected -- the new PatchDevAppxManifestIdentity target only fires when DevBuild=true AND WindowsPackageType=MSIX.

Validation

  • build.ps1 passes (all 5 projects)
  • OpenClaw.Shared.Tests: 2691 passed, 31 skipped
  • OpenClaw.Tray.Tests: 1505 passed, 0 failed

Enable Dev and Release builds to coexist on the same machine (similar
to WinUI Gallery). Debug/DevBuild=true builds get:

- Display name: 'OpenClaw Companion (Dev)' / 'OpenClaw Tray (Dev)'
- MSIX package identity: OpenClaw.Companion.Dev
- Separate data directory: %LOCALAPPDATA%\OpenClawTray-Dev
- Separate auto-start registry key: OpenClawTray-Dev
- Separate single-instance mutex: OpenClawTray-Dev
- Separate protocol handler: openclaw-dev://
- Separate installer AppId and install directory

Add AppIdentity.cs with compile-time constants controlled by the
DEV_BUILD define (auto-set for Debug configuration, or explicitly
via /p:DevBuild=true).

Update csproj with PatchDevAppxManifestIdentity MSBuild target that
patches the MSIX manifest for dev MSIX builds.

Update installer.iss to support /DDevBuild=1 preprocessor flag for
producing a side-by-side dev installer.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@clawsweeper

clawsweeper Bot commented Jul 2, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed July 2, 2026, 7:05 PM ET / 23:05 UTC.

Summary
This PR adds compile-time dev/release app identity constants and threads variant-specific names through tray startup, installer/MSIX metadata, autostart, protocol handling, and tray tests.

Reproducibility: yes. Source inspection shows the failing paths: DEV_BUILD is enabled for Debug, but SettingsManager still resolves %APPDATA%\OpenClawTray and DeepLinkParser still accepts only openclaw://.

Review metrics: 1 noteworthy metric.

  • Identity surface touched: 10 files changed, 193 additions, 42 deletions. The diff spans installer, MSIX build targets, app startup, protocol handling, autostart, and tests, so merge safety depends on cross-surface behavior rather than one unit test path.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Make every settings, gateway, identity, MCP token, protocol, and manifest path variant-aware while preserving release defaults.
  • [P1] Add focused tests for release and dev identities, including openclaw:// and openclaw-dev:// parsing/registration behavior.
  • [P1] Add redacted real behavior proof showing release and dev builds installed or launched side by side without shared state.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body lists build and unit test results, but it does not include real current-head proof of release and dev installs running side by side; contributor action is needed with redacted screenshots, terminal output, logs, or a recording. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Mantis proof suggestion
A Windows visual/runtime proof would materially help because the change is about side-by-side desktop identity, tray naming, data roots, mutexes, and protocol links. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis visual task: after fixes, verify release and dev builds run side by side with separate data roots, mutexes, tray names, and protocol links.

Risk before merge

  • [P1] Dev builds can still read and write release settings, gateway records, device identities, and the MCP token because SettingsManager.SettingsDirectoryPath remains %APPDATA%\OpenClawTray.
  • [P1] Unpackaged Debug builds can overwrite the release openclaw:// protocol registration because DeepLinkHandler.RegisterUriScheme is still hardcoded to openclaw.
  • [P1] Dev installer/MSIX links using openclaw-dev:// currently reach a parser that only accepts openclaw://, so registered dev links can no-op.
  • [P1] The PR body reports build/test validation but does not provide current-head proof that release and dev installs actually run side by side without sharing state.

Maintainer options:

  1. Fix identity plumbing before merge (recommended)
    Route SettingsManager, GatewayRegistry, MCP token, protocol registration, parser, and MSIX manifest behavior through one variant-aware contract, with tests for release and dev defaults.
  2. Require side-by-side proof
    After code fixes, require Windows proof showing release and dev running together with separate data roots, mutexes, protocol handlers, and start-menu links.
  3. Pause the feature direction
    If maintainers do not want a baked-in dev app identity surface, pause or close this PR and rework it under a release-engineering plan.

Next step before merge

  • [P1] This needs contributor repairs plus real Windows side-by-side proof; automation cannot supply the contributor's runtime proof for this PR.

Security
Needs attention: The diff touches credential-bearing data paths and protocol registration, and the proposed split still allows dev builds to access release credentials/state.

Review findings

  • [P1] Isolate the settings store before claiming dev separation — src/OpenClaw.Tray.WinUI/App.xaml.cs:248
  • [P1] Do not let Debug builds steal the release protocol handler — src/OpenClaw.Tray.WinUI/OpenClaw.Tray.WinUI.csproj:19-20
  • [P1] Teach the parser about the dev protocol — src/OpenClaw.Tray.WinUI/App.xaml.cs:340-341
Review details

Best possible solution:

Land the side-by-side feature only after every identity-sensitive path is variant-aware, release defaults remain unchanged, and current-head proof shows release and dev installs running independently.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection shows the failing paths: DEV_BUILD is enabled for Debug, but SettingsManager still resolves %APPDATA%\OpenClawTray and DeepLinkParser still accepts only openclaw://.

Is this the best way to solve the issue?

No. Centralizing identity constants is a reasonable direction, but this implementation must cover the storage, credential, parser, registry, and manifest surfaces before it is the maintainable fix.

Full review comments:

  • [P1] Isolate the settings store before claiming dev separation — src/OpenClaw.Tray.WinUI/App.xaml.cs:248
    This only changes IdentityDataPath, but _settings = new SettingsManager() and GatewayRegistry(SettingsManager.SettingsDirectoryPath, ...) still use SettingsManager.GetDefaultSettingsDirectory(), which is hardcoded to %APPDATA%\OpenClawTray. A dev build will therefore share release settings, gateways.json, device identities, and the MCP token, defeating the central side-by-side guarantee and risking release credentials/state.
    Confidence: 0.96
  • [P1] Do not let Debug builds steal the release protocol handler — src/OpenClaw.Tray.WinUI/OpenClaw.Tray.WinUI.csproj:19-20
    Debug builds now become DEV_BUILD by default, but unpackaged startup still calls DeepLinkHandler.RegisterUriScheme(), and that method writes Software\Classes\openclaw unconditionally. Running a dev build from F5/local debug can overwrite the release openclaw:// handler instead of registering openclaw-dev://.
    Confidence: 0.93
  • [P1] Teach the parser about the dev protocol — src/OpenClaw.Tray.WinUI/App.xaml.cs:340-341
    The PR accepts and registers openclaw-dev://, but DeepLinkHandler.Handle still delegates to OpenClaw.Shared.DeepLinkParser, whose scheme constant is only openclaw://. Dev start-menu shortcuts and MSIX protocol activations can pass the startup filter and then no-op when the parser returns null.
    Confidence: 0.95
  • [P2] Avoid leaving dev protocol in the source manifest — src/OpenClaw.Tray.WinUI/OpenClaw.Tray.WinUI.csproj:242
    PatchAppxManifestIdentity writes Package.appxmanifest in place, including the protocol name. A later release/alpha package build in the same worktree only patches identity, version, and display name, so it can retain openclaw-dev in the manifest unless this target patches a generated copy or restores the release protocol.
    Confidence: 0.86

Overall correctness: patch is incorrect
Overall confidence: 0.93

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 74604aebafef.

Label changes

Label changes:

  • add P2: This is a useful feature PR with bounded scope, but it has blocking compatibility and credential-store issues before merge.
  • add merge-risk: 🚨 compatibility: Merging can make dev builds collide with release settings/protocol behavior instead of running cleanly side by side.
  • add merge-risk: 🚨 auth-provider: The dev identity split misses gateway records, device identity files, and MCP token paths that carry credentials.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body lists build and unit test results, but it does not include real current-head proof of release and dev installs running side by side; contributor action is needed with redacted screenshots, terminal output, logs, or a recording. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P2: This is a useful feature PR with bounded scope, but it has blocking compatibility and credential-store issues before merge.
  • merge-risk: 🚨 compatibility: Merging can make dev builds collide with release settings/protocol behavior instead of running cleanly side by side.
  • merge-risk: 🚨 auth-provider: The dev identity split misses gateway records, device identity files, and MCP token paths that carry credentials.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body lists build and unit test results, but it does not include real current-head proof of release and dev installs running side by side; contributor action is needed with redacted screenshots, terminal output, logs, or a recording. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

Security concerns:

  • [medium] Dev builds still share credential-bearing app data — src/OpenClaw.Tray.WinUI/App.xaml.cs:248
    SettingsManager, GatewayRegistry, device identity files, and the MCP bearer token still resolve through %APPDATA%\OpenClawTray, so a dev build can read or mutate release gateway credentials despite claiming isolated identity.
    Confidence: 0.94
  • [medium] Debug builds can hijack release deep links — src/OpenClaw.Tray.WinUI/OpenClaw.Tray.WinUI.csproj:19
    Because Debug now sets DEV_BUILD but RegisterUriScheme() is still hardcoded to openclaw, a dev run can redirect release deep links to a developer build.
    Confidence: 0.9

Acceptance criteria:

  • [P1] ./build.ps1.
  • [P1] dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore.
  • [P1] dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore.
  • [P1] Windows proof: release and dev builds installed or launched side by side, separate settings/gateway/token folders, separate mutexes, release openclaw:// still routes to release, dev openclaw-dev:// routes to dev.

What I checked:

  • AGENTS policy read and applied: AGENTS.md was read fully; this PR touches tray UX/App.xaml.cs and setup/package identity, so the repository's proof expectations and App.xaml.cs guardrails apply. (AGENTS.md:1, 74604aebafef)
  • PR makes Debug builds dev identity by default: The PR sets DevBuild=true for Debug and defines DEV_BUILD, changing default local developer builds to the new identity path. (src/OpenClaw.Tray.WinUI/OpenClaw.Tray.WinUI.csproj:19, d15a393cd589)
  • Settings store remains release path: On the PR head, SettingsManager still defaults to %APPDATA%\OpenClawTray, while App and GatewayRegistry use SettingsManager.SettingsDirectoryPath for settings, gateways.json, identities, and MCP token paths. (src/OpenClaw.Tray.WinUI/Services/SettingsManager.cs:187, d15a393cd589)
  • Dev protocol is accepted before parsing but parser is unchanged: The PR accepts AppIdentity.ProtocolScheme in App.xaml.cs, but DeepLinkParser on the same head still hardcodes openclaw:// and returns null for openclaw-dev://. (src/OpenClaw.Shared/DeepLinkParser.cs:13, d15a393cd589)
  • Unpackaged protocol registration remains hardcoded: Current main's DeepLinkHandler registers HKCU Software\Classes\openclaw and logs openclaw://; the PR does not change this file, so Debug dev builds can still overwrite the release handler. (src/OpenClaw.Tray.WinUI/Services/DeepLinkHandler.cs:13, 74604aebafef)
  • Release CI manifest patch does not reset protocol: The release/alpha MSIX workflow patches identity, version, and display names, but not uap:Protocol, so a dev MSIX manifest mutation can persist in a reused local worktree. (.github/workflows/ci.yml:527, 74604aebafef)

Likely related people:

  • shanselman: Authored/merged the current release commit that owns the App.xaml.cs, installer, DeepLinkHandler, SettingsManager, and MSIX packaging surfaces most affected by this PR. (role: recent feature owner and merger; confidence: high; commits: 4166e0fd63f8; files: src/OpenClaw.Tray.WinUI/App.xaml.cs, src/OpenClaw.Tray.WinUI/Services/SettingsManager.cs, src/OpenClaw.Tray.WinUI/Services/DeepLinkHandler.cs)
  • bkudiess: Recently changed connection snapshot behavior merged on current main and appears in SettingsDirectoryPath/connection-adjacent history relevant to gateway state isolation. (role: recent adjacent contributor; confidence: medium; commits: 74604aebafef, 62533e2901bd; files: src/OpenClaw.Tray.WinUI/App.xaml.cs, src/OpenClaw.Tray.WinUI/Services/SettingsManager.cs, src/OpenClaw.Tray.WinUI/Services/NodeService.cs)
  • ranjesh: History around SettingsDirectoryPath and gateway credential flow includes reconnect, bootstrap token, and registry migration work that this PR's identity split must preserve. (role: connection and gateway registry contributor; confidence: medium; commits: c582509c6b6a, d8ff294d313d, f575edc3da21; files: src/OpenClaw.Tray.WinUI/App.xaml.cs, src/OpenClaw.Tray.WinUI/Services/SettingsManager.cs)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. labels Jul 2, 2026
@shanselman

Copy link
Copy Markdown
Contributor

Hi Karen — bot review with Scott's maintainer feedback included. This is a really useful feature direction, and the central AppIdentity idea is a good instinct. The reason we cannot take it yet is that side-by-side identity has to be all-or-nothing: if even one credential, settings, protocol, or startup surface is still shared, dev builds can affect a user's release install.

The blockers I see:

  1. openclaw-dev:// is registered/accepted, but still rejected by the parser. The PR wires dev links into installer/startup handling, but OpenClaw.Shared.DeepLinkParser still only accepts openclaw://. That means dev Start Menu links and protocol activations like openclaw-dev://setup can pass the outer startup check and then no-op when parsed.

  2. Dev/release data isolation is incomplete. The PR adds dev DataPath/IdentityDataPath, but many important paths still flow through SettingsManager.SettingsDirectoryPath, which remains %APPDATA%\OpenClawTray. That includes settings, gateways.json, per-gateway/device identity migration paths, MCP token path (NodeService.McpTokenPath), and other credential/state surfaces. A dev build can still read or mutate release gateway/MCP state.

  3. Auto-start is not fully split. The registry Run value is variant-aware, but the scheduled-task fallback uses the shared task name in WindowsStartupTaskRegistration, so dev and release can overwrite each other's auto-start registration.

  4. The MSIX manifest patch mutates the source manifest in place. The dev identity target writes directly to Package.appxmanifest. That can leave a dirty worktree or accidentally carry a dev protocol/name into a later release build if not restored or generated into an intermediate file.

  5. There are a few smaller cleanup items. For example MyAutoStartName is defined in installer.iss but not used, and a few log strings still hardcode openclaw://.

Bot-ready repair prompt you can give your coding agent:

Please repair PR #922 for OpenClaw side-by-side dev/release identity. The goal is complete isolation between release and dev builds, while preserving release defaults.

Requirements:
1. Define one variant-aware identity/data contract and route all identity-sensitive paths through it. Cover settings, logs, gateways.json, gateway identity directories, legacy identity migration paths, MCP token path, voice/model/cache paths, diagnostics/config folder links, and any other app-data paths currently using SettingsManager.SettingsDirectoryPath or hardcoded OpenClawTray.
2. Make deep-link handling variant-aware end to end. `openclaw-dev://` must parse and route correctly in dev builds. Release must continue to use `openclaw://`. If dev intentionally accepts release links as an alias, normalize that intentionally and add tests.
3. Make unpackaged protocol registration variant-aware. Debug/dev builds must not overwrite the release `openclaw://` handler.
4. Make scheduled-task auto-start names variant-aware, including cleanup/migration behavior for existing release task names.
5. Fix MSIX manifest patching so dev identity/protocol changes do not dirty or permanently mutate source-controlled `Package.appxmanifest`. Patch a generated copy or restore the original values after packaging.
6. Add focused tests for release and dev identities: data roots, protocol parsing/registration strings, autostart names, installer protocol/mutex/AppId/start-menu links, and MCP token path separation.
7. Add `## Real behavior proof` to the PR body showing release and dev builds installed or launched side-by-side with separate data roots, mutexes, protocol handlers, and auto-start names.
8. Run .\build.ps1, Shared tests, Tray tests, and any targeted installer/identity tests.

This is not a rejection of the feature. It is the right kind of feature, but the boundary is security/compatibility-sensitive, so we need it complete before merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants