Repository navigation
fix(gateway): preserve task pages during session metadata edits - #137975
Merged
Merged
Conversation
Contributor
|
🦞👀 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. |
Contributor
|
Codex review: needs maintainer review before merge. Reviewed September 7, 2026, 3:05 PM ET / 19:05 UTC. ClawSweeper reviewWhat this changesPreserves task pagination during session metadata edits while retaining access-change invalidation, adds Gateway regression coverage, and documents the behavior. Merge readiness✅ Ready for maintainer review This remains a useful fix absent from current main. No actionable introduced defect was found, and the updated comparison addresses the previous validation request. Maintainer authorship and the protected label also preclude cleanup closure. Priority: P2 Review scores
Verification
How this fits togetherThe Gateway serves task pages to OpenClaw clients and checks access through each task’s requesting session. Session mutations refresh displayed metadata and determine whether saved pagination cursors must expire. flowchart TD
A[Session edit] --> B[Commit session mutation]
B --> C{Read access changed?}
C -->|Yes or uncertain| D[Advance access revision]
C -->|No| E[Preserve task cursor]
B --> F[Refresh session projections]
D --> G[Task page authorization]
E --> G
G --> H[Authorized page or restart guidance]
Before mergeNone. Agent review detailsSecurityNone. PR surfaceSource +52, Tests +247, Docs 0. Total +299 across 11 files. View PR surface stats
Review metrics
Technical reviewBest possible solution: Keep metadata projection refresh separate from task-access invalidation while preserving current authorization checks and conservative handling of unknown session generations. Do we have a high-confidence way to reproduce the issue? Yes: obtain a task cursor, commit a session label/category edit, then request the next page. Current-main source unconditionally advances the cursor’s access revision; the captured baseline report confirms the resulting restart error. Is this the best way to solve the issue? Yes: classifying committed access facts at the mutation owner avoids unnecessary cursor expiry without adding configuration, changing stored formats, or weakening final authorization. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against c20600ad9394. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (4 earlier review cycles)
|
Integrate the existing task access-revision repair with current main while retaining the current patch admission, authorization, diagnostics, and task query behavior. Metadata changes refresh projections without expiring access; canonical moves and authorization-relevant changes remain conservative. Preserve the newer task performance coverage through the shared fixture.
Initialize each task registry after session seeding and before either authenticated client connects, preserving the original performance fixture order. Keep all assertions and deadlines unchanged.
Contributor
Author
|
Merged via squash.
|
github-actions Bot
pushed a commit
to Desicool/openclaw
that referenced
this pull request
Sep 8, 2026
…claw#137975) * perf(gateway): avoid task rescans for session metadata edits * test(gateway): place task metadata proof in the server test shard * test(gateway): preserve task fixture initialization order Initialize each task registry after session seeding and before either authenticated client connects, preserving the original performance fixture order. Keep all assertions and deadlines unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related: #141001, #137966.
Maintainer edits
This same-repository branch remains editable by maintainers.
What Problem This Solves
Fixes an issue where users loading Tasks would lose their pagination position or receive “Task activity did not stabilize” while another client only changed an existing session’s label or category.
Why This Change Was Made
The committed session-patch owner now distinguishes display changes on the same canonical session generation from changes to read access. Metadata edits still refresh session rows, sharing snapshots, list projections, and notifications; they preserve the access revision used by task cursors. Creation, missing generations, replacement, canonical moves, removed aliases, creator changes, visibility changes, and incognito changes remain conservative. Explicit sharing changes, role changes, profile-alias changes, and final row authorization retain their existing fences.
The access-change fact stays internal and never enters an RPC or event payload. Current main’s patch admission, catalog preparation, authorization errors, diagnostics, and task-query behavior are preserved. No schema, configuration, dependency, or public protocol change is included.
User Impact
Task pages remain usable during label and category updates without repeating scans for those display-only changes. Actual task activity and access changes can still require fresh pagination. This complements #141001’s earlier exit for an already-invalid task cursor; it does not claim to eliminate unrelated Gateway stalls.
Evidence
Current head:
d17bc69fae528b13ba9897587b1136239d72fdd2. Original head2c73864f9eb66c1b300ad4fb12f59aa07c5088e6is preserved in its ancestry; mainfe2f796e95e87773bdd2e038f45aa6191acff66ewas integrated through a merge. The remote branch update is a fast-forward.Current validation is complete: exact-head CI run 34152080784 is green with no requested rerun, and the corrected isolated comparison below passed all eight required stages. The historical local timeout and first comparison’s lost detailed capture remain retained and qualified; they are not rewritten as successful results.
INVALID_REQUESTwith restart-pagination guidance. The test takes 17.066 seconds; wrapper time, including cold transforms, is 80.188 seconds.Corrected isolated Linux comparison: passed
Blacksmith Testbox run 34152374111 compared baseline
fe2f796e95e87773bdd2e038f45aa6191acff66ewith candidated17bc69fae528b13ba9897587b1136239d72fdd2in separate detached worktrees within one frozen invocation. All eight required install/build/test/Gateway stages passed. Both complete raw-source manifests were independently rehashed from local Git blobs (39,266 baseline files and 39,268 candidate files), matching all ten remote phase receipts. Build stamps and the actual CLI entrypoints matched their source commits.The ordinary, separately launched Gateway fixtures completed 23 RPCs each against 25 persisted synthetic tasks. Baseline returned the expected expired-cursor error after committed label/category updates; candidate continued the same cursor with exactly
task-007throughtask-013. Both preserved session identity and lifecycle generation, refreshed metadata projections, denied the read-only viewer’s write, verified fresh membership, invalidated cursors for membership/visibility/requester creation, and returned the expected final task IDs. Both Gateway owners joined, and both proof processes exited naturally with code 0.The original 10,000-task performance test passed in 12.252 seconds on baseline. Candidate passed that unchanged case in 11.230 seconds, then the metadata regression in 1.096 seconds, in the required serial performance→metadata order. Both test declarations retain their 60-second deadlines. These focused fixture timings do not establish general Team latency or the historical timeout’s cause.
Required artifact collection succeeded within the unchanged 10 MiB limit: 6,370,630 bytes for the inner evidence archive, SHA-256
b0353107f064c605fb5f81e9ea1158051edb0f9e66c523109ce34704c3e4ed31. All 149 archive entries were recovered. A separate external result-portal synchronization timed out; native proof execution, process-closure verification and required artifact retrieval succeeded. The exact kept lease was stopped after both independent inspections and is verified completed. That lifecycle result is separate from the successful proof command; the hosting workflow is not presented as CI-green.Retained local failure
The original 10,000-task performance case exceeded its unchanged 60-second limit, reporting 60.122 seconds. The following metadata file correctly refused to replace test state while that Gateway fixture remained open. The runner retained its worker generation and temporary resource-ownership claim. The combined result was one failed test, 75 passed, one skipped; two failed files and eight passed, with exit 1. No deadline, assertion, fixture-lifetime guard, or retention result was weakened. Standalone metadata success does not convert this ordered run into a pass, and the timeout’s cause is not established.
Earlier isolated Linux comparison: failed/partial
Blacksmith Testbox run 34147263496 used the sealed baseline
fe2f796e95e87773bdd2e038f45aa6191acff66eand candidate0523a655793d9910d7375880cb5a12ef507e1312in separate detached worktrees. Recorded full-build exits passed for both sources (208.511 and 226.503 seconds), and the baseline's unchanged original 10,000-task performance test passed as one test in 23.424 seconds. The candidate ordered-test command failed at startup in 0.896 seconds; the separate baseline/candidate 25-task Gateway fixtures failed in 9.483/10.724 seconds. This is not candidate behavioral acceptance.Artifact collection then failed before native lease cleanup. The lease is confirmed completed; its backing Actions job was cancelled during cleanup and is not represented as green CI. The completed lease's detailed source manifests, test logs, Gateway responses/state and failure messages were not recovered. The retained local seven-file failure bundle contains stage summaries and the generated archive hash, not the missing detailed evidence. Neither successful stage summaries nor current CI substitute for that capture.
Two proof-harness defects are established independently. The pinned Vitest CLI retains
sequence.sequenceras a string while its pool requires a constructor; the external configuration must supply the imported class. Separately, reproducing the ten deterministic source-manifest copies from immutable Git blobs yields a 28.9 MB compressed archive before runtime evidence, exceeding native collection's unchanged 10 MiB limit. One canonical manifest per revision with every phase's hash receipt reduces that portion to 5.8 MB without removing verification. These transport/configuration corrections were verified before the retained diagnostic below. The first comparison’s detailed Gateway failure remains unavailable; the later diagnostic establishes its own concrete configuration failure without retroactively claiming recovered logs for the first run.Before the new full comparison, the standalone Gateway fixture received bounded redacted diagnostics on a retained isolated lease, as described below. Required proof remains the same: exact task IDs, metadata persistence, write denial, membership/visibility/creation fences, the original performance→metadata execution order and existing deadlines, complete source/build identity, and verified process closure. No provider or production Gateway operation is part of this work. All failed runs and original seals remain recorded.
Follow-up fixture repairs and current local proof
The candidate-only diagnostic run 34150367657 recovered all 99 artifact entries, complete source and build identities, the Gateway startup log, and natural fixture closure. The Gateway correctly rejected a conflicting synthetic authentication configuration: the test-instance helper supplied a shared token, while the standalone proof selected trusted-proxy mode without clearing that token. No authenticated request or pagination check ran. The external proof now explicitly clears the helper token; configuration-only validation confirms the serialized token is absent. Authentication behavior and assertions are unchanged. The lease is verified completed. This diagnosis directly covers the retained diagnostic; detailed logs from the earlier first comparison remain unavailable.
Separately, source comparison found that extracting the shared authenticated fixture had moved task-registry initialization after the clients connected. Commit
d17bc69fae528b13ba9897587b1136239d72fdd2restores original main’s pre-connection initialization in three test/helper files (+9/−4), with no production change. It does not establish that this ordering difference caused the historical timeout.The exact performance→metadata order now passes locally: the unchanged 10,000-task case takes 22.648 seconds, followed by the metadata regression in 2.274 seconds, with wrapper exit 0 and verified worker cleanup. The complete changed-file gate passes, including core-test types, all three dead-export scans and lint. Fresh independent P0–P2 review of this test-only adjustment is clean. The historical failure remains retained; the fully sealed comparison reported above now confirms recovery of the original ordered workload and the ordinary-request contract on this candidate.
Historical proof retained
The earlier source at base
c86d5b2plus the original patch had a real Gateway pre-fix reproduction and candidate cursor/creation proof. A source-verified Linux run passed 113 tests across 11 files, including the unchanged 10,000-task case in 16.978 seconds: 33839191012. Its later response-only cleanup passed 38 focused tests. Separate original-source runs passed the canonical export scans 33840867611 and remaining static guards 33842406239. Earlier local deadline and teardown failures remain retained. None of these historical runs is current-head acceptance.The latest ClawSweeper review at 18:39 UTC on September 7 inspected this
d17bc69head and identified no introduced correctness or security defect. Both P2 validation requests are addressed by the independently inspected full comparison and recorded current-head CI above. The current original workload recovered with unchanged deadlines; the earlier local timeout’s cause remains unknown, and the first remote comparison’s detailed diagnostics remain unrecovered. No causal attribution or retroactive pass is claimed.Production delta against the integrated main base: +65/−13, net +52 lines. Tests and shared fixtures: +774/−527, net +247 lines. Documentation: one paragraph updated. The larger test diff primarily moves shared setup while preserving the newer main assertions.