Skip to content

feat(gateway): let write-scope operators manage chat session organization#100964

Merged
steipete merged 1 commit into
mainfrom
feat/sessions-patch-write-scope
Jul 6, 2026
Merged

feat(gateway): let write-scope operators manage chat session organization#100964
steipete merged 1 commit into
mainfrom
feat/sessions-patch-write-scope

Conversation

@steipete

@steipete steipete commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

The session controls shipped in #100814 (rename, pin, archive, mark unread, move to group) mutate sessions through sessions.patch, which requires the operator.admin scope. The Android app deliberately connects as a bounded operator without admin (nativeClientOperatorScopes in apps/android/.../node/ConnectionManager.kt), so every one of those controls fails authorization with missing scope: operator.admin on Android. More broadly, user-level chat organization should not require admin rights on any client.

Two smaller gaps in the same surface: session list ordering had no tiebreaker for equal timestamps (unstable offset paging), and a stored channel-derived displayName beat a user-assigned label in the row projection, so renames of channel-named sessions vanished on the next refresh.

Tracking issue: #100712

Why This Change Was Made

  • sessions.patch becomes a params-aware dynamic-scope method (same mechanism as plugins.sessionAction): operator.write authorizes patches touching only user-level chat-organization fields — label, category, pinned, archived, unread (plus key/agentId targeting). Any other field, mixed patches, or unknown keys keep requiring operator.admin, fail closed. Write scope can already create sessions, send messages, and abort runs, so chat organization is not an escalation; the admin-only fields (sendPolicy, tool inheritance, exec routing, model, spawn metadata, …) stay admin-only. Existing handler guards are unchanged (no archiving active runs or an agent's main session; webchat mutations stay rejected).
  • A dedicated write-scoped RPC was rejected: it would duplicate the existing mutation path for the same fields and add protocol/codegen surface for no behavioral gain. Granting mobile clients admin was rejected; the Android operator session is least-privilege by design.
  • compareSessionEntryPairs gains a locale-independent key tiebreaker after pinnedAt/updatedAt, so list order and offset paging are deterministic across calls.
  • The row projection now prefers label over stored displayName: an explicit user rename must win over channel-derived names.
  • Subagent spawn pinned admin scopes via the static isAdminOnlyMethod check; with sessions.patch now dynamic it uses the params-aware least-privilege resolver, so spawn-metadata patches still pin to admin (preserves the [Bug]: sessions_spawn sub-agent fails with "pairing required" (1008) on v2026.4.1 #59428 no-scope-upgrade contract). isAdminOnlyMethod had no other callers outside its own module.

No protocol schema changes: sessions.patch params and sessions.list search/archived already exist; pnpm protocol:check is clean.

User Impact

Android (and any non-admin operator client) can now rename, pin, archive, categorize, and mark sessions unread. Session lists page deterministically, and renames of channel-named sessions stick. Admin-gated session mutations are unchanged for admin clients.

Evidence

  • New scope-policy tests in src/gateway/method-scopes.test.ts: write scope allowed for organization-field patches (incl. category/unread), denied for admin fields, mixed patches, and unknown keys; read scope denied; malformed params fall through to handler validation.
  • New WS-level test in src/gateway/server.sessions.store-rpc.test.ts: a client connected with ["operator.read", "operator.write"] renames/pins/archives/categorizes via sessions.patch, filters archived lists, server-searches by the renamed label (asserting displayName reflects the label over a stored channel name), and gets missing scope: operator.admin for sendPolicy/mixed patches. Plus a tie-ordering/paging test.
  • Hosted Testbox runs (Blacksmith): pnpm test src/gateway/method-scopes.test.ts src/gateway/server.sessions.store-rpc.test.ts src/gateway/session-utils.test.ts src/gateway/session-utils.search.test.ts src/gateway/sessions-patch.test.ts src/gateway/server.sessions.list-changed.test.ts src/agents/subagent-spawn.test.ts → 20 files, 1200 tests passed (+ 25 in the spawn shard); pnpm tsgo:core and pnpm tsgo:core:test green; oxlint/oxfmt clean on touched files.
  • Autoreview (Codex, gpt-5.5) run to a clean result on the combined change; review findings (label-vs-displayName precedence, cancellation handling in the client half) were verified and fixed before splitting.

Client follow-ups (stacked): iOS/shared and Android PRs add server-backed session search UI on top of this.

@steipete
feat(gateway): let write-scope operators manage chat session organiza…
9fe6ecd 
…tion

sessions.patch becomes a params-aware dynamic-scope method: operator.write
now authorizes patches that touch only user-level chat-organization fields
(label, category, pinned, archived, unread); every other field, mixed
patches, and unknown keys keep requiring operator.admin (fail closed).
This unblocks the session controls shipped in #100814 for the Android app,
whose bounded operator session intentionally never requests operator.admin.

Also: session list ordering gains a deterministic key tiebreaker for equal
pinnedAt/updatedAt (stable offset paging and prompt-cache friendliness), a
user-assigned label now beats stored channel-derived display names in the
row projection so renames survive refreshes, and subagent spawn pins its
gateway calls to admin via the params-aware least-privilege resolver
instead of the static admin-only method check (#59428 contract preserved).

Refs #100712
@openclaw-barnacle openclaw-barnacle Bot added gateway Gateway runtime agents Agent runtime and tooling size: M maintainer Maintainer-authored PR labels Jul 6, 2026
@clawsweeper

clawsweeper Bot commented Jul 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 6, 2026, 11:34 AM ET / 15:34 UTC.

Summary
The PR changes sessions.patch from a static admin-scoped method to params-aware dynamic scoping for session organization fields, preserves admin scoping for sensitive fields, updates subagent scope pinning, and adds gateway/session tests.

PR surface: Source +63, Tests +182. Total +245 across 8 files.

Reproducibility: yes. Source inspection shows current main keeps sessions.patch statically admin-scoped while write-scoped clients can already create, send, and abort sessions; the PR's tests model the write-only gateway path.

Review metrics: 1 noteworthy metric.

  • Gateway auth surface: 1 method changed from static admin to dynamic; 5 organization fields allowed at write scope. This is the maintainer-relevant boundary change that needs explicit review before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #100712
Summary: This PR is the gateway authorization candidate for the open session-management tracker; the merged session-control PR created much of the UI/store surface but did not settle this write-scope gateway boundary.

Members:

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

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🌊 off-meta tidepool
Patch quality: 🐚 platinum hermit
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:

  • [P2] Get explicit maintainer approval for the operator.write session-organization boundary before merge.

Risk before merge

  • [P1] Merging intentionally expands the sessions.patch security boundary: operator.write clients can mutate label, category, pinned, archived, and unread, so maintainers should explicitly accept that boundary even though admin fields stay fail-closed.
  • [P1] The Android fresh-token rationale in the PR body is partially stale against current main, which now grants fresh Android tokens operator.admin; the remaining benefit is stored legacy write-only tokens and other bounded operator clients.

Maintainer options:

  1. Approve the allowlisted boundary (recommended)
    Maintainers can land the PR after explicitly accepting that write-scoped operators may edit only session organization fields.
  2. Split the mutation surface
    Ask for a dedicated write-scoped organization RPC if dynamic scoping on sessions.patch is not the desired long-term contract.
  3. Pause the stacked client work
    Pause or close this gateway slice if user-level session organization should remain admin-only for now.

Next step before merge

  • [P1] The PR has no concrete code finding for automation, but the protected maintainer label and security-boundary change need human approval before merge.

Maintainer decision needed

  • Question: Should operator.write clients be allowed to mutate only session organization fields through the existing sessions.patch method?
  • Rationale: This is an intentional authorization-boundary change on a method that also carries admin-level session routing, model, tool, and spawn metadata fields, so automation should not treat it as routine cleanup.
  • Likely owner: steipete — They authored the session-management feature, opened the canonical tracker, and authored this gateway follow-up.
  • Options:
    • Approve Field-Allowlisted Dynamic Scope (recommended): Accept this PR's dynamic sessions.patch policy, where organization fields use operator.write and every other field remains admin-only.
    • Require A Separate Organization RPC: Ask for a new write-scoped session organization method to avoid dynamic scoping on the broader sessions.patch surface.
    • Keep Admin-Only For Now: Defer the gateway permission change and keep mobile or bounded-client session organization blocked until the product boundary is settled.

Security
Cleared: No concrete security defect was found in the patch; the intentional auth-boundary expansion remains a maintainer approval risk rather than a line-level bug.

Review details

Best possible solution:

Land the field-allowlisted dynamic sessions.patch scope after maintainer security approval, keeping admin-only fields fail-closed and the subagent admin pin intact.

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

Yes. Source inspection shows current main keeps sessions.patch statically admin-scoped while write-scoped clients can already create, send, and abort sessions; the PR's tests model the write-only gateway path.

Is this the best way to solve the issue?

Yes, pending maintainer approval. Reusing the existing patch schema with a fail-closed field allowlist is narrower than adding duplicate protocol surface, but the authorization boundary is a real product/security choice.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 9606fe43231b.

Label changes

Label changes:

  • add P2: This is a normal-priority session-management improvement with limited blast radius and no evidence of data loss, crash loops, or unusable core runtime.
  • add merge-risk: 🚨 security-boundary: The diff intentionally grants operator.write access to part of a previously admin-only session mutation method.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The contributor proof gate is not applied because the PR is maintainer-labeled; the body and CI provide automated gateway validation, but no separate real-device proof is required by this gate.

Label justifications:

  • P2: This is a normal-priority session-management improvement with limited blast radius and no evidence of data loss, crash loops, or unusable core runtime.
  • merge-risk: 🚨 security-boundary: The diff intentionally grants operator.write access to part of a previously admin-only session mutation method.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The contributor proof gate is not applied because the PR is maintainer-labeled; the body and CI provide automated gateway validation, but no separate real-device proof is required by this gate.
Evidence reviewed

PR surface:

Source +63, Tests +182. Total +245 across 8 files.

View PR surface stats
Area Files Added Removed Net
Source 6 77 14 +63
Tests 2 182 0 +182
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 8 259 14 +245

What I checked:

Likely related people:

  • steipete: Authored the merged session-management PR feat(sessions): grouping, unread state, and full session controls on web, iOS, and Android #100814 and this maintainer-labeled gateway follow-up for the same session organization surface. (role: feature owner and likely decision owner; confidence: high; commits: 88f1ec38d4a5, 9fe6ecda2dfd; files: src/gateway/method-scopes.ts, src/gateway/methods/core-descriptors.ts, src/gateway/session-utils.ts)
  • momothemage: Recent startup-scan refactor commit is the current blame source for the central gateway method table, method-scope resolver, session row projection, and subagent caller on main. (role: recent area contributor; confidence: medium; commits: 17605c57df26; files: src/gateway/method-scopes.ts, src/gateway/methods/core-descriptors.ts, src/gateway/session-utils.ts)
  • openperf: Authored the merged fix that pinned subagent gateway calls to admin scope, which this PR must preserve through the params-aware resolver. (role: introduced related subagent scope behavior; confidence: medium; commits: b40ef364b71c; files: src/agents/subagent-spawn.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.

@clawsweeper clawsweeper Bot added 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. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. labels Jul 6, 2026
This was referenced Jul 6, 2026
Merged
Merged
@steipete
steipete merged commit 38746af into main Jul 6, 2026
155 of 163 checks passed
@steipete
steipete deleted the feat/sessions-patch-write-scope branch July 6, 2026 20:08
@steipete

steipete commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

Merged via squash.

@github-actions github-actions Bot mentioned this pull request Jul 6, 2026
Open
@steipete steipete mentioned this pull request Jul 6, 2026
Closed
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 7, 2026
@steipete
feat(gateway): let write-scope operators manage chat session organiza…
2079cfc 
…tion (openclaw#100964)

sessions.patch becomes a params-aware dynamic-scope method: operator.write
now authorizes patches that touch only user-level chat-organization fields
(label, category, pinned, archived, unread); every other field, mixed
patches, and unknown keys keep requiring operator.admin (fail closed).
This unblocks the session controls shipped in openclaw#100814 for the Android app,
whose bounded operator session intentionally never requests operator.admin.

Also: session list ordering gains a deterministic key tiebreaker for equal
pinnedAt/updatedAt (stable offset paging and prompt-cache friendliness), a
user-assigned label now beats stored channel-derived display names in the
row projection so renames survive refreshes, and subagent spawn pins its
gateway calls to admin via the params-aware least-privilege resolver
instead of the static admin-only method check (openclaw#59428 contract preserved).

Refs openclaw#100712
@steipete steipete mentioned this pull request Jul 7, 2026
Merged
giodl73-repo pushed a commit to giodl73-repo/openclaw that referenced this pull request Jul 8, 2026
@steipete @giodl73-repo
feat(gateway): let write-scope operators manage chat session organiza…
c2f41e2 
…tion (openclaw#100964)

sessions.patch becomes a params-aware dynamic-scope method: operator.write
now authorizes patches that touch only user-level chat-organization fields
(label, category, pinned, archived, unread); every other field, mixed
patches, and unknown keys keep requiring operator.admin (fail closed).
This unblocks the session controls shipped in openclaw#100814 for the Android app,
whose bounded operator session intentionally never requests operator.admin.

Also: session list ordering gains a deterministic key tiebreaker for equal
pinnedAt/updatedAt (stable offset paging and prompt-cache friendliness), a
user-assigned label now beats stored channel-derived display names in the
row projection so renames survive refreshes, and subagent spawn pins its
gateway calls to admin via the params-aware least-privilege resolver
instead of the static admin-only method check (openclaw#59428 contract preserved).

Refs openclaw#100712
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

agents Agent runtime and tooling gateway Gateway runtime maintainer Maintainer-authored PR merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. P2 Normal backlog priority with limited blast radius. 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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@steipete