Repository navigation
feat(gateway): enforce scoped guest models and foreground-only work - #156453
vincentkoc wants to merge 41 commits into
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
Codex review: needs real behavior proof before merge. Reviewed September 24, 2026, 1:32 AM ET / 05:32 UTC (Revision 4). ClawSweeper reviewWhat this changesThe branch adds scoped guest model routing and foreground-only chat work across Gateway admission, provider hooks, sandbox cleanup, session records, and Control UI Stop and restart flows. Merge readiness⛔ Blocked before merge - 11 items remain Current main has the role model ceiling but not the foreground Stop and restart behavior, so this PR still has distinct value. The prior synchronous SQLite finding remains, an existing bundled provider route can lose finite-role access, and rollback safety for new persisted guest state is unresolved. Priority: P2 Review scores
Verification
How this fits togetherThe Gateway accepts authenticated guest requests and applies role policy before an agent reaches a provider or sandbox. It records foreground work and sends Stop and restart outcomes back to the conversation UI. flowchart LR
A[Authenticated guest] --> B[Gateway role policy]
B --> C[Model route check]
C --> D[Provider request]
B --> E[Foreground admission]
E --> F[Sandbox cleanup]
F --> G[Session notice and chat UI]
Decision needed
Why: The branch writes authority-related state that the older reader does not check, and the PR explicitly leaves rollback unqualified; the acceptable deployment boundary is a maintainer security choice. Before merge
Findings
Agent review detailsSecurityNeeds attention: No supply-chain change was identified, but candidate-written guest custody is not qualified against older-binary final effects. PR surfacePR surface statistics unavailable: the file list is incomplete. Review metrics
Stored data modelPersistent data-model change detected: Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Mantis proof suggestionRedacted web UI chat captures could independently show guest Stop, draft preservation, and the restart notice because the linked UI images could not be inspected. A maintainer can ask Mantis to capture proof by posting this exact PR comment: Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Keep policy in the existing Gateway owner, move retirement reads to its worker, qualify affected provider hooks, and establish an explicit, proven rollback boundary before guest rollout. Do we have a high-confidence way to reproduce the issue? Not applicable: this PR adds a guest mode. Source comparison shows current main lacks its foreground admission and restart records. Is this the best way to solve the issue? Unclear: reusing the existing role policy is sound, but the synchronous registry read, provider compatibility break, and rollback boundary prevent a best-solution verdict. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against c3daf0cddacd. LabelsLabel changes: No label changes. Label justifications:
EvidenceSecurity concerns:
What I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (3 earlier review cycles)
|
Apply the current session-read capability before the existing sharing and recipient fences, with negative and queued delivery coverage. Keep real UI fixtures inside canonical workspace ownership and report bounded run failures.
Represent ordinary sessions by omitting the true-only incognito marker while retaining every visibility case and assertion.
Inherit request qualification from each actual delegate through prompt-state and review-continuation decorators. Exercise the composed provider hook and explicit forbidden UI selection while preserving the shared model pin.
Keep foreground draft editing separate from send admission, preserve Stop and shortcut behavior, and verify cleanup refusals at the owning boundary.
Check the response discriminant before reading the denial fields in the real Gateway lifecycle fixture.
Retain bounded DOM and canonical history identities plus the restart notice screenshot when the original count assertion fails.
Read the same optional __openclaw metadata property using the canonical bracket syntax.
Keep exact transcript counts separate from the pane screen-reader announcement while preserving whole-pane failure diagnostics.
|
@clawsweeper re-review Please review exact head addc4ec. The body now records the existing modelPolicy convergence, revision-bound native execution/cleanup evidence and the explicitly unqualified older-binary rollback boundary. |
|
|
🦞👀 Re-review progress:
|
|
Superseded for replacement by small, independently reviewable PRs. |
Fixes #156269.
What Problem This Solves
Shared Gateway operators need to offer guests approved models and bounded conversation work with understandable Stop, cleanup and restart outcomes.
User Impact
Guest restrictions reuse the existing role
modelPolicy. Accepted work must satisfy both its immutable original ceiling and current policy. Automatic selection chooses an allowed route for that caller without changing shared staff pins; explicitly forbidden choices and unsupported routes visibly refuse.Foreground-only guest work preserves conversation history and drafts. Stop holds admission until owned cleanup settles. Restart records a visible stopped notice without automatically replaying work; continuing requires a fresh authenticated request. Uncertain sandbox cleanup prevents reuse.
Catalogs, session metadata and UI choices respect the same restrictions. Narrow session events require current identity and every referenced session’s live sharing fence. Unrestricted staff behavior, defaults and workspace locations remain unchanged.
Why This Change Was Made
Model authority, foreground admission, native allocation/retirement and session/UI access must agree throughout execution and cleanup. The existing policy owner carries authority through nested work and binds logical selection to the actual provider delegate. This does not certify additional native runtimes or Codex routes.
Change scope against the frozen base
Production/tooling: 334 paths, +13,019/−5,772 lines (net +7,247). Tests/helpers: 151 paths, +21,257/−965 (net +20,292). Production growth implements the four shared authority, foreground, native lifecycle and session-access boundaries above; it does not add a separate policy configuration or database schema.
Existing JSON records gain
foregroundRunandretirementPolicymetadata. No newroles.modelsconfiguration, database, table, column or schema-version bump is introduced. Older-binary rollback of candidate state remains unqualified: restart, backup and updater evidence do not establish equivalent enforcement after downgrade.openclaw/rfcs#74 remains a separate, unaccepted draft. Source landing does not deploy this feature or change operator access infrastructure.
Evidence
Current candidate:
addc4ec21a4a7226530e612b8f4881626e0920a1, tree8fdca6d8a4bb3749a9de27290f1d2c50cf2672b4.0c7d77cdpassed.d63b7688was clean. Subsequent diagnostic and transcript-selector changes are test-only and received scoped review.36c5fc4862ad09b22bf63edcec9a0da9ccaa0c76, with no skips. The 444-case composite remains revision-bound: 299 ateda43e0eacb294d6b4ec8cad0d7720b522503947and 145 atee23da7771abd28bfcd953e2ef918805d38fa4df; these are not an exact-head aggregate.Restart checks count one transcript notice, excluding its separate accessibility announcement, and verify reload, no replay and a fresh request.
Real Linux UI: Stop, cleanup uncertainty and restart
Running work before Stop:
Draft preserved after Stop:
Cleanup still pending:
Cleanup uncertainty refuses reuse:
Running work before Gateway restart:
History and visible notice after restart:
ClawSweeper’s prior parallel-policy finding is addressed by convergence on the existing
modelPolicyowner. Native final-effect results address its requested execution/cleanup proof; older-state rollback qualification remains explicitly open.Named follow-up: Guest Publish PR capability messaging — the narrow guest panel currently reports missing
operator.read; improve its gating/message separately.