feat(auto-reply): expose safe requester identity metadata#82353
feat(auto-reply): expose safe requester identity metadata#82353kiranmagic7 wants to merge 12 commits into
Conversation
|
Codex review: found issues before merge. Reviewed June 8, 2026, 8:41 PM ET / 00:41 UTC. Summary PR surface: Source +106, Tests +97. Total +203 across 3 files. Reproducibility: not applicable. as a bug reproduction; this is a feature PR. The supplied terminal proof directly exercises the changed prompt builder and shows the intended after-fix payload. Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Settle one trusted identity contract and canonical resolver boundary, then route core and plugin callers through that shared path before merging. Do we have a high-confidence way to reproduce the issue? Not applicable as a bug reproduction; this is a feature PR. The supplied terminal proof directly exercises the changed prompt builder and shows the intended after-fix payload. Is this the best way to solve the issue? No. The implementation is plausible, but the best fix is to align the trusted identity field and resolver with existing identity-link semantics and the related sender-principal proposal first. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against 5b76436c452a. Label changesLabel justifications:
Evidence reviewedPR surface: Source +106, Tests +97. Total +203 across 3 files. View PR surface stats
Security concerns:
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
|
d37b595 to
83caed7
Compare
83caed7 to
47a12c8
Compare
47a12c8 to
d82ef2d
Compare
|
Heads up: this PR needs to be updated against current |
|
@clawsweeper re-review One shared current-main UI test expectation failed in GitHub CI across the updated branches: I added the one-line expectation update and verified it locally: |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review Addressed the trusted-identity review finding by removing Verification on head |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review Refreshed this branch onto current The previous deterministic The only remaining GitHub failure I see is |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Follow-up on the only red GitHub check for current head The failed job is It timed out with no output after these unrelated gateway-server files had already started passing, not on this PR's touched auto-reply identity path. I ran the same shard include set locally on the PR head: The PR-specific checks remain green: |
|
@clawsweeper re-review I pushed an empty CI-retrigger commit because the only remaining GitHub failure on the previous head was a no-output timeout in an unrelated gateway-server shard, and this fork account cannot rerun maintainer-side jobs directly. Current head: Code and proof are unchanged from the real fix. I also ran the exact previously failing shard locally on the PR head: PR-specific local verification remains: |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Friendly follow-up: this still looks ready for maintainer review. Latest head |
|
@clawsweeper re-review Re-requesting because the PR still carries the |
|
🦞👀 Command router queued. I will update this comment with the next step. Re-review progress:
|
|
Acknowledged the updated review. I agree the remaining blocker is the trusted identity contract, not CI: current head From my side, I can update the branch in either direction once that decision is made:
I will not push a schema rename or authority-boundary change without that maintainer direction. |
Summary
requester_identityto trusted inbound metadata when a stable provider id matchessession.identityLinksNativeDirectUserId,SenderId,SenderE164, directOriginatingTo); do not trust display names, usernames, group titles, quotes, or historyReal behavior proof
Behavior or issue addressed: Trusted inbound metadata can now include a canonical requester identity only when a stable provider id matches configured
session.identityLinks; spoofable display metadata is not trusted and raw sender ids are not emitted.Real environment tested: Local OpenClaw source checkout on macOS/Darwin, branch
kiran/requester-identity-safe-metadata, running the changedbuildInboundMetaSystemPromptimplementation through Node + tsx from the working tree.Exact steps or command run after this patch:
Evidence after fix: Terminal output from the command above:
Observed result after fix: The trusted metadata payload emitted
requester_identity.canonical_id = "alice"from the configured stabletelegram:123456789link, whilesender_id_exposedandspoofable_name_exposedwere bothfalse.What was not tested: End-to-end delivery through a live Telegram bot/account was not tested; this proof exercises the changed OpenClaw metadata builder directly. Full CI and local checks are supplemental below.
Verification
pnpm exec vitest run src/auto-reply/reply/inbound-meta.test.ts— 48 passed on currentupstream/mainpnpm exec oxfmt --check src/auto-reply/reply/inbound-meta.ts src/auto-reply/reply/get-reply-run.ts src/auto-reply/reply/inbound-meta.test.tsgit diff --checkpnpm tsgo:corepnpm lint:coreSafety notes
Disclosure
AI-assisted, per the repository contribution policy.