Open
Conversation
Contributor
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (4 files)
Reviewed by gpt-5.5-2026-04-23 · 661,817 tokens |
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
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.
Summary
Every inbound email to a bot has been failing with HTTP 400 since the OpenClaw 2026.4.23 image finished rolling out. OpenClaw refuses any hook mapping that resolves
sessionKeyfrom a request body unlesshooks.allowRequestSessionKey: trueis set. Thecloudflare-email-inboundmapping usessessionKey: '{{payload.sessionKey}}'so the platform worker can compute a stable key likeinbound-email:YYYY-MM-DD-<slug>and coalesce one thread into a single agent session. Without the flag, OpenClaw returned{"error":"Hook rejected","hookStatus":400}for every email, the controller faithfully proxied that, and the bot never woke.This change sets the flag in
generateBaseConfigand backfills it on existing configs viasanitizeExistingConfigBeforeDoctor, so running instances pick up the fix on the next controller restart with no manual reprovision.While verifying the fix, two adjacent issues surfaced and are repaired in the same PR:
kiloclaw-inbound-emailworker was missinglogpush: true, so its trace events never reached thecloudflare-logpushAxiom dataset even thoughobservability.enabledwas set. The other kiloclaw workers already had the flag. Added it and a short README explaining the requirement.errorvalue to a structured log keycontrollerErrorMessage, so the next downstream regression is filterable in Axiom without needing to SSH into a running instance.Verification
http://127.0.0.1:3001/hooks/emailfrom inside a Fly instance returned HTTP 400 with the exactsessionKey is disabled for externally supplied hook payload valuesbody, confirming the root cause before applying any code change.payload.sessionKeyreturned 200 with an automatically generated session id, and unmapped paths like/hooks/testreturned 404. The flag only widens acceptance for the existing inbound email mapping.cloudflare-email-inboundis the only mapping that templatessessionKeyfrom payload, so other webhook flows are not in scope.falseback totrue, and that the bootstrap sanitize step backfills the flag on an existing config that lacked it.pnpm run lint,pnpm run typecheck, andpnpm run format:checkall clean.inbound email controller response { status: 200 }in Axiom, and confirm the bot opens aninbound-email:YYYY-MM-DD-<slug>session.ScriptName == "kiloclaw-inbound-email"to confirm the worker logs are now reaching the dataset.kiloclaw-inbound-email-dlqonce normal delivery is confirmed.Visual Changes
N/A
Reviewer Notes
ensureInboundEmailHookFlagsdeliberately overrides any existing value ofhooks.allowRequestSessionKey, including an explicitfalse. The reasoning lives in the function comment and a vitest case: thecloudflare-email-inboundmapping is force installed bygenerateBaseConfigon every run, so the flag it depends on must converge alongside the mapping. An operator who needs to disable inbound email entirely should clear thekiloclaw_instances.inbound_email_enabledcolumn, not flip this flag.A separate regression with the same symptom timeline (Apr 22 success, May 7 failure shown on the trigger requests page) was traced during this investigation to the same
feat(kilo-chat): rip out stream chatcommit. That regression sits inservices/webhook-agent-ingest: the queue consumer atqueue-consumer.ts:175still POSTs to/api/platform/send-chat-message, a route the same commit deleted with no replacement. The kiloclaw worker now returns "Authentication required" for that unmatched path, and captured webhook requests end up in thefailedstate. That fix is intentionally scoped to a separate PR because it requires reimplementing the chat delivery on top of the chat plugin and warrants coordination with the original author.