fix(matrix): warn on accumulated token storage roots#97353
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
openclaw-barnacle
Bot
added
docs
|
Codex review: needs maintainer review before merge. Reviewed June 27, 2026, 11:37 PM ET / 03:37 UTC. Summary PR surface: Source +28, Tests +54, Docs +2. Total +84 across 3 files. Reproducibility: yes. Source inspection shows current main scans/selects Matrix token-hash storage roots without an accumulated-root warning, and the PR body includes redacted after-fix warning output. Review metrics: none identified. Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Next step before merge
Security Review detailsBest possible solution: Land this focused Matrix plugin warning after maintainer review and required checks, leaving automatic cleanup, doctor commands, and strict failure modes to separate follow-ups. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection shows current main scans/selects Matrix token-hash storage roots without an accumulated-root warning, and the PR body includes redacted after-fix warning output. Is this the best way to solve the issue? Yes. A Matrix-plugin-local structured warning plus docs preserves storage selection semantics and is narrower than automatic archiving, a new doctor command, or fail-closed behavior. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 830467bc9306. Label changesLabel justifications:
Evidence reviewedPR surface: Source +28, Tests +54, Docs +2. Total +84 across 3 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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
Bot
added
proof: sufficient
|
CI note:
I tried to rerun failed jobs, but GitHub rejected it because this account does not have repository admin rights. Could a maintainer rerun the failed job when convenient? |
(cherry picked from commit f420157)
(cherry picked from commit f420157)
2 participants
Closes #76613
What Problem This Solves
Matrix token rotation can leave multiple populated token-hash storage roots under the same account directory. Current storage-root resolution can still choose the correct active root, but operators get no signal that stale compatible roots have accumulated, which makes crypto-store debugging and cleanup harder.
The previous candidate PR #94644 was closed unmerged after review because it warned for a single legitimate reusable token-rotation sibling and did not expose parseable warning metadata or docs guidance.
Why This Change Was Made
This change keeps Matrix storage selection semantics unchanged and adds observability only when true multi-root accumulation exists: more than one compatible populated token-hash root is present. The warning includes structured metadata for
parentDir,canonicalTokenHash,selectedTokenHash, populated root hashes, sibling hashes, and count.It also documents the operational distinction between one normal older token-hash root and multiple populated roots that should be inspected and archived carefully.
AI-assisted: prepared with Codex and verified locally.
User Impact
Matrix operators get a clear warning when stale populated storage roots accumulate after token rotations, without being told to clean up the normal single-root continuity case. The warning is machine-parseable and the docs explain safe manual cleanup.
Evidence
node scripts/run-vitest.mjs extensions/matrix/src/matrix/client/storage.test.tsnode_modules\.bin\oxfmt.cmd --check --threads=1 extensions/matrix/src/matrix/client/storage.ts extensions/matrix/src/matrix/client/storage.test.ts docs/channels/matrix.mdgit diff --checknode scripts/run-oxlint.mjs extensions/matrix/src/matrix/client/storage.ts extensions/matrix/src/matrix/client/storage.test.tsnode_modules\.bin\tsx.cmd --input-type=module -:{ "resolvedTokenHash": "348e9df2a42bd6e3", "warningCount": 1, "warnings": [ { "message": "matrix: multiple populated token-hash storage roots detected", "meta": { "parentDir": "<stateDir>/matrix/accounts/default/matrix.example.org__bot_example.org", "canonicalTokenHash": "348e9df2a42bd6e3", "selectedTokenHash": "348e9df2a42bd6e3", "populatedTokenHashes": [ "348e9df2a42bd6e3", "9bdf10a691a1cfda" ], "populatedSiblingTokenHashes": [ "9bdf10a691a1cfda" ], "populatedRootCount": 2 } } ] }Attempted
pnpm docs:list, but this Codex checkout triggered a pnpm install/module purge prompt and failed in non-interactive mode withERR_PNPM_ABORTED_REMOVE_MODULES_DIR_NO_TTY; the relevant Matrix docs section was read directly.