Skip to content

fix(ios): scope session mutations to selected agent#103415

Merged
steipete merged 2 commits into
openclaw:mainfrom
lin-hongkuan:lhk/fix-ios-session-mutation-scope-103366
Jul 10, 2026
Merged

fix(ios): scope session mutations to selected agent#103415
steipete merged 2 commits into
openclaw:mainfrom
lin-hongkuan:lhk/fix-ios-session-mutation-scope-103366

Conversation

@lin-hongkuan

@lin-hongkuan lin-hongkuan commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Normalize iOS session patch, delete, and fork targets through the same selected-agent routing used by chat send/history/reset.
  • Preserve global agent routing and already-scoped agent:* keys.
  • Add instance-level regression coverage for the exact gateway methods and payloads dispatched by all three mutations.

Issue / Motivation

Fixes #103366.

When the iOS client selected an agent and held an unscoped session key, chat operations targeted agent:<selected>:<key>, but patch/delete/fork sent the raw key. Those mutations could therefore address a different session; delete also requests transcript deletion.

Changes

  • Route patchSession, deleteSession, and forkSession through sessionTarget(for:).
  • Remove the narrower global-only agent helper.
  • Add a narrow request recorder seam so tests prove instance-method dispatch, not only static JSON builders.
  • Cover unscoped, global, and already-scoped session keys across all three operations.

Testing

  • git diff --check — passed.
  • gitleaks dir --no-banner --redact apps/ios/Sources/Chat/IOSGatewayChatTransport.swift — no leaks found.
  • gitleaks dir --no-banner --redact apps/ios/Tests/IOSGatewayChatTransportTests.swift — no leaks found.
  • Static Swift key-path regression check — passed.
  • Focused Xcode/iOS tests were not runnable in this Linux environment because Swift/Xcode are unavailable; the added test is intended for the repository's macOS CI.

What Problem This Solves

It makes destructive and state-changing session mutations address the same selected-agent session that the iOS chat pane is actually using. For an unscoped scratch key with selected agent ops, all three mutations now target agent:ops:scratch rather than raw scratch.

Evidence

The new regression test captures the actual dispatched requests for patch, delete, and fork and checks:

  • unscoped key → agent:reviewer:<key> with no redundant agentId;
  • global remains global with agentId: reviewer;
  • existing agent:ops:main remains unchanged;
  • methods, mutation-specific fields, and 15-second timeouts are preserved.

Repository hygiene checks passed as listed above. Runtime behavior is unchanged when no selected agent is set.

@lin-hongkuan
lin-hongkuan force-pushed the lhk/fix-ios-session-mutation-scope-103366 branch from a7c3811 to 522df62 Compare July 10, 2026 05:19
@lin-hongkuan

lin-hongkuan commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Rebased onto current main at head 522df62e7f0f. The regression test captures the actual sessions.patch, sessions.delete, and sessions.create requests for unscoped, global, and already-scoped keys. Linux verification: git diff --check passed; both changed Swift files passed gitleaks scans. The macOS CI jobs provide the executable Xcode/iOS validation.

@clawsweeper

clawsweeper Bot commented Jul 10, 2026 •
edited
Loading

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 mentioned this pull request Jul 10, 2026
Closed
@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. P2 Normal backlog priority with limited blast radius. labels Jul 10, 2026
@clawsweeper

clawsweeper Bot commented Jul 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 10, 2026, 9:06 AM ET / 13:06 UTC.

Summary
The PR normalizes iOS patch, delete, and fork session targets to the selected agent, preserves fork child ownership, adds instance-level request coverage, and updates release notes.

PR surface: Docs +1, Other +86. Total +87 across 3 files.

Reproducibility: yes. at source level: the previous iOS mutation methods bypassed selected-agent normalization, and the Gateway contracts show those raw targets resolve differently from adjacent chat operations. A failing connected-device run was not performed.

Review metrics: none identified.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #103366
Summary: This PR is the focused candidate fix for the linked iOS selected-agent session-mutation bug.

Members:

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

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
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:

  • [P1] Add redacted connected-Gateway iOS evidence showing patch, delete, and fork affect the selected-agent session.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The exact-head Xcode simulator run executes the real transport methods but injects a request recorder instead of a connected Gateway; add redacted simulator/device logs, terminal or live output, a recording, or a linked artifact showing the corrected mutations, redact endpoints, identifiers, tokens, and user data, then update the PR body for automatic re-review or ask a maintainer to comment @clawsweeper re-review.

Risk before merge

  • [P1] Because delete includes transcript deletion and fork creates persistent session state, an undetected client/server routing mismatch could mutate, remove, or create state under the wrong agent; the recorder test cannot observe that server-side outcome.

Maintainer options:

  1. Require connected session proof (recommended)
    Capture redacted simulator or device output against a connected Gateway showing patch, delete, and fork act on the selected-agent session before merge.
  2. Accept contract-only evidence
    Merge based on the inspected server contracts, exact-head CI, and recorder test while explicitly accepting that the server-side mutation outcome was not observed.

Next step before merge

  • [P1] No automated code repair is needed; keep the PR open for contributor-owned connected-Gateway proof and exact-head maintainer merge handling.

Security
Cleared: The diff introduces no concrete dependency, workflow, permission, secret-handling, package-resolution, downloaded-artifact, or supply-chain concern.

Review details

Best possible solution:

Keep this canonical resolver-based implementation, then demonstrate on a redacted connected iOS simulator or device that patch, delete, and fork affect the selected-agent session before merging.

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

Yes at source level: the previous iOS mutation methods bypassed selected-agent normalization, and the Gateway contracts show those raw targets resolve differently from adjacent chat operations. A failing connected-device run was not performed.

Is this the best way to solve the issue?

Yes. Reusing the canonical session target resolver removes duplicate routing policy, and deriving fork child ownership from the normalized parent matches the server create contract.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 5a451f92f993.

Label changes

Label changes:

  • add merge-risk: 🚨 session-state: The PR changes routing for transcript deletion and fork creation, and current proof does not observe the resulting server-side session state.

Label justifications:

  • P2: This is a bounded iOS multi-agent bug fix with potentially destructive wrong-session behavior but limited affected configurations.
  • merge-risk: 🚨 session-state: The PR changes routing for transcript deletion and fork creation, and current proof does not observe the resulting server-side session state.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The exact-head Xcode simulator run executes the real transport methods but injects a request recorder instead of a connected Gateway; add redacted simulator/device logs, terminal or live output, a recording, or a linked artifact showing the corrected mutations, redact endpoints, identifiers, tokens, and user data, then update the PR body for automatic re-review or ask a maintainer to comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Docs +1, Other +86. Total +87 across 3 files.

View PR surface stats
Area Files Added Removed Net
Source 0 0 0 0
Tests 0 0 0 0
Docs 1 1 0 +1
Config 0 0 0 0
Generated 0 0 0 0
Other 2 111 25 +86
Total 3 112 25 +87

What I checked:

  • Canonical iOS routing: The current head sends patch, delete, and fork through the existing sessionTarget resolver, preserving global aliases and already agent-scoped keys. (apps/ios/Sources/Chat/IOSGatewayChatTransport.swift:399, c2262aa04cd1)
  • Gateway mutation contract: Gateway patch and delete resolve non-global ownership from an agent-scoped key, while selected global sessions use agentId; conflicting key and agentId combinations are rejected. (src/gateway/server-methods/sessions.ts:2001, 5a451f92f993)
  • Fork ownership contract: sessions.create selects the child store from agentId and resolves the parent separately from parentSessionKey, so the PR correctly derives the fork child agent from the normalized parent. (src/gateway/session-create-service.ts:172, 5a451f92f993)
  • Focused regression coverage: The added exact-head test invokes all three transport methods for unscoped, global, and already-scoped keys and verifies nine emitted requests, mutation fields, ownership fields, and timeouts. (apps/ios/Tests/IOSGatewayChatTransportTests.swift:283, c2262aa04cd1)
  • Exact-head validation: The current head passes macos-swift, ios-build, security-fast, dependency guard, and related repository checks. (c2262aa04cd1)
  • Proof limitation: The reported 28-test Xcode simulator run replaces Gateway requests with a recorder and explicitly did not use a connected Gateway or destructive session mutation. (c2262aa04cd1)

Likely related people:

  • steipete: Git history associates the current iOS transport and Gateway session-create ownership code with commit abbd5ae, and this person authored the PR's current-head fork-ownership correction and simulator validation. (role: introduced behavior and recent corrective contributor; confidence: high; commits: abbd5ae3ea73, c2262aa04cd1; files: apps/ios/Sources/Chat/IOSGatewayChatTransport.swift, apps/ios/Tests/IOSGatewayChatTransportTests.swift, src/gateway/session-create-service.ts)
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 (1 earlier review cycle)
  • reviewed 2026-07-10T10:44:53.955Z sha 522df62 :: needs real behavior proof before merge. :: none

@steipete

steipete commented Jul 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Found and fixed one remaining contract bug before landing: sessions.create.agentId owns the forked child, so normalized non-global parents must still send the parent agent. The updated branch now keeps patch/delete scoped to the normalized parent target and derives fork child ownership from that target (reviewer, reviewer, and ops in the three regression cases).

Focused proof on head 59b54a06b12e:

  • swiftformat --lint --config config/swiftformat apps/ios/Sources/Chat/IOSGatewayChatTransport.swift apps/ios/Tests/IOSGatewayChatTransportTests.swift
  • Xcode 26.6, iPhone 17 Pro / iOS 26.5 simulator: OpenClawTests/IOSGatewayChatTransportTests — 28 passed, 0 failed, 0 skipped.
  • Autoreview: clean, no accepted/actionable findings.
  • git diff --check

Final exact-head landing proof on 797719674b7643f9ba19069474d176a9d2383abb:

  • CI run 29099688499 completed successfully, including ios-build and macos-swift.
  • .agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main --engine codex --model gpt-5.5 --thinking high completed clean with no accepted/actionable findings (confidence 0.86).
  • Squash-landed as b2c1db4b5eb601c498f36f649dda1155448476f6.

The changelog credits both the PR author and issue reporter. The simulator regression validates the real transport methods and captured Gateway payloads; no connected production Gateway or destructive session mutation was used.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 10, 2026 •
edited
Loading

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.

Re-review progress:

@steipete
steipete force-pushed the lhk/fix-ios-session-mutation-scope-103366 branch from 59b54a0 to c2262aa Compare July 10, 2026 12:47
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. label Jul 10, 2026
@steipete
steipete force-pushed the lhk/fix-ios-session-mutation-scope-103366 branch from c2262aa to 7977196 Compare July 10, 2026 14:24
@steipete
steipete merged commit b2c1db4 into openclaw:main Jul 10, 2026
41 checks passed
@steipete

steipete commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Merged via squash.

This was referenced Jul 10, 2026
Closed
Merged
@steipete

steipete commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Post-merge live proof is green on current main at bd9b1b428c365d890a0c155ee5fe62bada03c9ca (including the routing fix from b2c1db4b5eb601c498f36f649dda1155448476f6).

Scenario:

  • Started a real source Gateway with an isolated state directory, shared-token auth, and two configured agents: main and reviewer.
  • Connected from an iPhone 17 Pro simulator (iOS 26.5) through the real Swift GatewayNodeSession; no request recorder or mocked mutation transport.
  • Seeded independent global sessions for both agents, then exercised IOSGatewayChatTransport(gateway:globalAgentId:) with globalAgentId: "reviewer".
  • Verified patchSession("global") changed only the reviewer store; the main label stayed unchanged.
  • Verified forkSession("global") returned an agent:reviewer:dashboard:* child.
  • Verified deleteSession("global") removed only the reviewer global session while the main global session and reviewer fork child remained present.

Proof command shape: pnpm ios:gen, xcodebuild build-for-testing, then one simulator-hosted test-without-building run with the live Gateway URL/token injected into the private .xctestrun environment. Exact result: 1 passed, 0 failed, 0 skipped; xcodebuild exited 0. The temporary token was redacted after the run and the isolated Gateway was stopped.

This closes the earlier proof gap: the scoped routing behavior has now traversed the production Swift WebSocket and Gateway session-store paths end to end.

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 11, 2026
@lin-hongkuan @steipete
fix(ios): scope session mutations to selected agent (openclaw#103415)
46c94e3 
* fix(ios): scope session mutations to selected agent

* fix(ios): preserve agent ownership for session mutations

---------

Co-authored-by: lin-hongkuan <[email protected]>
Co-authored-by: Peter Steinberger <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

app: ios App: ios merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P2 Normal backlog priority with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: iOS sessions.delete/patch/fork skip the agent-scoping applied to chat.send, targeting the wrong session

2 participants

@lin-hongkuan @steipete