Skip to content

fix(macos): preserve device identity storage - #101105

Merged
steipete merged 4 commits into
openclaw:mainfrom
yetval:fix/macos-device-identity-preserve-existing
Jul 7, 2026
Merged

steipete merged 4 commits into
openclaw:mainfrom
yetval:fix/macos-device-identity-preserve-existing

Conversation

@yetval

@yetval yetval commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Summary

The macOS identity store no longer chooses App Group storage unless the running bundle has the matching com.apple.security.application-groups entitlement. Unentitled macOS app builds fall back to the legacy Application Support identity directory instead of repeatedly using an App Group path they cannot reliably persist.

To keep that fallback upgrade-safe, an unentitled build now migrates a readable existing App Group identity/device.json into the selected legacy store before generating a fresh identity. The same installation keeps its device id across the storage switch instead of presenting a new one.

Existing unrecognized identity/device.json files are also preserved in both the Swift and TypeScript loaders. The loader can still return a fresh in-memory identity when it cannot decode the file, but it no longer replaces existing identity bytes unless it is creating a missing file or performing a proven safe normalization.

Root cause

apps/shared/OpenClawKit/Sources/OpenClawKit/DeviceIdentity.swift selected App Group storage whenever FileManager.containerURL(forSecurityApplicationGroupIdentifier:) returned a URL:

if let appGroupStateDirURL {
    return appGroupStateDirURL
}

On affected shipped macOS builds, that could select the App Group container even though the bundle did not carry the matching App Groups entitlement. When persistence at that path failed, loadOrCreate returned generated identities, leading to pairing churn.

The Swift and TypeScript loadOrCreate implementations also treated an existing but unrecognized identity file like a missing file:

case .unknown:
    break

That path generated and saved a new identity over existing bytes.

Fix

Swift now checks the running bundle entitlement before selecting App Group identity storage:

if appGroupStateDirAvailable, let appGroupStateDirURL {
    return appGroupStateDirURL
}

When the entitlement is absent, the existing legacy Application Support fallback wins. To make that switch upgrade-safe, an unentitled build first tries to migrate a readable App Group identity into the legacy store:

if let migrated = self.migratedIdentity(from: migrationSourceURL, to: url) {
    return migrated
}

migrationSourceURL is the App Group identity/<file> and is non-nil only when the entitlement is absent and the App Group container resolves. Migration reads and re-saves an already-valid identity, so the same device id survives while future writes go to the store the build can actually persist. Both loaders still preserve existing unrecognized identity files and only persist a generated identity when the file is missing.

Why this is the right boundary

The storage decision belongs in DeviceIdentityPaths, because every Swift caller reaches identity storage through that path resolver. The no-overwrite guard and the one-time migration belong in the identity loaders, because Swift and TypeScript both own identity/device.json and must preserve existing state consistently. OpenClawKit has no openclaw doctor --fix surface, so the bounded migration at the load boundary is the owner for this preservation.

This resolves the ClawSweeper P1 (DeviceIdentity.swift: preserve existing App Group identities before falling back) by implementing the recommended maintainer option: preserve/migrate a valid readable App Group identity into the chosen legacy store before an unentitled build switches to legacy storage. It does not accept a one-time re-pair and does not require provisioning the app with App Groups.

The TypeScript node loader has no App Group container (resolveStateDir in src/config/paths.ts never selects one), so this migration is macOS-only; the TypeScript side keeps only the no-overwrite guard.

Verification

  • swift test --filter DeviceIdentityStoreTests builds OpenClawKit and passes all 13 cases (10 existing plus 3 new: migrate-on-fallback, keep-existing-legacy, no-source-when-entitled) on Apple Swift 6.3.2 / arm64 macOS.
  • git diff --check passed.
  • New Swift regression fails on pristine 54f0b2589b11 (pre-migration head) and passes with this patch.
  • The App Group container path requires a provisioned signed macOS bundle, so the migration driver injects the App Group directory; the entitlement/container system APIs themselves are not exercisable from an unsigned local build.

Real behavior proof

Behavior addressed: An unentitled macOS build that already held a readable App Group identity/device.json presented a new device id after upgrading, because the entitlement-aware fallback read empty legacy storage and minted a fresh identity.
Real environment tested: The production DeviceIdentityStore.loadOrCreate and DeviceIdentityPaths.appGroupMigrationSourceURL were driven on a real on-disk App Group + legacy layout on the pre-migration head 54f0b2589b11 behavior (single-argument loader, unchanged by this patch) and on PR head eab93b9f6b7a3e1569107b9d28ae27106d940886, on Apple Swift 6.3.2 arm64 macOS. Only the App Group container location was injected; the loader, decoder, save, and migration selection are the production functions.
Exact steps or command run after this patch: Wrote a valid existing identity/device.json under the App Group directory, then invoked the production loader for an unentitled build with empty legacy storage: the pre-fix path via DeviceIdentityStore.loadOrCreate(fileURL:) and the fixed path via DeviceIdentityStore.loadOrCreate(fileURL:migrationSourceURL:), and read back the returned device id and files. Driven under swift test --filter AppGroupMigrationProofScratch, capturing the returned device id values to disk.
Evidence after fix:

existingAppGroupDeviceId=56475aa75463474c0285df5dbf2bcab73da651358839e9b77481b2eab107708c
# BEFORE: unentitled build falls back to empty legacy storage (pre-fix path)
before.returnedDeviceId=92bed52befa0e1ea4975a47d7fef066c50d103d9ff0992f8b45b7bb83bfee63a
before.matchesExistingAppGroupIdentity=no
before.mintedNewLegacyFile=yes
# AFTER: unentitled build migrates the readable app group identity into legacy
after.returnedDeviceId=56475aa75463474c0285df5dbf2bcab73da651358839e9b77481b2eab107708c
after.matchesExistingAppGroupIdentity=yes
after.survivesLegacyReload=yes
after.appGroupFileUnchanged=yes

Observed result after fix: On identical inputs the pre-fix path returns a new device id and writes a fresh legacy file, while the fixed path returns the same device id the install already had, keeps it across a reload, and leaves the App Group file byte-for-byte unchanged.
What was not tested: The full OpenClaw.app relaunch and gateway restart lifecycle was not run here (that needs the built signed app plus a running gateway); the entitlement read and container resolution themselves are now exercised on a real code-signed binary in the section below. No full pnpm build was run because the change is Swift-only.

Signed macOS entitlement proof (real code signature)

The production entitlement gate (DeviceIdentityPaths.hasAppGroupEntitlement, i.e. SecTaskCreateFromSelf + SecTaskCopyValueForEntitlement) was compiled into a Mach-O binary and ad-hoc code-signed twice with codesign --sign - --entitlements: once carrying com.apple.security.application-groups = [group.ai.openclawfoundation.app.shared] and once with an empty entitlements dict. SecTaskCopyValueForEntitlement reads the running binary's own embedded entitlements, so the real signature drives the result. FileManager.containerURL(forSecurityApplicationGroupIdentifier:) was also called live.

Embedded entitlements read back from each signature (codesign -d --entitlements):

entitled binary   -> com.apple.security.application-groups = [group.ai.openclawfoundation.app.shared]
unentitled binary -> {}  (no application-groups)

Live output from the signed binaries:

# unentitled build (signed without the entitlement)
hasAppGroupEntitlement=no
appGroupContainerResolved=yes
resolverWouldSelect=legacy
migrationSourceActive=yes
# entitled build (signed with the entitlement)
hasAppGroupEntitlement=yes
appGroupContainerResolved=yes
resolverWouldSelect=appGroup
migrationSourceActive=no

On a real signature the entitlement read flips as expected: an unentitled build resolves to legacy storage with the App Group migration source active, and an entitled build keeps App Group storage. The App Group container resolves on macOS, so the migration source is reachable for a real unentitled build, not only under an injected flag. Combined with the loader proof above, the full chain is covered end to end except the OpenClaw.app UI relaunch and gateway restart.

Related

Fixes #99283.
Related to #99522.
Distinct from #79715, which fixed Swift and TypeScript schema compatibility but did not make App Group selection entitlement-aware.

@yetval

yetval commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-run

@clawsweeper

clawsweeper Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper

clawsweeper Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 6, 2026, 11:46 PM ET / 03:46 UTC.

Summary
The branch makes macOS device identity storage entitlement-aware, migrates valid App Group identity and auth files to legacy storage on fallback, and preserves unrecognized identity files in the Swift and TypeScript loaders with regression coverage.

PR surface: Source +4, Tests +28, Other +305. Total +337 across 5 files.

Reproducibility: yes. source inspection plus the linked reports give a high-confidence reproduction path: current main and v2026.6.11 prefer App Group storage before legacy while shipped macOS entitlements omit App Groups. I did not independently run a full signed app relaunch in this read-only review.

Review metrics: 3 noteworthy metrics.

  • Identity storage behavior: 1 fallback decision changed, 1 migration source added. Changing macOS identity store selection and fallback migration affects existing installs during upgrade.
  • Persisted identity/auth artifacts: 2 stored files migrated copy-if-missing. The branch now carries both identity and device-auth state so preserving the deviceId does not still force re-pairing.
  • Identity overwrite contract: 2 loaders preserve unrecognized files. Swift and TypeScript now avoid overwriting unknown existing identity bytes, which changes persisted-state recovery behavior.

Stored data model
Persistent data-model change detected: serialized state: src/infra/device-identity.state-dir.test.ts, serialized state: src/infra/device-identity.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #99283
Summary: This PR is the candidate fix for the formally linked macOS App Group identity rewrite issue, and it overlaps the same root cause reported in the macOS 27 App Group entitlement issue.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

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

Rank-up moves:

  • none.

Risk before merge

  • [P1] The PR intentionally moves shipped unentitled macOS app builds from App Group storage to legacy Application Support storage, so the entitlement gate and one-time migration remain upgrade-sensitive even with green checks.
  • [P1] The branch now migrates identity and auth together, but the complete signed OpenClaw.app relaunch plus gateway restart path remains release-validation proof rather than proof supplied in this review.

Maintainer options:

  1. Land with targeted storage proof (recommended)
    Accept the signed entitlement output plus real-file migration proof and merge after the normal exact-head checks because the fix now preserves both identity and auth state.
  2. Require full app lifecycle proof
    Before merge, run a signed OpenClaw.app relaunch and gateway restart on macOS to prove the same deviceId and stored token survive the complete workflow.
  3. Pause for App Groups provisioning
    If maintainers prefer shipping the App Groups entitlement instead of legacy fallback, pause this PR and route the release-signing/provisioning choice separately.

Next step before merge

  • No automated repair job is needed; the PR has no blocking findings and the remaining action is maintainer merge and release-validation handling.

Security
Cleared: No concrete supply-chain or security regression was found; the diff changes native identity/auth storage logic and tests without adding dependencies, workflows, downloads, or broader secret access.

Review details

Best possible solution:

Land the entitlement-aware fallback with identity plus auth copy-if-missing migration after exact-head merge handling, and keep a full signed OpenClaw.app relaunch plus gateway restart as release validation.

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

Yes, source inspection plus the linked reports give a high-confidence reproduction path: current main and v2026.6.11 prefer App Group storage before legacy while shipped macOS entitlements omit App Groups. I did not independently run a full signed app relaunch in this read-only review.

Is this the best way to solve the issue?

Yes, this is the best fix direction: gate App Group storage on the actual entitlement, fall back to writable legacy storage for unentitled builds, and migrate valid identity/auth state before minting anything new. Provisioning App Groups alone is heavier and does not cover shipped unentitled installs already in the field.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 780ca1d25315.

Label changes

Label justifications:

  • P1: The PR fixes a shipped macOS identity regression that can break pairing, node continuity, and stored device-token reuse for real users.
  • merge-risk: 🚨 compatibility: Merging changes the macOS identity storage target and one-time migration behavior for existing unentitled app installs.
  • merge-risk: 🚨 session-state: Device identity stability controls node identity and session continuity across reconnects and app restarts.
  • merge-risk: 🚨 auth-provider: The migrated auth store contains device tokens keyed by the preserved device identity, so mistakes can force re-pairing or token loss.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body and follow-up comments provide sufficient after-fix live output for the proof gate: real-file loader migration output, signed Mach-O entitlement/container output, and current-head maintainer validation for the auth-store fixup.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body and follow-up comments provide sufficient after-fix live output for the proof gate: real-file loader migration output, signed Mach-O entitlement/container output, and current-head maintainer validation for the auth-store fixup.
Evidence reviewed

PR surface:

Source +4, Tests +28, Other +305. Total +337 across 5 files.

View PR surface stats
Area Files Added Removed Net
Source 1 4 0 +4
Tests 2 40 12 +28
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 2 311 6 +305
Total 5 355 18 +337

What I checked:

Likely related people:

  • steipete: Peter Steinberger has the heaviest sampled history across the device identity/auth and macOS signing surfaces, originally introduced role-scoped device auth, and authored the current-head fixup that migrates auth with identity. (role: feature-history contributor and PR fixup author; confidence: high; commits: 73e9e787b4df, d8cc7db5e6cc, cb1ff87006f2; files: apps/shared/OpenClawKit/Sources/OpenClawKit/DeviceIdentity.swift, apps/shared/OpenClawKit/Sources/OpenClawKit/DeviceAuthStore.swift, scripts/codesign-mac-app.sh)
  • BunsDev: BunsDev authored the merged shared Swift/TypeScript device identity schema compatibility fix that established much of the invalid-file preservation behavior this PR extends. (role: prior related fix author; confidence: high; commits: 66fbec10c99f, 1c10b0dc67a5, dafbdb6f20fd; files: apps/shared/OpenClawKit/Sources/OpenClawKit/DeviceIdentity.swift, apps/shared/OpenClawKit/Tests/OpenClawKitTests/DeviceIdentityStoreTests.swift, src/infra/device-identity.ts)
  • joshavant: joshavant recently changed the shared Swift identity/auth profile split and then removed an iOS identity migration path in the same storage area. (role: recent identity-profile contributor; confidence: high; commits: f13dc76ba181, 99e69e16b7b8; files: apps/shared/OpenClawKit/Sources/OpenClawKit/DeviceIdentity.swift, apps/shared/OpenClawKit/Sources/OpenClawKit/DeviceAuthStore.swift, apps/shared/OpenClawKit/Sources/OpenClawKit/GatewayChannel.swift)
  • Josh Lehman: Josh Lehman authored the recent refactor that created the current OpenClawKit DeviceIdentity and DeviceAuthStore file locations now being patched. (role: recent refactor owner; confidence: medium; commits: cb0d8a1294ea; files: apps/shared/OpenClawKit/Sources/OpenClawKit/DeviceIdentity.swift, apps/shared/OpenClawKit/Sources/OpenClawKit/DeviceAuthStore.swift, apps/shared/OpenClawKit/Tests/OpenClawKitTests/DeviceIdentityStoreTests.swift)
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.
Review history (3 earlier review cycles)
  • reviewed 2026-07-06T19:40:06.399Z sha 528ae815c9bf6f2402204b1a1c2151bb192c187e :: needs real behavior proof before merge. :: [P1] Preserve existing App Group identities before falling back
  • reviewed 2026-07-06T21:10:23.783Z sha eab93b9 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-06T21:56:42.718Z sha eab93b9 :: needs maintainer review before merge. :: none

@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. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. labels Jul 6, 2026
@yetval

yetval commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the P1 upgrade-preservation finding and refreshed the proof for head 711afa6454f066784e14ac10366c655f19ba0b1a.

What changed since the last review

  • Implemented the recommended maintainer option (preserve/migrate). An unentitled macOS build now migrates a readable existing App Group identity/device.json into the selected legacy store before generating a new identity, so the same install keeps its device id across the storage switch. New helper DeviceIdentityPaths.appGroupMigrationSourceURL supplies the App Group source only when the entitlement is absent; DeviceIdentityStore.loadOrCreate migrates it at the load boundary (OpenClawKit has no openclaw doctor --fix surface, so the load boundary is the owner).
  • Added 3 Swift regression cases: migrate-on-fallback, keep-existing-legacy (migration never clobbers an existing legacy identity), and no-source-when-entitled. All 13 DeviceIdentityStoreTests pass on Apple Swift 6.3.2.
  • The TypeScript node loader has no App Group container (resolveStateDir in src/config/paths.ts), so the migration is macOS-only; the TS side keeps only the no-overwrite guard.

Real behavior proof (refreshed, in PR body)
Drove the production DeviceIdentityStore.loadOrCreate on a real on-disk App Group + legacy layout, identical inputs:

existingAppGroupDeviceId=56475aa75463474c0285df5dbf2bcab73da651358839e9b77481b2eab107708c
# BEFORE (pre-migration path): unentitled build reads empty legacy storage
before.returnedDeviceId=92bed52befa0e1ea4975a47d7fef066c50d103d9ff0992f8b45b7bb83bfee63a
before.matchesExistingAppGroupIdentity=no
# AFTER (this patch): unentitled build migrates the readable app group identity
after.returnedDeviceId=56475aa75463474c0285df5dbf2bcab73da651358839e9b77481b2eab107708c
after.matchesExistingAppGroupIdentity=yes
after.survivesLegacyReload=yes
after.appGroupFileUnchanged=yes

Known gap: no signed macOS app relaunch or gateway restart on a provisioned App Group container was run here; the entitlement/container system APIs need a signed bundle. The driver injects only the App Group directory location; the loader, decoder, save, and migration selection are the production functions.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

yetval added 3 commits July 6, 2026 16:58
Avoid selecting App Group identity storage when the running app lacks the matching application-groups entitlement, so unentitled macOS builds fall back to the legacy writable identity directory. Existing unrecognized identity files are now preserved instead of being replaced by a newly generated identity, matching the no-overwrite behavior already used for recognized invalid identity data.
Limit the App Group entitlement probe to macOS builds so iOS keeps its existing App Group path behavior. Update the state-dir identity regression to assert the new no-overwrite contract for invalid identity-shaped files.
Unentitled macOS builds now migrate a readable App Group device identity
into the selected legacy store before falling back, so an upgrade preserves
the existing device id instead of minting a new one.
@yetval
yetval force-pushed the fix/macos-device-identity-preserve-existing branch from 711afa6 to eab93b9 Compare July 6, 2026 20:59
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 6, 2026
@yetval

yetval commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the rank-up move: added a real code-signed macOS entitlement proof for head eab93b9f6b7a3e1569107b9d28ae27106d940886.

The prior proof injected the appGroupStateDirAvailable flag; this exercises the actual system-API boundary the review said "was not run here". The production entitlement gate (SecTaskCreateFromSelf + SecTaskCopyValueForEntitlement) was compiled into a Mach-O binary and ad-hoc code-signed twice (codesign --sign - --entitlements). SecTaskCopyValueForEntitlement reads the running binary's own embedded entitlements, so the real signature drives the result. FileManager.containerURL(forSecurityApplicationGroupIdentifier:) was called live.

Embedded entitlements (codesign -d --entitlements, both signatures valid on disk):

entitled binary   -> com.apple.security.application-groups = [group.ai.openclawfoundation.app.shared]
unentitled binary -> {}  (no application-groups)

Live output:

# unentitled build (signed without the entitlement)
hasAppGroupEntitlement=no
appGroupContainerResolved=yes
resolverWouldSelect=legacy
migrationSourceActive=yes
# entitled build (signed with the entitlement)
hasAppGroupEntitlement=yes
appGroupContainerResolved=yes
resolverWouldSelect=appGroup
migrationSourceActive=no

So on a real signature the entitlement read flips exactly as the resolver expects: an unentitled build routes to legacy with the App Group migration source active; an entitled build keeps App Group storage. The App Group container resolves on macOS, so the migration source is genuinely reachable for a real unentitled build, not only under an injected flag.

Coverage now: the real SecTask entitlement read and container resolution are exercised on a code-signed binary (this comment), and the production loader migration preserving the same device id is exercised on real files (PR body). The only remaining gap is the OpenClaw.app UI relaunch plus gateway restart, which needs the built signed app and a running gateway. Given patch quality is platinum hermit and the correctness finding is resolved, this may fit maintainer option 2 (accept the targeted proof and own the app-relaunch coverage for follow-up) - happy to run the full app/gateway proof if someone can point me at a signed build.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 6, 2026
…back

The app group to legacy identity migration now carries the sibling
device-auth store file, so an unentitled build keeps its stored device
tokens (keyed by the migrated deviceId) instead of re-pairing. The
process-immutable entitlement check is resolved once per process rather
than creating a SecTask on every state-dir lookup, and the migration
source API returns an identity+auth pair struct.
@steipete

steipete commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Maintainer fixup pushed: cb1ff87 (thanks @yetval — the entitlement gate + migration direction is right and stays as designed).

What changed

  • Device-auth store now migrates with the identity. DeviceAuthStore resolves the same state dir (DeviceAuthStore.fileURLDeviceIdentityPaths.stateDirURL()), so the entitlement-aware fallback moves auth storage too. Migrating only identity/device.json kept the deviceId stable but dropped the stored device tokens, forcing a needless re-pair — the unresolved P1 risk from the ClawSweeper review. migratedIdentity now copies the sibling auth file (device-auth.json per profile) into the selected store, copy-if-missing so an existing destination auth store is never clobbered. A deviceId-mismatched copy stays inert behind DeviceAuthStore.loadToken's deviceId check, so no extra validation is needed.
  • Entitlement check cached per process. SecTaskCreateFromSelf ran on every stateDirURL() resolution — every identity load and every auth read/write. Entitlements are baked into the code signature, so the gate is now a one-time static let.
  • API reshaped: appGroupMigrationSourceURL(identityFileName:)appGroupMigrationSource(profile:) returning an identity+auth pair; the duplicate loadOrCreate overload collapsed into a default parameter; duplicated switch arms merged; invariant comments added (why the gate is macOS-only, why migration is one-time, why auth must move with identity).

Upgrade-cohort check (why the rest of the design is kept as-is): every shipped macOS build is unentitled (scripts/codesign-mac-app.sh signs a fixed entitlements plist without application-groups), so this PR flips the whole install base to legacy storage. The legacy→App Group copy-migration added in f13dc76 was removed the same day in 99e69e1 and both first shipped together in v2026.6.10, so it never ran in a release: installs since v2026.6.10 have App Group state only (migration + auth copy covers them), and older installs that upgraded through v2026.6.10 keep their pre-June legacy identity+auth pair, which "existing legacy identity wins" reunites correctly.

Proof (fixup head)

  • swift test --filter DeviceIdentityStoreTests: 14/14 pass (12 prior cases, migrate case extended to assert the auth file arrives byte-identical in legacy and the App Group copy is untouched, plus a new no-clobber case for an existing destination auth store).
  • node scripts/run-vitest.mjs src/infra/device-identity.test.ts src/infra/device-identity.state-dir.test.ts: 13/13 pass (TS change is comment-only).
  • swiftformat --lint --config config/swiftformat clean on both changed Swift files (also fixes a pre-existing redundantThrows in the new test).
  • Structured second-model review (Codex/gpt-5.5) of the fixup: no actionable findings.

@steipete
steipete merged commit 7900ff4 into openclaw:main Jul 7, 2026
161 of 162 checks passed
@steipete

steipete commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Merged via squash.

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 7, 2026
* fix(macos): preserve device identity storage

Avoid selecting App Group identity storage when the running app lacks the matching application-groups entitlement, so unentitled macOS builds fall back to the legacy writable identity directory. Existing unrecognized identity files are now preserved instead of being replaced by a newly generated identity, matching the no-overwrite behavior already used for recognized invalid identity data.

* fix(macos): repair identity CI coverage

Limit the App Group entitlement probe to macOS builds so iOS keeps its existing App Group path behavior. Update the state-dir identity regression to assert the new no-overwrite contract for invalid identity-shaped files.

* fix(macos): migrate existing app group identity on legacy fallback

Unentitled macOS builds now migrate a readable App Group device identity
into the selected legacy store before falling back, so an upgrade preserves
the existing device id instead of minting a new one.

* fix(macos): migrate device auth store with identity on app group fallback

The app group to legacy identity migration now carries the sibling
device-auth store file, so an unentitled build keeps its stored device
tokens (keyed by the migrated deviceId) instead of re-pairing. The
process-immutable entitlement check is resolved once per process rather
than creating a SecTask on every state-dir lookup, and the migration
source API returns an identity+auth pair struct.

---------

Co-authored-by: Peter Steinberger <steipete@gmail.com>
sheyanmin pushed a commit to sheyanmin/openclaw that referenced this pull request Jul 8, 2026
* fix(macos): preserve device identity storage

Avoid selecting App Group identity storage when the running app lacks the matching application-groups entitlement, so unentitled macOS builds fall back to the legacy writable identity directory. Existing unrecognized identity files are now preserved instead of being replaced by a newly generated identity, matching the no-overwrite behavior already used for recognized invalid identity data.

* fix(macos): repair identity CI coverage

Limit the App Group entitlement probe to macOS builds so iOS keeps its existing App Group path behavior. Update the state-dir identity regression to assert the new no-overwrite contract for invalid identity-shaped files.

* fix(macos): migrate existing app group identity on legacy fallback

Unentitled macOS builds now migrate a readable App Group device identity
into the selected legacy store before falling back, so an upgrade preserves
the existing device id instead of minting a new one.

* fix(macos): migrate device auth store with identity on app group fallback

The app group to legacy identity migration now carries the sibling
device-auth store file, so an unentitled build keeps its stored device
tokens (keyed by the migrated deviceId) instead of re-pairing. The
process-immutable entitlement check is resolved once per process rather
than creating a SecTask on every state-dir lookup, and the migration
source API returns an identity+auth pair struct.

---------

Co-authored-by: Peter Steinberger <steipete@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P1 High-priority user-facing bug, regression, or broken workflow. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: M status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: macOS app rewrites persisted App Group device identity after restart, causing new nodeId and repeated pairing prompts

2 participants