Fix LINE webhook retryable acknowledgement by NianJiuZst · Pull Request #102141 · openclaw/openclaw

NianJiuZst · 2026-07-08T09:57:41Z

What Problem This Solves

LINE webhook handlers acknowledged signed event requests before knowing whether local dispatch even started. If the handler rejected immediately, OpenClaw still returned 200, so LINE would not redeliver a retryable webhook even though the event was not accepted.

The same early-ACK shape existed in the standalone Express middleware, the Node webhook handler, and the shared multi-account monitor route.

Why This Change Was Made

This change introduces a bounded dispatch-acceptance step for LINE webhooks. Each entrypoint starts event handling, waits one event-loop turn to surface startup-time rejection as a 500 response, then acknowledges with 200 once dispatch is accepted. If processing continues asynchronously, later failures are still logged in the background instead of holding the provider HTTP response for the full agent turn.

The LINE message adapter now defaults to after_agent_dispatch while preserving shipped after_receive_record support. The exported Node/Express handlers and shared monitor route have a real after_receive_record path for compatibility, and unsupported policy inputs normalize to the default dispatch-acceptance behavior instead of hanging.

User Impact

Immediate LINE webhook dispatch failures can now trigger provider redelivery instead of being swallowed behind a 200 response. Normal successful turns still ACK quickly after local dispatch starts, avoiding long webhook waits while the agent run continues. Existing consumers that inspect or choose after_receive_record remain compatible.

Evidence

node scripts/run-vitest.mjs extensions/line/src/webhook-node.test.ts extensions/line/src/channel.sendPayload.test.ts extensions/line/src/monitor.lifecycle.test.ts (60 tests passed)
node scripts/run-vitest.mjs extensions/line/src/bot-handlers.test.ts (28 tests passed)
Local HTTP transcript through createLineNodeWebhookHandler:
- slow dispatch: status=200, body {"status":"ok"}, response returned before the pending handler was released
- immediate dispatch failure: status=500, body {"error":"Internal server error"}
git diff --check -- extensions/line/src/webhook-ack.ts extensions/line/src/webhook.ts extensions/line/src/webhook-node.ts extensions/line/src/outbound.ts extensions/line/src/webhook-node.test.ts extensions/line/src/channel.sendPayload.test.ts extensions/line/src/monitor.ts extensions/line/src/monitor.lifecycle.test.ts docs/channels/line.md
.agents/skills/autoreview/scripts/autoreview --mode local (clean after preserving after_receive_record support and narrowing/normalizing the exposed ACK policy)

Remote Testbox proof was not run from this checkout because the local Crabbox wrapper selected binary 0.18.0, while blacksmith-testbox requires Crabbox >=0.22.0.

clawsweeper · 2026-07-08T10:01:02Z

Codex review: found issues before merge. Reviewed July 15, 2026, 11:21 PM ET / July 16, 2026, 03:21 UTC.

Summary
Changes LINE webhook entrypoints to acknowledge after per-event acceptance, adds deadline and replay handling, expands regression coverage, exports acknowledgement callback types, and documents webhook redelivery.

PR surface: Source +467, Tests +942, Docs +10. Total +1419 across 14 files.

Reproducibility: yes. at source level. The existing early-ACK class and the patch’s immediate-rejection behavior are clear from the handlers and supplied HTTP transcript, while the reviewer’s lifecycle trace establishes the remaining delayed pre-adoption ambiguity.

Review metrics: 2 noteworthy metrics.

ACK policy surface: 1 default changed, 1 shipped policy removed. The adapter changes existing upgrade behavior rather than merely adding a retry-capable option.
Exported webhook contract: 3 callback or handler types added. These exports could become a public LINE-specific lifecycle API before the shared Plugin SDK contract is settled.

Root-cause cluster
Relationship: partial_overlap
Canonical: #108411
Summary: The related issue is canonical for the missing shared reply-lane settlement contract, while this PR contains the LINE-specific consumer, batching, replay, timeout, tests, and documentation that should remain stacked on that work.

Members:

canonical: [Feature]: Per-event turn-acceptance result for ack-first webhook channels (reply-lane admission / durable receive contract) #108411 - It tracks the shared typed outcome needed to distinguish adopted, retryable rejection, and legitimate completion without dispatch.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🐚 platinum hermit
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

[P2] Preserve the shipped ACK policy unless maintainers approve and prove an upgrade migration.
Adapt the LINE acknowledgement waiter to the canonical per-event outcome tracked by [Feature]: Per-event turn-acceptance result for ack-first webhook channels (reply-lane admission / durable receive contract) #108411.

Risk before merge

[P1] Existing LINE installations would silently move from the shipped receive-record acknowledgement policy to agent-dispatch acknowledgement, changing upgrade behavior and making webhook redelivery configuration operationally significant.
[P1] Because bare inbound-run completion cannot distinguish retryable pre-adoption rejection from legitimate no-dispatch completion, the branch can still return HTTP 200 for an event LINE should redeliver.
[P1] The new exported acknowledgement modes and callback types risk becoming a LINE-specific parallel contract before maintainers settle the general Plugin SDK lifecycle.

Maintainer options:

Stack on the canonical core result (recommended)
Preserve the shipped default and revise the strict acknowledgement path to aggregate explicit adopted, rejected, and completed-without-dispatch outcomes.
Intentionally migrate the ACK default
Maintainers may accept the compatibility change only with explicit upgrade policy and fresh-install plus upgrade proof, including redelivery-disabled installations.
Pause for the prerequisite issue
Keep this branch available but pause merge until [Feature]: Per-event turn-acceptance result for ack-first webhook channels (reply-lane admission / durable receive contract) #108411 establishes the shared lifecycle contract.

Next step before merge

[P2] The next step requires maintainer ownership of the shared Plugin SDK lifecycle and ACK-default migration decision; it is not a safe mechanical branch repair yet.

Maintainer decision needed

Question: Should LINE retain after_receive_record by default while an opt-in after_agent_dispatch policy is implemented on the canonical per-event reply-lane result?
Rationale: Choosing the permanent ACK default and Plugin SDK settlement boundary affects upgrades, other webhook channels, message-loss behavior, and the public plugin contract; line-level repair cannot safely choose that direction.
Likely owner: steipete — They are assigned and supplied the most specific current analysis of where the acceptance boundary must live.
Options:
- Preserve default, add canonical opt-in (recommended): Keep the shipped policy as default and stack an explicit dispatch-acceptance policy on the core result tracked by [Feature]: Per-event turn-acceptance result for ack-first webhook channels (reply-lane admission / durable receive contract) #108411.
- Migrate the default after core work: Finish the canonical result first, then intentionally change the default with upgrade documentation and proof for existing installations.
- Accept the LINE-local contract: Merge the current channel-specific inference while explicitly accepting its remaining false-ack and API-drift risks.

Security
Cleared: The diff introduces no new dependency source, workflow permission, secret access, lifecycle hook, artifact execution, or other concrete security or supply-chain concern.

Review findings

[P1] Keep the shipped ACK policy as the default — extensions/line/src/outbound.ts:449-450
[P1] Use a canonical result for non-adopted outcomes — extensions/line/src/webhook-ack.ts:54

Review details

Best possible solution:

Preserve after_receive_record as the compatible default, add dispatch-acceptance as an explicit policy backed by the canonical per-event core result, and retain this PR’s LINE batch aggregation, replay lifecycle, deadline, tests, and redelivery documentation as the consumer.

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

Yes at source level. The existing early-ACK class and the patch’s immediate-rejection behavior are clear from the handlers and supplied HTTP transcript, while the reviewer’s lifecycle trace establishes the remaining delayed pre-adoption ambiguity.

Is this the best way to solve the issue?

No. The LINE-specific machinery is useful, but interpreting callback or promise settlement inside the channel is not the narrowest maintainable fix; it should consume a canonical core per-event outcome and preserve the shipped default unless maintainers approve migration.

Full review comments:

[P1] Keep the shipped ACK policy as the default — extensions/line/src/outbound.ts:449-450
This still flips LINE from after_receive_record to after_agent_dispatch and removes the shipped policy from the supported list. Existing installations can therefore acquire new redelivery and failure semantics on upgrade. Preserve after_receive_record as the default and supported policy, then expose strict dispatch acceptance explicitly unless maintainers approve and prove a migration.
Confidence: 0.99
[P1] Use a canonical result for non-adopted outcomes — extensions/line/src/webhook-ack.ts:54
The new waiter treats handler completion without an adoption callback as ACK-safe, but core admission can resolve without distinguishing retryable busy or lifecycle rejection from a legitimate handled, dropped, or observe-only event. That can still return 200 and suppress redelivery. Aggregate a typed core adopted/rejected/completed-without-dispatch result instead of inferring safety from bare completion.
Confidence: 0.96

Overall correctness: patch is incorrect
Overall confidence: 0.97

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 957cc81175a3.

Label changes

Label justifications:

P1: The PR addresses silent inbound message loss, but its remaining false-ack path can still suppress LINE redelivery for real users.
merge-risk: 🚨 compatibility: The patch changes LINE’s shipped default ACK policy and expands exported webhook handler contracts.
merge-risk: 🚨 message-delivery: Returning HTTP 200 for an unresolved retryable pre-adoption outcome can cause LINE to discard an inbound event permanently.
rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🐚 platinum hermit and patch quality is 🦐 gold shrimp.
status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): The PR includes after-fix live HTTP output from the real Node webhook handler showing slow-dispatch and immediate-rejection responses; this proves the changed handler behavior, although it does not resolve the architectural correctness blockers.
proof: sufficient: Contributor real behavior proof is sufficient. The PR includes after-fix live HTTP output from the real Node webhook handler showing slow-dispatch and immediate-rejection responses; this proves the changed handler behavior, although it does not resolve the architectural correctness blockers.

Evidence reviewed

PR surface:

Source +467, Tests +942, Docs +10. Total +1419 across 14 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	9	584	117	+467
Tests	4	984	42	+942
Docs	1	14	4	+10
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	14	1582	163	+1419

What I checked:

Shipped policy changed: The PR changes LINE from after_receive_record to after_agent_dispatch and removes the former from supportedAckPolicies, despite prior review requesting preservation of the shipped receive policy. (extensions/line/src/outbound.ts:449, bc21c897583a)
Incomplete settlement inference: The new acknowledgement helper treats handler completion without an adoption callback as an ACK-safe fallback, but a resolved inbound run does not distinguish retryable admission rejection from legitimate completion without dispatch. (extensions/line/src/webhook-ack.ts:54, bc21c897583a)
Canonical adoption signal exists: Current main has an onTurnAdopted signal threaded into the turn kernel, but it represents only the adopted half of the required webhook settlement contract. (src/channels/turn/kernel.ts:194, 957cc81175a3)
Non-adopted outcomes remain ambiguous: Current reply-lane admission can finish with skipped or lifecycle-invalidated outcomes without providing LINE a typed retry-versus-ack result. (src/auto-reply/reply/dispatch-from-config.lifecycle.ts:247, 957cc81175a3)
Maintainer-side design investigation: The assigned reviewer reported that the LINE callback boundary still precedes decisive lane ownership and requested a canonical reply-lane admission or durable-receive contract before acknowledgement.
Canonical prerequisite: The open issue [Feature]: Per-event turn-acceptance result for ack-first webhook channels (reply-lane admission / durable receive contract) #108411 tracks a typed per-event adopted, rejected, or completed-without-dispatch result and explicitly describes this PR as the intended LINE-side consumer.

Likely related people:

steipete: They traced the LINE callback through reply-lane admission, identified the remaining false-ack boundary, and specified the canonical contract needed before merge. (role: assigned reviewer and current design investigator; confidence: high; files: extensions/line/src/bot-handlers.ts, extensions/line/src/webhook-ack.ts, src/channels/turn/kernel.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

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 keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Review history (7 earlier review cycles)

reviewed 2026-07-08T10:08:43.148Z sha 2afc69d :: needs real behavior proof before merge. :: [P1] Keep the shipped LINE ACK policy supported
reviewed 2026-07-08T10:19:11.202Z sha 2afc69d :: needs real behavior proof before merge. :: [P1] Keep the shipped LINE ACK policy supported
reviewed 2026-07-08T10:37:41.523Z sha 8a1bf70 :: needs maintainer review before merge. :: none
reviewed 2026-07-08T10:49:09.136Z sha 8a1bf70 :: needs changes before merge. :: [P2] Make the webhook timer binding const
reviewed 2026-07-08T14:10:56.647Z sha 4907dcd :: needs maintainer review before merge. :: none
reviewed 2026-07-13T12:02:16.501Z sha 331fe0e :: found issues before merge. :: [P1] Preserve the shipped receive ACK policy
reviewed 2026-07-13T13:09:05.956Z sha 34631c9 :: found issues before merge. :: [P1] Preserve the shipped receive ACK policy

steipete

Requesting changes before this can land.

waitForLineWebhookDispatchAcceptance() treats a zero-delay timer as successful after_agent_dispatch acceptance. That only proves the promise stayed pending for one event-loop turn. An asynchronous failure during allowlist/media preparation, session recording, or dispatch admission can reject later, after all three HTTP paths have already sent 200; LINE then cannot redeliver. A webhook containing multiple events is weaker still, because the batch is acknowledged without proving that each later event reached a dispatch boundary.

Please replace the timer heuristic with an explicit per-event acceptance signal from the inbound turn owner. The request should acknowledge only after every event is either accepted for agent dispatch or completes without requiring dispatch. Add regressions for a delayed pre-dispatch rejection and a multi-event callback.
The channel setup docs need to tell operators to enable Webhook redelivery in LINE Developers Console. LINE documents that redelivery is disabled by default and that non-2xx responses are redelivered only when it is enabled: receiving messages and webhook error/timeout behavior.

The intent is sound, and the shared Node/Express/monitor plumbing is a good direction. The current implementation does not yet uphold its declared acknowledgement policy, so I cannot land it at this head.

steipete · 2026-07-12T00:44:10Z

Maintainer follow-up after attempting the explicit per-event acceptance design locally:

The necessary acceptance point is deeper than the current LINE callback surface. The inbound callback still runs before runDispatch() and before reply-lane admission/session initialization/config resolution. Signaling success there would still return 200 for later pre-dispatch failures, so the event could be lost instead of redelivered.

A correct fix needs a canonical reply-lane admission/durable receive contract exposed through the Plugin SDK, then LINE can aggregate that explicit result for every event in the webhook batch. The acceptance signal must occur only after the lane owns the turn; failures before that point must reject the webhook. Please keep the delayed-failure and multi-event regressions, and retain the operator docs for enabling LINE Webhook redelivery.

I am not pushing the experimental local callback patch because it would only move the false-ack boundary rather than remove it.

steipete · 2026-07-17T05:14:39Z

AI-assisted (approved by Peter)

Thank you @NianJiuZst for identifying the retryable-ack loss mode and contributing the original work. The maintainer rework has landed in #109655, with your co-author credit preserved in the landed change.

The landed fix uses the decided SQLite spool-first architecture: durable admission before HTTP 200, channel-owned retry/replay, and typed terminal dead-letter states. Closing this superseded PR in favor of the landed rework.

openclaw-barnacle Bot added channel: line Channel integration: line size: M labels Jul 8, 2026

NianJiuZst marked this pull request as ready for review July 8, 2026 10:14

openclaw-barnacle Bot added the docs Improvements or additions to documentation label Jul 8, 2026

NianJiuZst force-pushed the codex/line-webhook-retryable-ack branch 2 times, most recently from 0680a15 to 1b98343 Compare July 8, 2026 13:45

clawsweeper Bot added the merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. label Jul 8, 2026

steipete requested changes Jul 11, 2026

View reviewed changes

steipete self-assigned this Jul 11, 2026

NianJiuZst force-pushed the codex/line-webhook-retryable-ack branch from 4907dcd to 331fe0e Compare July 13, 2026 10:49

openclaw-barnacle Bot added size: L and removed size: M labels Jul 13, 2026

openclaw-barnacle Bot added size: XL and removed size: L labels Jul 13, 2026

clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jul 13, 2026

NianJiuZst added 8 commits July 15, 2026 20:44

fix(line): acknowledge webhook events after reply-lane adoption

0eac345

fix(line): serialize group history at reply-lane adoption

db983fa

fix(line): retry every pre-acceptance failure

3f73348

fix(line): address webhook review and CI

f466704

fix(line): preserve legacy webhook dispatch acknowledgements

7393df1

refactor(line): declare webhook acceptance on the bot

d2477c0

fix(line): make webhook deadline retries effective

f3448cf

fix(line): finalize group history after acceptance

bc21c89

NianJiuZst force-pushed the codex/line-webhook-retryable-ack branch from 34631c9 to bc21c89 Compare July 15, 2026 12:53

edenfunf mentioned this pull request Jul 15, 2026

[Feature]: Per-event turn-acceptance result for ack-first webhook channels (reply-lane admission / durable receive contract) #108411

Closed

steipete mentioned this pull request Jul 17, 2026

fix(line): preserve webhook events through dispatch failures #109655

Merged

steipete closed this Jul 17, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Fix LINE webhook retryable acknowledgement#102141

Fix LINE webhook retryable acknowledgement#102141
NianJiuZst wants to merge 8 commits into
openclaw:mainfrom
NianJiuZst:codex/line-webhook-retryable-ack

NianJiuZst commented Jul 8, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jul 8, 2026 •

edited

Loading

Uh oh!

steipete left a comment

Uh oh!

steipete commented Jul 12, 2026

Uh oh!

steipete commented Jul 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

NianJiuZst commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

clawsweeper Bot commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

steipete left a comment

Choose a reason for hiding this comment

Uh oh!

steipete commented Jul 12, 2026

Uh oh!

steipete commented Jul 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

NianJiuZst commented Jul 8, 2026 •

edited

Loading

clawsweeper Bot commented Jul 8, 2026 •

edited

Loading