Skip to content

fix(line): canonicalize lowercased LINE chat ids at send boundary (#81628)#81670

Closed
SymbolStar wants to merge 1 commit into
openclaw:mainfrom
SymbolStar:fix/issue-81628
Closed

fix(line): canonicalize lowercased LINE chat ids at send boundary (#81628)#81670
SymbolStar wants to merge 1 commit into
openclaw:mainfrom
SymbolStar:fix/issue-81628

Conversation

@SymbolStar

@SymbolStar SymbolStar commented May 14, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #81628. LINE delivery silently dropped messages because delivery.to could reach the LINE plugin lowercased (e.g. cxxx… instead of Cxxx…); the LINE Messaging API rejects those ids with HTTP 400 The property, 'to', in the request body is invalid. Per the codex review on the issue, the fix lives at the channel-owned delivery boundary rather than in core.

Root cause

LINE chat IDs (userId / groupId / roomId) are case-sensitive (per spec: capital U / C / R followed by 32 lowercase hex chars). Several core paths can surface a lowercased value:

  • src/routing/session-key.ts lowercases peer ids when building agent session keys (buildAgentPeerSessionKey → normalizeLowercaseStringOrEmpty).
  • src/agents/tools/cron-tool.ts inferDeliveryFromSessionKey() parses that lowercased fragment and assigns it directly to delivery.to when there is no current delivery context — exactly the path used by long-running jobs whose reply token has expired.
  • src/infra/outbound/delivery-queue-recovery.ts buildRecoveryDeliverParams replays the stored params.to verbatim, so once a lowercased id is queued every retry hits the same 400 until the entry is moved to failed/.

The reply-token-bound path inside the inbound webhook keeps the original casing, which is why fast replies look fine and only post-token-expiry / long task pushes fail — matching the bug report.

Fix

Extend normalizeTarget() in extensions/line/src/send.ts (the function every push / reply / loading-animation call already routes through):

  1. Strip the existing line:(group|room|user): / line: prefixes (unchanged).
  2. If the result matches the LINE chat-id shape ^[curCUR][0-9a-f]{32}$, restore the leading letter to its canonical uppercase. Non-id-shaped strings (custom test fixtures, future formats) pass through unchanged.

This is intentionally a single channel-owned guard so we do not have to teach core about LINE casing, and so it also covers durable-recovery replays where the queued to is whatever core enqueued.

Tests

extensions/line/src/send.test.ts adds three regression cases:

  • Lowercased chat id (simulating the cron / session-key fallback path) is sent with capital first letter.
  • Durable replay-style input line:user:u<hex> is stripped and uppercased before reaching client.pushMessage.
  • Non-id-shaped recipients (e.g. U-quick) still pass through unchanged so existing fixtures keep working.

Risk / scope

  • Only touches the LINE plugin (extensions/line/src/send.ts + its test).
  • Behaviour change is gated on the LINE chat-id shape regex, so non-LINE-id inputs are unaffected.
  • Does not modify core session-key or delivery-queue logic; their lowercasing stays as-is for routing identity, but the wire-level to is now always canonical.

Verification done locally

  • git diff --stat confirms only the two files changed.
  • pnpm is not installed on this dev box (per TOOLS.md constraint), so the relevant suites listed in the codex review (extensions/line/src/send.test.ts, extensions/line/src/channel.sendPayload.test.ts, extensions/line/src/bot-message-context.test.ts, extensions/line/src/monitor-durable.test.ts, extensions/line/src/auto-reply-delivery.test.ts, src/agents/tools/cron-tool.test.ts, src/infra/outbound/delivery-queue.recovery.test.ts, src/infra/outbound/deliver.test.ts) are validated via CI.

Fixes #81628

fix(line): canonicalize lowercased LINE chat ids at send boundary
da21f7d 
LINE userId/groupId/roomId are case-sensitive (per spec: capital U/C/R
followed by 32 lowercase hex chars). The push API returns HTTP 400 when
the leading letter is lowercased.

Some core paths surface a lowercased recipient at the channel-owned
delivery boundary:

- src/routing/session-key.ts lowercases peer ids when composing agent
  session keys.
- src/agents/tools/cron-tool.ts inferDeliveryFromSessionKey() parses
  that lowercased fragment and assigns it to delivery.to when there is
  no current delivery context, which is the path used by long-running
  jobs whose reply token has expired.
- src/infra/outbound/delivery-queue-recovery.ts replays the stored
  params.to verbatim, so once a lowercased id is queued every retry
  reproduces the same 400.

Per the codex review on openclaw#81628, fix this at the channel-owned boundary
rather than spreading channel knowledge into core: extend
extensions/line/src/normalizeTarget() so that whenever the stripped
recipient matches the LINE chat-id shape (^[curCUR][0-9a-f]{32}$) we
restore the leading letter to its canonical uppercase form. Non-id
shaped strings (custom test fixtures, future formats) pass through
unchanged.

Adds three regression cases in extensions/line/src/send.test.ts
covering: lowercase chat-id input, durable replay with line:user:
prefix kept, and the existing pass-through behaviour for non-id
recipients.

Fixes openclaw#81628
@openclaw-barnacle openclaw-barnacle Bot added channel: line Channel integration: line size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 14, 2026
@clawsweeper

clawsweeper Bot commented May 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Close: the same LINE lowercased-recipient bug was fixed by the merged replacement PR, and current main now has the LINE send guard, cron delivery-context path, and regression coverage. The only meaningful leftover in this branch is the alternate auto-uppercase strategy, which is superseded by the mainline preserve-or-fail-permanently behavior.

Canonical path: Keep the merged replacement behavior on main and close this alternate branch; future cleanup should continue toward explicit delivery-context persistence rather than auto-canonicalizing invalid LINE targets.

So I’m closing this here because the remaining work is already tracked in the canonical issue.

Review details

Best possible solution:

Keep the merged replacement behavior on main and close this alternate branch; future cleanup should continue toward explicit delivery-context persistence rather than auto-canonicalizing invalid LINE targets.

Do we have a high-confidence way to reproduce the issue?

No against current main: the original source-visible path is covered by the merged replacement and later delivery-context code, and current tests cover lowercased LINE group, DM, per-peer, and send-boundary cases. The pre-fix failure was source-reproducible from session-key lowercasing and cron fallback, but that is no longer the current-main behavior.

Is this the best way to solve the issue?

No for this PR as the merge path: the replacement PR is the narrower maintained solution because it preserves case-exact stored delivery context and fails unrecoverable lowercased LINE-shaped recipients as permanent errors. Auto-uppercasing at the send boundary is now a superseded alternative.

Security review:

Security review cleared: The PR diff only changes LINE target normalization and tests; it does not add dependencies, CI changes, secrets handling, install scripts, or other supply-chain surfaces.

What I checked:

Likely related people:

  • steipete: Merged the replacement PR, posted the landing proof, authored the per-peer follow-up commit on that branch, and current blame/shortlog show repeated work across the LINE send, cron delivery, and session-key surfaces. (role: recent area contributor and merger of superseding fix; confidence: high; commits: 8338412b547f, dbd289948f30, b5ba210fd5c6; files: extensions/line/src/send.ts, src/agents/tools/cron-tool.ts, src/cron/delivery-context.ts)
  • edenfunf: Authored the merged replacement PR commits that added the LINE session-key fallback guard, LINE send-time permanent error guard, and focused regression tests. (role: superseding fix author; confidence: high; commits: e83ce4435a1d, 5dc511d36d94; files: src/agents/tools/cron-tool.ts, src/agents/tools/cron-tool.test.ts, extensions/line/src/send.ts)
  • Kaspre: Recent session-key remapping work is adjacent to the canonical lowercased session-key behavior involved in the original failure path. (role: recent session-key contributor; confidence: medium; commits: 7eefb26bc8d8; files: src/routing/session-key.ts, src/sessions/session-key-utils.ts)
  • masatohoshino: Contributed LINE outbound media work that touched the LINE send/outbound boundary and related tests, adjacent to this PR's send normalization surface. (role: LINE outbound feature contributor; confidence: medium; commits: 9449e54f4ffc; files: extensions/line/src/send.ts, extensions/line/src/outbound.ts, extensions/line/src/channel.sendPayload.test.ts)

Codex review notes: model gpt-5.5, reasoning high; reviewed against 16e5d6692dcc; fix evidence: commit 8338412b547f, main fix timestamp 2026-05-15T16:22:59Z.

@clawsweeper clawsweeper Bot mentioned this pull request May 15, 2026
Merged
@clawsweeper clawsweeper Bot closed this May 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

channel: line Channel integration: line size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: [core] LINE delivery-recovery sends wrong recipient (lowercased / session-key prefix kept), causing silent push failures

1 participant

@SymbolStar