Skip to content

fix(line): warn on inert webhook_host / webhook_port instead of seeding them - #3329

Open
ex-takashima wants to merge 3 commits into
sipeed:mainfrom
ex-takashima:fix/line-inert-webhook-bind-keys
Open

fix(line): warn on inert webhook_host / webhook_port instead of seeding them#3329
ex-takashima wants to merge 3 commits into
sipeed:mainfrom
ex-takashima:fix/line-inert-webhook-bind-keys

Conversation

@ex-takashima

@ex-takashima ex-takashima commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Fixes #3328.

Problem

line.settings.webhook_host / webhook_port are declared, defaulted, and env-bound, but nothing reads them. The LINE channel implements WebhookPath() and is mounted as a handler on the shared gateway HTTP server (pkg/channels/manager.go, m.mux.Handle(wh.WebhookPath(), wh)) — it has no listener of its own, so there is no port for it to bind. The only other references are the openclaw importer, a test fixture, and a Web UI label.

The webhook is actually served on gateway.port (default 18790), while every fresh config was seeded with webhook_port: 18791. Both numbers are plausible and adjacent, and nothing distinguishes them. Anyone putting a reverse proxy or tunnel in front of PicoClaw reads the config, points at 18791, and gets a listener that never answers — while channel-enabled, process-healthy, tunnel-connected, and credentials-valid all report fine. The failure surfaces only as webhooks silently 404ing.

Change

Option (1) from the issue — stop advertising them — plus the warning the issue suggests for configs that already set them.

  • Defaults (pkg/config/defaults.go): drop both keys from the LINE channel, so new configs no longer advertise a port that is never bound. Only webhook_path remains, which is genuinely honoured.
  • Migration (pkg/migrate/sources/openclaw/openclaw_config.go): the importer copied both keys unconditionally, so every migrated config.json got an inert "webhook_host": "", "webhook_port": 0 even when the source never set them. Now carried over only when actually set.
  • Struct (pkg/config/config.go): both fields kept, marked Deprecated:, with omitempty added. They are what makes a stale config detectable — that is the only reason to keep them, and they are no longer written back out.
  • Warning (pkg/channels/line/line.go): when either is set, log one warning at channel startup naming gateway.host / gateway.port as the real setting, and echo the effective webhook_path.

Net effect: fresh and migrated configs are clean, and a config that still sets these says so out loud instead of failing silently.

Behaviour

Startup with a legacy config carrying both keys:

WARN [line] Ignoring LINE webhook bind settings: the webhook is served by the shared gateway HTTP server
  ignored=webhook_host, webhook_port  use_instead=gateway.host / gateway.port  webhook_path=/webhook/line

Nothing else changes — the fields were inert before this PR and are inert after it. Deleting the fields outright would also have been load-compatible (unknown keys inside a channel settings block are silently ignored, since Settings is a RawNode decoded with a plain json.Unmarshal; only top-level unknown fields are a hard error) — but then a config still setting them would go on being ignored in silence, which is the actual complaint in #3328.

Tests

  • TestInertBindKeys — table test over unset / host-only / port-only / both
  • TestWebhookPathIgnoresBindSettings — path resolution unaffected by the bind keys
  • TestDefaultChannels_LINEOmitsInertWebhookBindKeys — defaults no longer seed them
  • TestLINESettings_InertWebhookBindKeysRemainLoadable — a config setting them still decodes
  • TestLINESettings_InertWebhookBindKeysNotSerializedWhenUnsetomitempty keeps them out of saved configs
  • TestToStandardConfig_LINEOmitsUnsetWebhookBindKeys / TestToStandardConfig_LINEKeepsSetWebhookBindKeys — migration output, both directions

go vet and go test ./pkg/config/... ./pkg/channels/line/... ./pkg/migrate/... pass. (pkg/channels/matrix fails to build in my container for an unrelated pre-existing reason — missing libolm C headers.)

Notes for reviewers

Web UI left aloneweb/frontend/src/components/channels/channel-forms/generic-form.tsx still maps webhook_host / webhook_port to "Webhook listening host/port" descriptions. With the keys gone from defaults and migration, the field only renders for configs that already carry it; removing the entries would also orphan the string in ~20 locale files. Happy to fold that in if you'd prefer.

Docs need no change — the issue states these are documented in docs/channels/line/README.md, but no LINE README (en/ja/zh/fr/pt-br/vi) mentions webhook_host, webhook_port, or 18791. The en README already correctly says the shared gateway serves the webhook on 18790.

🤖 Generated with Claude Code

ex-takashima and others added 3 commits August 12, 2026 01:48
…ng them

The LINE channel is mounted as a handler on the shared gateway HTTP server
(manager.go, mux.Handle(wh.WebhookPath(), wh)) and has no listener of its
own, so line.settings.webhook_host and webhook_port bind nothing. Nothing
read them: the only other references were the openclaw importer, a test
fixture, and a Web UI label.

They were still seeded into every fresh config as 0.0.0.0 / 18791 while the
webhook is actually served on gateway.port (default 18790). Both numbers are
plausible and adjacent, so anyone putting a reverse proxy or tunnel in front
of PicoClaw reads the config, points at 18791, and gets a listener that never
answers, while every other signal reports healthy.

- Drop both keys from the LINE channel defaults so new configs no longer
  advertise a port that is never bound.
- Keep the fields parseable and mark them deprecated. Removing them would
  turn every existing config that sets them into a hard "unknown field"
  load error, so they stay accepted and ignored, now with `omitempty` so
  they are not written back out.
- Warn once at channel startup when either is set, naming gateway.host /
  gateway.port as the real setting.

Fixes sipeed#3328

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… configs

The openclaw importer copied webhook_host / webhook_port unconditionally, so
every migrated config.json carried an inert `"webhook_host": "", "webhook_port": 0`
even when the source config never set them — the same misleading keys this
branch removes from the defaults. Carry them over only when actually set, so
the channel can still warn about them at startup.

Also correct the rationale comments: unknown keys inside a channel `settings`
block are silently ignored (Settings is a RawNode decoded with a plain
json.Unmarshal; only top-level unknown fields are a hard error). The fields are
kept so a config carrying them can be detected and warned about, not because
removing them would break config loading.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] line.settings.webhook_host / webhook_port are never read — they have defaults and docs but no consumer

1 participant