From 85e6f216b55dc4373ea5c27c7d42c8557ed5462b Mon Sep 17 00:00:00 2001 From: Plateau Nguyen Date: Wed, 15 Apr 2026 22:26:13 +0700 Subject: [PATCH] feat(channels): Pancake comment reply fix + platform select + UI improvements (#904) * feat(channels): make Pancake platform a required select with 11 options - Add mandatory platform select field (11 options) to Pancake channel schema - Add config required validation on channel instance form (create-only) - Add i18n fieldOptions/fieldConfig for platform in en/vi/zh locales - Add TDD tests for Pancake config schema (channel-schemas.test.ts) - Add backend test TestFactoryExplicitPlatformPreserved - Add slog.Debug for auto-detect path in pancake.go - Update Platform field comment in types.go for clarity - Update plan status to completed; add changelog entry * feat(channels): hide comment_reply for non-social Pancake platforms Extend showWhen to accept string | string[] values. Apply it to features.comment_reply so the field only appears for platforms that support public posts/comments (facebook, instagram, threads, tiktok, youtube). E-commerce platforms (shopee, lazada, tokopedia) and messaging-only platforms (line, google, chat_plugin) no longer show the irrelevant Comment Reply toggle. Also guard depValue before String() coercion to avoid the "undefined" literal matching hazard. * feat(channels): Pancake comment reply fix + platform select + UI improvements - Fix comment reply: pass message_id (reply_to_comment_id) to Pancake API ReplyComment now requires messageID param; guard added for empty ID - Add SendMessageRequest.MessageID field (omitempty) for reply_comment action - Platform select: required field with 11 options, showWhen gate for comment_reply - Webhook Page ID moved to Advanced collapsible section in channel form (auto-expands when existing value is configured) - routing_metadata.go: centralize routing metadata key constants - config-flatten.ts: flatten/unflatten nested config for form state Closes: Pancake comment reply returns 'Missing required field: message_id' --- cmd/gateway_consumer_normal.go | 13 +- docs/17-changelog.md | 7 ++ ...04-14-pancake-platform-mandatory-select.md | 85 +++++++++++++ .../2026-04-14-routing-metadata-refactor.md | 75 +++++++++++ ...60412-pancake-comment-reply-first-inbox.md | 39 ++++++ internal/channels/events.go | 26 ---- internal/channels/pancake/api_client.go | 8 +- internal/channels/pancake/api_client_test.go | 7 +- internal/channels/pancake/comment_handler.go | 6 + .../channels/pancake/comment_handler_test.go | 34 +++++ internal/channels/pancake/pancake.go | 16 ++- internal/channels/pancake/pancake_test.go | 119 ++++++++++++++++-- internal/channels/pancake/types.go | 10 +- internal/channels/pancake/webhook_handler.go | 32 +++-- internal/channels/routing_metadata.go | 43 +++++++ internal/channels/routing_metadata_test.go | 39 ++++++ ui/web/src/i18n/locales/en/channels.json | 16 ++- ui/web/src/i18n/locales/vi/channels.json | 16 ++- ui/web/src/i18n/locales/zh/channels.json | 16 ++- ui/web/src/lib/config-flatten.ts | 70 +++++++++++ ui/web/src/pages/channels/channel-fields.tsx | 6 +- .../channels/channel-instance-form-dialog.tsx | 20 ++- .../channels/channel-instance-form-step.tsx | 27 +++- .../pages/channels/channel-schemas.test.ts | 43 +++++++ ui/web/src/pages/channels/channel-schemas.ts | 30 ++++- 25 files changed, 724 insertions(+), 79 deletions(-) create mode 100644 docs/journals/2026-04-14-pancake-platform-mandatory-select.md create mode 100644 docs/journals/2026-04-14-routing-metadata-refactor.md create mode 100644 docs/journals/260412-pancake-comment-reply-first-inbox.md create mode 100644 internal/channels/routing_metadata.go create mode 100644 internal/channels/routing_metadata_test.go create mode 100644 ui/web/src/lib/config-flatten.ts create mode 100644 ui/web/src/pages/channels/channel-schemas.test.ts diff --git a/cmd/gateway_consumer_normal.go b/cmd/gateway_consumer_normal.go index a3167b80..72dd996d 100644 --- a/cmd/gateway_consumer_normal.go +++ b/cmd/gateway_consumer_normal.go @@ -11,6 +11,7 @@ import ( "github.com/nextlevelbuilder/goclaw/internal/agent" "github.com/nextlevelbuilder/goclaw/internal/bus" + "github.com/nextlevelbuilder/goclaw/internal/channels" "github.com/nextlevelbuilder/goclaw/internal/channels/telegram/voiceguard" "github.com/nextlevelbuilder/goclaw/internal/i18n" "github.com/nextlevelbuilder/goclaw/internal/scheduler" @@ -214,22 +215,12 @@ func processNormalMessage( // Build outbound metadata for reply-to + thread routing BEFORE RegisterRun // so block.reply handler can use it for routing intermediate messages. - outMeta := make(map[string]string) + outMeta := channels.CopyFinalRoutingMeta(msg.Metadata) if isGroup { if mid := msg.Metadata["message_id"]; mid != "" { outMeta["reply_to_message_id"] = mid } } - // Channel routing keys — keep in sync with routingMetaKeys in channels/events.go. - for _, k := range []string{ - tools.MetaMessageThreadID, "local_key", "placeholder_key", "group_id", - "feishu_reply_target_id", - "fb_mode", "sender_id", "page_id", "reply_to_comment_id", - } { - if v := msg.Metadata[k]; v != "" { - outMeta[k] = v - } - } // Register run with channel manager for streaming/reaction event forwarding. // Use localKey (composite key with topic suffix) so streaming/reaction events diff --git a/docs/17-changelog.md b/docs/17-changelog.md index 98c170b0..e23f1d51 100644 --- a/docs/17-changelog.md +++ b/docs/17-changelog.md @@ -282,6 +282,13 @@ Tenant admins can override tool configuration without affecting other tenants. O ### Fixed +#### Pancake Platform Field — Mandatory Select (2026-04-14) + +- **`platform` field changed from optional text to required dropdown**: `channel-schemas.ts` replaces the free-text `platform` field with a `select` type (11 options: facebook, instagram, threads, tiktok, youtube, shopee, line, google, chat_plugin, lazada, tokopedia). `required: true` is now enforced by the form dialog on create. +- **Config required-field validation (create-only)**: `channel-instance-form-dialog.tsx` now enforces `required` on config fields at submit time. Validation is scoped to create flows only — edit flows are unchanged to avoid silent breakage on existing channels with no stored `platform`. +- **i18n**: Platform field label options and config field metadata added to `channels.json` for en/vi/zh. +- **Auto-detect log level**: `pancake.go` auto-detect block demoted from `slog.Info` to `slog.Debug` to stop log noise on every channel start; auto-detect remains as a fallback for existing channels with no stored `platform`. + #### Feishu/Lark Writer Management Commands — Issue #818 Closed (2026-04-11) - **Issue #818 resolution**: Closes UX gap where users saw `/addwriter` error messages but Feishu had no handler - **Phase 1 — Thread reply routing**: Inbound messages with `thread_id` now properly route responses back to the same Feishu thread via `/open-apis/im/v1/messages/{id}/reply`. New `feishu_reply_target_id` metadata key included in `routingMetaKeys` allowlist. Graceful fallback to `SendMessage()` if thread root deleted diff --git a/docs/journals/2026-04-14-pancake-platform-mandatory-select.md b/docs/journals/2026-04-14-pancake-platform-mandatory-select.md new file mode 100644 index 00000000..408d876d --- /dev/null +++ b/docs/journals/2026-04-14-pancake-platform-mandatory-select.md @@ -0,0 +1,85 @@ +# Pancake Platform: Mandatory Dropdown + Form Validation Gap Fix + +**Date**: 2026-04-14 09:00 +**Severity**: Medium +**Component**: `ui/web/src/pages/channels/`, `internal/channels/pancake/` +**Status**: Resolved + +## What Happened + +Pancake channel creation let operators leave the `platform` field blank or type a freeform string. Auto-detection from webhook payloads was the supposed fallback — but auto-detection is best-effort and silent-wrong when it fires on an ambiguous payload. An operator could create a Pancake channel, send messages through it for days, and only discover the platform was misidentified when behavior diverged between Facebook and Instagram routing. + +Separately, the channel form had no pre-submit validation on `required` config fields. The schema marked fields as `required: true` but the form dialog never checked — it shipped empty strings to the backend and let the backend fail. + +Both were fixed: platform field is now a required dropdown with 11 explicit options, and the form dialog enforces `required` config fields on submit (create path only, to avoid breaking legacy channels with no platform stored in DB). + +## The Brutal Truth + +The "auto-detect as fallback" design was wishful thinking. Auto-detection works when every webhook payload has a reliable, consistent platform discriminator. It doesn't work when platform discrimination is inferred from message shape rather than an explicit field. Shipping a form with an optional platform and crossing fingers on runtime detection was a correctness gamble that never needed to be made — the user knows which platform they're configuring at creation time. + +The `required` validation gap is more embarrassing. The schema system had a `required` property on field descriptors, but the form dialog never read it before submitting. The property was there for documentation purposes only. This is the worst kind of dead code: it looks like it works, reviewers assume it works, and it silently fails to enforce anything. + +## Technical Details + +Schema change in `channel-schemas.ts`: + +```ts +// Before +{ key: "platform", type: "text", required: false, hint: "Auto-detected if empty" } + +// After +{ key: "platform", type: "select", required: true, defaultValue: "", + options: [ + { value: "facebook", label: "Facebook" }, + { value: "instagram", label: "Instagram" }, + // ... 9 more + ] } +``` + +Validation block added in `channel-instance-form-dialog.tsx`, scoped to create-only: + +```ts +if (!isEditing) { + const missing = schema.configFields + .filter(f => f.required && !cleanConfig[f.key]) + .map(f => f.key); + if (missing.length > 0) { + setErrors({ config: missing }); + return; + } +} +``` + +Validated against `cleanConfig` (post empty-string strip), not `configValues` — catches both `undefined` and `""` from an unselected dropdown. Validating `configValues` would have missed the `""` case because `cleanConfig` strips empty strings before the check. + +Backend: `pancake.go` auto-detect log demoted from `slog.Info` to `slog.Debug`. The previous level would have spammed logs for every legacy channel restart indefinitely. + +Test run: 22 UI tests pass (`channel-schemas.test.ts` + pre-existing suite). Go pancake tests pass. PG and SQLite builds clean. + +## What We Tried + +No dead ends. TDD approach: wrote `channel-schemas.test.ts` with 5 failing assertions (Red), implemented the schema change, confirmed Green (22/22). The test-first pass caught a defaultValue omission before it would have caused a subtle unset-state bug in the dropdown. + +## Root Cause Analysis + +Two independent oversights: + +1. Platform field: schema author assumed auto-detection was robust enough to make explicit selection optional. It isn't. Explicit always beats inferred for configuration that has deterministic user knowledge at creation time. + +2. Required validation: the `required` property was added to the schema descriptor type without a corresponding enforcement pass in the form dialog. Classic schema-as-documentation antipattern — the property communicates intent but enforces nothing. No test existed to verify that submitting a blank required field was actually blocked. + +## Lessons Learned + +1. **If the user knows it at creation time, make them set it.** "Auto-detect as fallback" is only valid when the value genuinely cannot be known upfront. Platform is a deliberate operator choice, not a runtime observable. + +2. **Schema properties that imply behavior must be enforced.** `required: true` in a field descriptor is a promise to the operator. If the form doesn't read it, the promise is a lie. Any time a new semantic property is added to a schema type, ask immediately: "where does this actually get enforced?" + +3. **Validate `cleanConfig`, not `configValues`.** The form strips empty strings from config before submission. Validating before the strip means `""` from an unselected dropdown bypasses the required check. Validate the value in the same form it will be submitted. + +4. **Edit-path must not block legacy data.** Existing Pancake channels in production have no `platform` stored — they relied entirely on auto-detection. Making `platform` required on edit would lock operators out of editing those channels until they backfill the field. Create-only enforcement + runtime auto-detect fallback is the right seam. + +## Next Steps + +- [ ] Confirm exact Pancake API keys for `threads`, `youtube`, `google`, `chat_plugin` against live Pancake docs or a real webhook payload. Current values are best-guess from naming convention — if any are wrong, routing will silently mismatch. Owner: whoever next handles a Pancake integration with those platforms. +- [ ] Backfill migration or UI prompt for existing Pancake channels with no `platform` stored — auto-detect will cover them at runtime but operators have no visibility into which platform was inferred. Low urgency, high debuggability value. +- [ ] Extend required-field validation to the edit path once legacy channels have been audited. Right now edit path has no guard. Owner: channels team, after confirming no production channel has an unset required field. diff --git a/docs/journals/2026-04-14-routing-metadata-refactor.md b/docs/journals/2026-04-14-routing-metadata-refactor.md new file mode 100644 index 00000000..b37da538 --- /dev/null +++ b/docs/journals/2026-04-14-routing-metadata-refactor.md @@ -0,0 +1,75 @@ +# Routing Metadata Refactoring + Pancake Comment Diagnostics + +**Date**: 2026-04-14 03:00 +**Severity**: High +**Component**: `internal/channels`, `internal/channels/pancake`, `cmd/gateway_consumer_normal.go` +**Status**: Resolved + +## What Happened + +Pancake page comment replies were landing in inbox instead of the public comment thread. The inbound handler correctly stamped `pancake_mode=comment` onto message metadata, but outbound delivery discarded that key before it reached `Send()`. Every comment reply quietly defaulted to inbox routing. No error, no log, no crash — just silent wrong behavior that required a live test to detect. + +The fix cascaded into a proper structural refactor: extracted all routing key definitions and copy helpers into `internal/channels/routing_metadata.go`, added `pancake_mode` to the canonical key set, switched `gateway_consumer_normal.go` to use `channels.CopyFinalRoutingMeta()` instead of its own private inline loop, and wrapped the feature-disabled log in `sync.Once` to prevent spam. 16 new comment handler tests and 2 routing_metadata unit tests were added. All 77 channels package tests and 64 pancake package tests pass. + +## The Brutal Truth + +This was not subtle. The metadata key list in `cmd/gateway_consumer_normal.go` and the one in `internal/channels/events.go` had already diverged before `pancake_mode` was ever needed. We had two separate hardcoded string slices doing the same job in different files, neither derived from the other. Adding a new routing key meant updating two places, and we missed one. Classic DRY violation with delayed detonation. + +The maddening part: the bug was completely silent. No test caught it because there were no tests for metadata preservation across the inbound→outbound hop. The only signal was a live comment reply going to the wrong place. We were testing whether messages arrived, not whether they arrived *correctly routed*. + +The `sync.Once` diagnostic fix is also a minor embarrassment. The original `slog.Info` on every disabled comment event would have hammered logs the moment any page with `comment_reply=false` received traffic. It should have been gated from the start. `sync.Once` is the minimal fix, but the real lesson is: always ask "how often will this fire in production?" before adding a per-event log inside a hot path. + +## Technical Details + +Root cause path: + +1. `pancake/comment_handler.go` sets `metadata["pancake_mode"] = "comment"` (line 77) +2. `cmd/gateway_consumer_normal.go` line 218: `outMeta := channels.CopyFinalRoutingMeta(msg.Metadata)` — **before the fix** this was an inline loop over a hardcoded key slice that did not include `pancake_mode` +3. `outMeta` is passed into `ChannelMgr.RegisterRun()` and then forwarded to all outbound publishes +4. `pancake/pancake.go` `Send()` checks `msg.Metadata["pancake_mode"]` to branch between comment reply API and inbox reply API — empty key → inbox branch + +The surviving inline loop in `events.go` block.reply path (lines 271-275) still reads directly from `routingMetaKeys` (now the canonical exported slice), which is technically correct but still not using the `copyRoutingMeta()` helper. Code review flagged this as informational — non-blocking because block replies intentionally exclude `placeholder_key` and the keys read are identical to what `copyRoutingMeta()` would copy. But it is residual inconsistency. + +`finalReplyMetaKeys` design: + +```go +var finalReplyMetaKeys = append([]string{ + "placeholder_key", // final outbound can update placeholder; block replies must not +}, routingMetaKeys...) +``` + +`placeholder_key` is in `finalReplyMetaKeys` only. Block replies use `routingMetaKeys` (no `placeholder_key`). If a block reply carried `placeholder_key`, the placeholder message would be overwritten mid-stream before the final response was ready. Keeping the two lists distinct was the right call. + +Test counts: `go test ./internal/channels/... -count=1` → 77 pass, 0 fail. `go test ./internal/channels/pancake/... -count=1` → 64 pass, 0 fail. + +Code review findings (all informational, non-blocking): +- `events.go` block.reply path still uses inline loop — not using `copyRoutingMeta()` helper +- `routing_metadata_test.go` missing nil-map input test for `copySelectedMeta` +- `sync.Once` on `commentReplyDisabledOnce` cannot be reset between tests — `sync.Once` reinit path untested + +## What We Tried + +No dead ends this session. The root cause was identified immediately from the previous investigation journal (`260414-0225-pancake-comment-routing-metadata-fix.md`). This session was the structural cleanup and test coverage pass that followed the point-fix. + +## Root Cause Analysis + +Two metadata preservation code paths were maintained independently with no shared contract. When `pancake_mode` was added as a routing key in the pancake channel, it was added to the inbound metadata map but there was no mechanism to enforce that all outbound copy paths would automatically include it. The lack of a canonical routing key registry meant every place that copied metadata had to be manually updated — and one wasn't. + +Secondary cause: zero tests existed for the metadata→outbound preservation hop. The inbound handler was tested (produces metadata), the outbound channel was tested (accepts metadata), but the pipeline connecting them was not. The gap was invisible until production routing broke. + +## Lessons Learned + +1. **Routing keys must have one source of truth.** Any code that copies a subset of metadata for routing must derive from a shared registry. Ad-hoc key lists in consumer code are a time bomb. `routing_metadata.go` is that registry now — it must stay the only place key lists live. + +2. **Test the pipeline seam, not just the endpoints.** Testing that inbound sets a metadata key and testing that outbound reads a metadata key is not the same as testing that the key survives the round trip. The round-trip test (`TestHandleCommentEvent_FeatureEnabled` checking `msg.Metadata["pancake_mode"] == "comment"`) is the test that would have caught this bug before it shipped. + +3. **Per-event logs in hot paths need rate controls from day one.** `slog.Info` inside a webhook event handler that fires on every Facebook comment is a logging DoS waiting to happen. If a diagnostic log is informational and fires on a repeatable condition (feature disabled), use `sync.Once` or a rate limiter. Don't retrofit it after the fact. + +4. **Separation of `placeholder_key` from routing keys is a load-bearing design choice.** If this distinction is ever collapsed or "simplified," block replies will corrupt placeholders mid-stream. Future developer: do not merge `finalReplyMetaKeys` and `routingMetaKeys` without understanding this. + +## Next Steps + +- [ ] Refactor `events.go` block.reply path to call `copyRoutingMeta()` instead of the remaining inline loop — eliminates the last inconsistency flagged in code review. Owner: any engineer touching `events.go` next. No urgency, non-blocking. +- [ ] Add nil-map safety test for `copySelectedMeta(nil, keys)` in `routing_metadata_test.go` — current implementation calls `src[k]` on nil map which panics in Go. Owner: next person adding routing metadata tests. **This is a latent panic, not just a missing test.** +- [ ] Add outbound Pancake action log (`reply_comment` vs `reply_inbox`) so operators have runtime proof of which branch fires without needing to read source code. Owner: pancake channel work. +- [ ] Evaluate whether all channel-specific routing keys should be defined in one canonical schema to prevent the same drift happening for future channels. This is architectural, needs team discussion before acting. diff --git a/docs/journals/260412-pancake-comment-reply-first-inbox.md b/docs/journals/260412-pancake-comment-reply-first-inbox.md new file mode 100644 index 00000000..f0db067f --- /dev/null +++ b/docs/journals/260412-pancake-comment-reply-first-inbox.md @@ -0,0 +1,39 @@ +# 2026-04-12 Pancake Comment Auto-Reply + First Inbox + +## Summary + +Extended Pancake (pages.fm) channel to handle comment-on-post conversations and first-inbox auto-replies in parallel with existing inbox flows. Implemented 5 phases: types/API client, post fetcher with singleflight, comment filtering/routing, webhook router split, and send path refactor. All flows remain isolated; zero impact on inbox stability. + +## Key Changes + +- **Types:** Added `Features.FirstInbox`, `CommentReplyOptions` (filter/keywords/includePostContext), `PostID` to conversations, `PancakePost` type +- **API Client:** New `ReplyComment()`, `PrivateReply()`, `GetPosts()` methods with action-based routing +- **Post Fetcher:** New `post_fetcher.go` with `sync.Map` cache + `singleflight` dedup (key: `"posts"` because Pancake requires list-all, not per-post lookup) +- **Comment Handler:** New `comment_handler.go` with feature gate, self-reply prevention, staff skip, dedup by `comment:{msgID}`, keyword/all filter, echo dedup, post context enrichment +- **Webhook Router:** `webhook_handler.go` split `if convType != "INBOX"` into `switch` cases for INBOX vs COMMENT with PostID normalization +- **Send Path:** `pancake.go` refactored `Send()` into `sendInboxReply()` / `sendCommentReply()` based on `metadata["pancake_mode"]`; added `sendFirstInbox()` with `sync.Map` dedup + +## Design Decisions + +- **ChatID for comments = ConversationID:** Pancake groups comment conversations per sender per post internally (unlike Facebook where we constructed `postID:senderID` ourselves) +- **sendFirstInbox error handling:** Delete `firstInboxSent` entry on error to allow retry, not the stored time. Allows operator to re-trigger without waiting for TTL +- **Singleflight key scope:** All posts fetched with key `"posts"` (not per-postID) because Pancake API requires `GetPosts()` list-all, not per-post lookup. Prevents API stampede on concurrent comment arrivals for different posts +- **Additive architecture:** INBOX flow completely unchanged. Comment flow runs parallel. Feature gate ensures zero blast radius + +## Test Coverage + +- 72 tests pass with `-race` (integration + unit) +- Both `go build ./...` (PG) and `go build -tags sqliteonly ./...` (SQLite) clean +- `go vet ./...` clean +- Post fetcher, comment handler, send path all tested with dedup/concurrency/negative cache scenarios + +## Follow-up Fixes + +Code review fixes applied (PR #841 feedback from @mrgoonie): +- Added `slog.Debug` on post context fetch failure in `buildCommentContent()` +- Documented `GetPosts()` bypass of `doRequest()` (response body parsing requirement) +- Added 72h TTL eviction in `runDedupCleaner()` for `firstInboxSent` sync.Map (prevents unbounded growth) +- Wrapped `sendCommentReply()` with `context.WithTimeout(30s)` (bounds API hang risk) +- Replaced custom `containsStr` test helper with `strings.Contains` + +All non-blocking, no architecture changes. diff --git a/internal/channels/events.go b/internal/channels/events.go index 21faffbd..dac8bab1 100644 --- a/internal/channels/events.go +++ b/internal/channels/events.go @@ -346,32 +346,6 @@ func extractPayloadString(payload any, key string) string { return "" } -// routingMetaKeys enumerates the metadata keys that must survive the hop from -// inbound RunContext.Metadata into outbound OutboundMessage.Metadata so that -// replies, block replies, retries, and placeholder updates all land in the -// correct thread / topic / subgroup routing bucket on each channel. -var routingMetaKeys = []string{ - "message_thread_id", // telegram forum topics - "local_key", // composite chat-id suffix - "group_id", // legacy group identifier - "feishu_reply_target_id", // feishu/lark thread reply routing - "fb_mode", // facebook messenger vs comment routing - "sender_id", // facebook sender for first-inbox - "page_id", // facebook page routing - "reply_to_comment_id", // facebook comment reply target -} - -// copyRoutingMeta copies channel routing metadata from RunContext.Metadata -// into a new map suitable for outbound messages. -func copyRoutingMeta(src map[string]string) map[string]string { - out := make(map[string]string) - for _, k := range routingMetaKeys { - if v := src[k]; v != "" { - out[k] = v - } - } - return out -} // toolStatusMap maps builtin tool names to user-friendly status messages. var toolStatusMap = map[string]string{ diff --git a/internal/channels/pancake/api_client.go b/internal/channels/pancake/api_client.go index 77f369ba..287db116 100644 --- a/internal/channels/pancake/api_client.go +++ b/internal/channels/pancake/api_client.go @@ -164,10 +164,12 @@ func (c *APIClient) UploadMedia(ctx context.Context, filename string, data io.Re } // ReplyComment sends a reply to a comment on a post (action: "reply_comment"). -func (c *APIClient) ReplyComment(ctx context.Context, conversationID, content string) error { +// messageID is the ID of the comment being replied to — required by Pancake API. +func (c *APIClient) ReplyComment(ctx context.Context, conversationID, messageID, content string) error { body, _ := json.Marshal(SendMessageRequest{ - Action: "reply_comment", - Message: content, + Action: "reply_comment", + Message: content, + MessageID: messageID, }) url := fmt.Sprintf("%s/pages/%s/conversations/%s/messages", c.pageV1BaseURL, c.pageID, conversationID) if err := c.doRequest(ctx, http.MethodPost, url, bytes.NewReader(body)); err != nil { diff --git a/internal/channels/pancake/api_client_test.go b/internal/channels/pancake/api_client_test.go index a2b62673..926c5173 100644 --- a/internal/channels/pancake/api_client_test.go +++ b/internal/channels/pancake/api_client_test.go @@ -17,7 +17,7 @@ func TestReplyComment_MatchesOfficialContract(t *testing.T) { client := NewAPIClient("user-token", "page-token", "page-123") client.httpClient = &http.Client{Transport: transport} - if err := client.ReplyComment(context.Background(), "conv-123", "thank you"); err != nil { + if err := client.ReplyComment(context.Background(), "conv-123", "msg-456", "thank you"); err != nil { t.Fatalf("ReplyComment returned error: %v", err) } @@ -42,6 +42,9 @@ func TestReplyComment_MatchesOfficialContract(t *testing.T) { if got, want := payload["message"], "thank you"; got != want { t.Fatalf("payload.message = %#v, want %#v", got, want) } + if got, want := payload["message_id"], "msg-456"; got != want { + t.Fatalf("payload.message_id = %#v, want %#v", got, want) + } } func TestReplyComment_ReturnsError(t *testing.T) { @@ -55,7 +58,7 @@ func TestReplyComment_ReturnsError(t *testing.T) { client := NewAPIClient("user-token", "page-token", "page-123") client.httpClient = &http.Client{Transport: transport} - if err := client.ReplyComment(context.Background(), "conv-123", "thank you"); err == nil { + if err := client.ReplyComment(context.Background(), "conv-123", "msg-456", "thank you"); err == nil { t.Fatal("expected ReplyComment to return error on HTTP 400") } } diff --git a/internal/channels/pancake/comment_handler.go b/internal/channels/pancake/comment_handler.go index fd851a88..68858348 100644 --- a/internal/channels/pancake/comment_handler.go +++ b/internal/channels/pancake/comment_handler.go @@ -13,6 +13,12 @@ import ( func (ch *Channel) handleCommentEvent(data MessagingData) { // Feature gate. if !ch.config.Features.CommentReply { + ch.commentReplyDisabledOnce.Do(func() { + slog.Info("pancake: comment ignored because comment_reply disabled", + "page_id", ch.pageID, + "channel_name", ch.Name(), + "hint", "enable config.features.comment_reply to auto-reply page comments") + }) return } diff --git a/internal/channels/pancake/comment_handler_test.go b/internal/channels/pancake/comment_handler_test.go index 8c2c9c9b..18b3f0c6 100644 --- a/internal/channels/pancake/comment_handler_test.go +++ b/internal/channels/pancake/comment_handler_test.go @@ -1,8 +1,10 @@ package pancake import ( + "bytes" "context" "encoding/json" + "log/slog" "net/http" "net/http/httptest" "strings" @@ -12,6 +14,15 @@ import ( "github.com/nextlevelbuilder/goclaw/internal/bus" ) +func capturePancakeSlog(t *testing.T) (*bytes.Buffer, func()) { + t.Helper() + var buf bytes.Buffer + handler := slog.NewTextHandler(&buf, &slog.HandlerOptions{Level: slog.LevelDebug}) + prev := slog.Default() + slog.SetDefault(slog.New(handler)) + return &buf, func() { slog.SetDefault(prev) } +} + // newTestChannel builds a minimal Channel for handler tests. // apiSrv may be nil when API calls are not expected. func newTestChannel(t *testing.T, pageID string, cfg pancakeInstanceConfig) (*Channel, *bus.MessageBus) { @@ -82,6 +93,29 @@ func TestHandleCommentEvent_FeatureGated(t *testing.T) { } } +func TestHandleCommentEvent_FeatureDisabledLogsDiagnostic(t *testing.T) { + cfg := pancakeInstanceConfig{} + ch, msgBus := newTestChannel(t, "page-1", cfg) + buf, restore := capturePancakeSlog(t) + defer restore() + + ch.handleCommentEvent(commentEvent("page-1", "conv-1", "user-1", "msg-1", "hello")) + ch.handleCommentEvent(commentEvent("page-1", "conv-2", "user-2", "msg-2", "hello again")) + + out := buf.String() + if count := strings.Count(out, "comment_reply disabled"); count != 1 { + t.Fatalf("expected exactly one diagnostic log for disabled comment reply, got %d logs:\n%s", count, out) + } + if !strings.Contains(out, "page-1") { + t.Fatalf("expected diagnostic log to include page_id, got:\n%s", out) + } + + _, ok := consumeInbound(t, msgBus, 50*time.Millisecond) + if ok { + t.Error("expected no message published when CommentReply is disabled") + } +} + func TestHandleCommentEvent_FeatureEnabled(t *testing.T) { cfg := pancakeInstanceConfig{} cfg.Features.CommentReply = true diff --git a/internal/channels/pancake/pancake.go b/internal/channels/pancake/pancake.go index f7bdb4e1..f623baa2 100644 --- a/internal/channels/pancake/pancake.go +++ b/internal/channels/pancake/pancake.go @@ -34,6 +34,7 @@ type Channel struct { config pancakeInstanceConfig apiClient *APIClient pageID string + webhookPageID string // native platform ID used in webhook event.page_id (may differ from Pancake internal pageID) pageName string // resolved from Pancake page metadata at Start() platform string // resolved from Pancake page metadata at Start() webhookSecret string // optional HMAC-SHA256 secret for webhook verification @@ -51,6 +52,10 @@ type Channel struct { // postFetcher fetches and caches page post content for comment context enrichment. postFetcher *PostFetcher + // commentReplyDisabledOnce prevents repeated info logs when COMMENT webhooks + // arrive but the feature is disabled in channel config. + commentReplyDisabledOnce sync.Once + stopCh chan struct{} stopCtx context.Context stopFn context.CancelFunc @@ -79,6 +84,7 @@ func New(cfg pancakeInstanceConfig, creds pancakeCreds, config: cfg, apiClient: apiClient, pageID: cfg.PageID, + webhookPageID: cfg.WebhookPageID, platform: cfg.Platform, webhookSecret: creds.WebhookSecret, postFetcher: NewPostFetcher(apiClient, cfg.PostContextCacheTTL), @@ -131,6 +137,8 @@ func (ch *Channel) Start(ctx context.Context) error { slog.Warn("pancake: could not resolve platform from page metadata", "page_id", ch.pageID, "err", err) } else { if page.Platform != "" { + slog.Debug("pancake: platform auto-detected; set platform explicitly in config to avoid startup API call", + "page_id", ch.pageID, "platform", page.Platform) ch.platform = page.Platform } if page.Name != "" { @@ -161,7 +169,7 @@ func (ch *Channel) Start(ctx context.Context) error { // Stop gracefully shuts down the channel. func (ch *Channel) Stop(_ context.Context) error { - globalRouter.unregister(ch.pageID) + globalRouter.unregister(ch.pageID, ch.webhookPageID) ch.stopFn() close(ch.stopCh) ch.SetRunning(false) @@ -254,8 +262,12 @@ func (ch *Channel) sendCommentReply(ctx context.Context, msg bus.OutboundMessage ch.rememberOutboundEcho(conversationID, part) } + commentID := msg.Metadata["reply_to_comment_id"] + if commentID == "" { + return fmt.Errorf("pancake: reply_to_comment_id missing in outbound metadata for comment reply") + } for _, part := range parts { - if err := ch.apiClient.ReplyComment(ctx, conversationID, part); err != nil { + if err := ch.apiClient.ReplyComment(ctx, conversationID, commentID, part); err != nil { ch.handleAPIError(err) ch.forgetOutboundEcho(conversationID, part) return err diff --git a/internal/channels/pancake/pancake_test.go b/internal/channels/pancake/pancake_test.go index e7f49bfb..7975267b 100644 --- a/internal/channels/pancake/pancake_test.go +++ b/internal/channels/pancake/pancake_test.go @@ -536,6 +536,56 @@ func TestWebhookRouterRoutesCommentEvent(t *testing.T) { } } +// TestWebhookRouterRoutesWebhookPageID covers the production Facebook COMMENT case: +// Pancake sends event.page_id = Facebook native page ID (780222461832476), +// but the channel is configured with Pancake's internal page ID (1098014820065543). +// Fix: configure webhook_page_id = "fb-native-id" so the router registers under both. +func TestWebhookRouterRoutesWebhookPageID(t *testing.T) { + cfg := pancakeInstanceConfig{} + cfg.Features.CommentReply = true + cfg.WebhookPageID = "fb-native-id" // Facebook native page ID sent in webhook event.page_id + + msgBus := bus.New() + cfg.PageID = "pancake-internal-id" + creds := pancakeCreds{APIKey: "k", PageAccessToken: "t"} + ch, err := New(cfg, creds, msgBus, nil) + if err != nil { + t.Fatalf("New: %v", err) + } + ch.platform = "facebook" + + // Simulate register — both IDs should be in the router. + router := &webhookRouter{instances: make(map[string]*Channel)} + router.register(ch) + + if router.instances["pancake-internal-id"] == nil { + t.Error("router should have channel registered under Pancake internal page ID") + } + if router.instances["fb-native-id"] == nil { + t.Error("router should have channel registered under Facebook native page ID (webhook_page_id)") + } + + // Webhook arrives with Facebook native page ID — must route to the channel. + body := buildWebhookBody("fb-native-id", "conv-1", "COMMENT", "user-1", "msg-1", "hello", "") + req := httptest.NewRequest(http.MethodPost, webhookPath, strings.NewReader(body)) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d", w.Code) + } + + ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + defer cancel() + msg, ok := msgBus.ConsumeInbound(ctx) + if !ok { + t.Fatal("expected message published: webhook_page_id should route COMMENT to correct channel") + } + if msg.Metadata["pancake_mode"] != "comment" { + t.Errorf("pancake_mode = %q, want comment", msg.Metadata["pancake_mode"]) + } +} + func TestWebhookRouterRoutesInboxEvent(t *testing.T) { cfg := pancakeInstanceConfig{} cfg.Features.InboxReply = true @@ -675,6 +725,26 @@ func TestSend_CommentMode(t *testing.T) { } } +func TestSend_CommentMode_MissingCommentID_ReturnsError(t *testing.T) { + cfg := pancakeInstanceConfig{} + ch, _ := newChannelWithMultiCapture(t, cfg) + + err := ch.Send(context.Background(), bus.OutboundMessage{ + ChatID: "conv-123", + Content: "reply text", + Metadata: map[string]string{ + "pancake_mode": "comment", + // reply_to_comment_id intentionally absent + }, + }) + if err == nil { + t.Fatal("expected error when reply_to_comment_id is missing, got nil") + } + if !strings.Contains(err.Error(), "reply_to_comment_id") { + t.Errorf("error message should mention reply_to_comment_id, got: %v", err) + } +} + func TestSend_CommentMode_WithFirstInbox(t *testing.T) { cfg := pancakeInstanceConfig{} cfg.Features.FirstInbox = true @@ -685,8 +755,9 @@ func TestSend_CommentMode_WithFirstInbox(t *testing.T) { ChatID: "conv-123", Content: "reply text", Metadata: map[string]string{ - "pancake_mode": "comment", - "sender_id": "user-1", + "pancake_mode": "comment", + "sender_id": "user-1", + "reply_to_comment_id": "msg-1", }, }) if err != nil { @@ -722,12 +793,14 @@ func TestSend_CommentMode_FirstInboxDedup(t *testing.T) { ChatID: "conv-123", Content: "hi", Metadata: map[string]string{ - "pancake_mode": "comment", - "sender_id": "user-1", + "pancake_mode": "comment", + "sender_id": "user-1", + "reply_to_comment_id": "msg-1", }, } ch.Send(context.Background(), outMsg) //nolint:errcheck outMsg.ChatID = "conv-456" // second comment, different conv, same sender + outMsg.Metadata["reply_to_comment_id"] = "msg-2" ch.Send(context.Background(), outMsg) //nolint:errcheck transport.mu.Lock() @@ -764,8 +837,9 @@ func TestSend_CommentMode_FirstInboxDisabled(t *testing.T) { ChatID: "conv-123", Content: "reply", Metadata: map[string]string{ - "pancake_mode": "comment", - "sender_id": "user-1", + "pancake_mode": "comment", + "sender_id": "user-1", + "reply_to_comment_id": "msg-1", }, }) @@ -813,8 +887,9 @@ func TestSend_CommentMode_EchoRemembered(t *testing.T) { ChatID: "conv-echo", Content: "some reply", Metadata: map[string]string{ - "pancake_mode": "comment", - "sender_id": "user-1", + "pancake_mode": "comment", + "sender_id": "user-1", + "reply_to_comment_id": "msg-1", }, }) @@ -899,6 +974,34 @@ func TestSendFirstInbox_ErrorRetryAllowed(t *testing.T) { } } +// TestFactoryExplicitPlatformPreserved verifies that explicit platform from config +// is loaded into the channel and is not overwritten. +func TestFactoryExplicitPlatformPreserved(t *testing.T) { + cfg := json.RawMessage(`{ + "page_id": "123", + "platform": "instagram", + "features": {"inbox_reply": true} + }`) + creds := json.RawMessage(`{ + "api_key": "test_key", + "page_access_token": "test_token" + }`) + ch, err := Factory("test", creds, cfg, nil, nil) + if err != nil { + t.Fatalf("Factory failed: %v", err) + } + pc := ch.(*Channel) + if pc.platform != "instagram" { + t.Errorf("expected platform instagram from config, got %q", pc.platform) + } + // Verify auto-detect block condition: ch.platform is already set, + // so getPage would NOT be called at Start(). platform should remain "instagram". + // (Start() skips GetPage when ch.platform != "") + if pc.platform == "" { + t.Error("platform must not be empty after Factory with explicit platform config") + } +} + // TestCommentFlowEndToEnd is the Phase 5 integration scenario wired inline. func TestCommentFlowEndToEnd(t *testing.T) { cfg := pancakeInstanceConfig{} diff --git a/internal/channels/pancake/types.go b/internal/channels/pancake/types.go index 86dc72cc..34683c20 100644 --- a/internal/channels/pancake/types.go +++ b/internal/channels/pancake/types.go @@ -14,8 +14,11 @@ type pancakeCreds struct { // pancakeInstanceConfig holds non-secret config from channel_instances.config JSONB. type pancakeInstanceConfig struct { - PageID string `json:"page_id"` - Platform string `json:"platform,omitempty"` // auto-detected at Start(): facebook/zalo/instagram/tiktok/whatsapp/line + PageID string `json:"page_id"` + WebhookPageID string `json:"webhook_page_id,omitempty"` // native platform page ID sent in webhooks (e.g. Facebook page ID vs Pancake internal ID) + Platform string `json:"platform,omitempty"` // set explicitly via UI; auto-detected at Start() as fallback for existing channels + // Known values: facebook/instagram/threads/tiktok/youtube/shopee/line/google/chat_plugin/lazada/tokopedia + // Excluded (have native channel implementations): telegram/zalo/whatsapp Features struct { InboxReply bool `json:"inbox_reply"` CommentReply bool `json:"comment_reply"` @@ -92,7 +95,7 @@ type MessagingData struct { ConversationID string PostID string // present for COMMENT events; empty for INBOX Type string // "INBOX" or "COMMENT" - Platform string // "facebook", "zalo", "instagram", "tiktok", "whatsapp", "line" + Platform string // platform identifier from Pancake: facebook/instagram/tiktok/line/etc. See pancakeInstanceConfig.Platform for full list. AssigneeIDs []string Message MessagingMessage } @@ -120,6 +123,7 @@ type PageInfo struct { type SendMessageRequest struct { Action string `json:"action"` Message string `json:"message,omitempty"` + MessageID string `json:"message_id,omitempty"` // required for reply_comment: ID of the comment being replied to ContentIDs []string `json:"content_ids,omitempty"` } diff --git a/internal/channels/pancake/webhook_handler.go b/internal/channels/pancake/webhook_handler.go index deb4d533..2eab2c11 100644 --- a/internal/channels/pancake/webhook_handler.go +++ b/internal/channels/pancake/webhook_handler.go @@ -52,12 +52,18 @@ func (r *webhookRouter) register(ch *Channel) { r.mu.Lock() defer r.mu.Unlock() r.instances[ch.pageID] = ch + if ch.webhookPageID != "" && ch.webhookPageID != ch.pageID { + r.instances[ch.webhookPageID] = ch + } } -func (r *webhookRouter) unregister(pageID string) { +func (r *webhookRouter) unregister(pageID string, webhookPageID string) { r.mu.Lock() defer r.mu.Unlock() delete(r.instances, pageID) + if webhookPageID != "" && webhookPageID != pageID { + delete(r.instances, webhookPageID) + } } // webhookRoute returns the path+handler on first call; ("", nil) for subsequent calls. @@ -113,19 +119,19 @@ func (r *webhookRouter) ServeHTTP(w http.ResponseWriter, req *http.Request) { return } - // Resolve page_id: try top-level first, then data-level, then extract from conversation ID. + // Resolve page_id: top-level field takes priority, then data-level, then first conv ID segment. pageID := event.PageID if pageID == "" { pageID = data.PageID } if pageID == "" { - // Conversation ID format: "pageID_senderID" — extract page portion. + // Last resort: extract from conversation ID (format: pageID_senderID for INBOX events). if idx := strings.Index(data.Conversation.ID, "_"); idx > 0 { pageID = data.Conversation.ID[:idx] } } - // Resolve conversation type — only process INBOX messages. + // Resolve conversation type. convType := strings.ToUpper(data.Conversation.Type) if event.EventType != "" && !strings.EqualFold(event.EventType, "messaging") { @@ -136,17 +142,21 @@ func (r *webhookRouter) ServeHTTP(w http.ResponseWriter, req *http.Request) { return } - slog.Debug("pancake: webhook event parsed", - "event_type", event.EventType, - "page_id", pageID, + // INFO-level: always visible without DEBUG — shows which field sourced the page_id. + slog.Info("pancake: webhook page_id resolution", + "event_page_id", event.PageID, + "data_page_id", data.PageID, "conv_id", data.Conversation.ID, + "resolved_page_id", pageID, "conv_type", convType, "sender_id", data.Conversation.From.ID, - "sender_name", data.Conversation.From.Name, "msg_id", data.Message.ID) if pageID == "" { - slog.Warn("pancake: could not determine page_id from webhook payload") + slog.Warn("pancake: could not determine page_id from webhook payload", + "event_page_id", event.PageID, + "data_page_id", data.PageID, + "conv_id", data.Conversation.ID) w.WriteHeader(http.StatusOK) return } @@ -155,7 +165,6 @@ func (r *webhookRouter) ServeHTTP(w http.ResponseWriter, req *http.Request) { r.mu.RUnlock() if target == nil { - // Log all registered page IDs for debugging. r.mu.RLock() var registered []string for pid := range r.instances { @@ -164,6 +173,9 @@ func (r *webhookRouter) ServeHTTP(w http.ResponseWriter, req *http.Request) { r.mu.RUnlock() slog.Warn("pancake: no channel instance for page_id", "page_id", pageID, + "event_page_id", event.PageID, + "data_page_id", data.PageID, + "conv_id", data.Conversation.ID, "registered_pages", registered) w.WriteHeader(http.StatusOK) return diff --git a/internal/channels/routing_metadata.go b/internal/channels/routing_metadata.go new file mode 100644 index 00000000..7de8b97f --- /dev/null +++ b/internal/channels/routing_metadata.go @@ -0,0 +1,43 @@ +package channels + +// routingMetaKeys enumerates the metadata keys that must survive the hop from +// inbound RunContext.Metadata into outbound OutboundMessage.Metadata so that +// replies, block replies, retries, and placeholder updates all land in the +// correct thread / topic / subgroup routing bucket on each channel. +var routingMetaKeys = []string{ + "message_thread_id", // telegram forum topics + "local_key", // composite chat-id suffix + "group_id", // legacy group identifier + "feishu_reply_target_id", // feishu/lark thread reply routing + "fb_mode", // facebook messenger vs comment routing + "sender_id", // facebook sender for first-inbox / pancake sender for first-inbox + "page_id", // facebook page routing + "reply_to_comment_id", // facebook/pancake comment reply target + "pancake_mode", // pancake inbox vs comment routing +} + +var finalReplyMetaKeys = append([]string{ + "placeholder_key", // final outbound can update placeholder; block replies must not +}, routingMetaKeys...) + +func copySelectedMeta(src map[string]string, keys []string) map[string]string { + out := make(map[string]string) + for _, k := range keys { + if v := src[k]; v != "" { + out[k] = v + } + } + return out +} + +// CopyFinalRoutingMeta copies the routing metadata required for final outbound +// delivery after an inbound message has been processed by the agent loop. +func CopyFinalRoutingMeta(src map[string]string) map[string]string { + return copySelectedMeta(src, finalReplyMetaKeys) +} + +// copyRoutingMeta copies only the subset safe for intermediate block replies, +// retries, and placeholder updates. +func copyRoutingMeta(src map[string]string) map[string]string { + return copySelectedMeta(src, routingMetaKeys) +} diff --git a/internal/channels/routing_metadata_test.go b/internal/channels/routing_metadata_test.go new file mode 100644 index 00000000..62d52654 --- /dev/null +++ b/internal/channels/routing_metadata_test.go @@ -0,0 +1,39 @@ +package channels + +import "testing" + +func TestCopyRoutingMeta_PreservesPancakeCommentRouting(t *testing.T) { + src := map[string]string{ + "pancake_mode": "comment", + "reply_to_comment_id": "msg-123", + "sender_id": "user-123", + } + + got := copyRoutingMeta(src) + + for key, want := range map[string]string{ + "pancake_mode": "comment", + "reply_to_comment_id": "msg-123", + "sender_id": "user-123", + } { + if got[key] != want { + t.Fatalf("copyRoutingMeta()[%q] = %q, want %q", key, got[key], want) + } + } +} + +func TestCopyFinalRoutingMeta_PreservesPlaceholderAndPancakeMode(t *testing.T) { + src := map[string]string{ + "placeholder_key": "placeholder-123", + "pancake_mode": "comment", + } + + got := CopyFinalRoutingMeta(src) + + if got["placeholder_key"] != "placeholder-123" { + t.Fatalf("CopyFinalRoutingMeta()[%q] = %q, want %q", "placeholder_key", got["placeholder_key"], "placeholder-123") + } + if got["pancake_mode"] != "comment" { + t.Fatalf("CopyFinalRoutingMeta()[%q] = %q, want %q", "pancake_mode", got["pancake_mode"], "comment") + } +} diff --git a/ui/web/src/i18n/locales/en/channels.json b/ui/web/src/i18n/locales/en/channels.json index f9cca4b7..19502329 100644 --- a/ui/web/src/i18n/locales/en/channels.json +++ b/ui/web/src/i18n/locales/en/channels.json @@ -316,7 +316,8 @@ "label": "Tool Allowlist", "help": "Restrict which tools the agent can use in this group" }, - "system_prompt": { "label": "System Prompt" } + "system_prompt": { "label": "System Prompt" }, + "platform": { "label": "Platform", "help": "Select the platform this Pancake page serves." } }, "fieldOptions": { "block_reply": { @@ -361,6 +362,19 @@ "auto": "Auto", "raw": "Raw", "card": "Card" + }, + "platform": { + "facebook": "Facebook", + "instagram": "Instagram", + "threads": "Threads (Beta)", + "tiktok": "TikTok", + "youtube": "YouTube (Beta)", + "shopee": "Shopee", + "line": "Line", + "google": "Google", + "chat_plugin": "Chat Plugin", + "lazada": "Lazada", + "tokopedia": "Tokopedia" } }, "groupOverrides": { diff --git a/ui/web/src/i18n/locales/vi/channels.json b/ui/web/src/i18n/locales/vi/channels.json index b545d811..710b5b2b 100644 --- a/ui/web/src/i18n/locales/vi/channels.json +++ b/ui/web/src/i18n/locales/vi/channels.json @@ -243,7 +243,8 @@ "thread_ttl": { "label": "TTL tham gia luồng (giờ)", "help": "Số giờ trước khi bot ngừng tự động trả lời trong các luồng đã tham gia. 0 = luôn yêu cầu @đề cập." }, "skills": { "label": "Bộ lọc skill", "help": "Giới hạn skill khả dụng cho nhóm này" }, "tools": { "label": "Danh sách công cụ được phép", "help": "Giới hạn công cụ agent có thể dùng trong nhóm này" }, - "system_prompt": { "label": "Prompt hệ thống" } + "system_prompt": { "label": "Prompt hệ thống" }, + "platform": { "label": "Nền tảng", "help": "Chọn nền tảng mà trang Pancake này phục vụ." } }, "fieldOptions": { "block_reply": { @@ -288,6 +289,19 @@ "auto": "Tự động", "raw": "Thô", "card": "Thẻ" + }, + "platform": { + "facebook": "Facebook", + "instagram": "Instagram", + "threads": "Threads (Beta)", + "tiktok": "TikTok", + "youtube": "YouTube (Beta)", + "shopee": "Shopee", + "line": "Line", + "google": "Google", + "chat_plugin": "Chat Plugin (Website)", + "lazada": "Lazada", + "tokopedia": "Tokopedia" } }, "groupOverrides": { diff --git a/ui/web/src/i18n/locales/zh/channels.json b/ui/web/src/i18n/locales/zh/channels.json index 7742cb31..3315484c 100644 --- a/ui/web/src/i18n/locales/zh/channels.json +++ b/ui/web/src/i18n/locales/zh/channels.json @@ -243,7 +243,8 @@ "thread_ttl": { "label": "线程参与 TTL（小时）", "help": "Bot 停止自动回复已参与线程的小时数。0 = 始终需要 @提及。" }, "skills": { "label": "Skill过滤", "help": "限制此群组可用的Skill" }, "tools": { "label": "工具白名单", "help": "限制Agent在此群组中可使用的工具" }, - "system_prompt": { "label": "系统提示词" } + "system_prompt": { "label": "系统提示词" }, + "platform": { "label": "平台", "help": "选择此 Pancake 页面所服务的平台。" } }, "fieldOptions": { "block_reply": { @@ -288,6 +289,19 @@ "auto": "自动", "raw": "原始", "card": "卡片" + }, + "platform": { + "facebook": "Facebook", + "instagram": "Instagram", + "threads": "Threads (测试版)", + "tiktok": "TikTok", + "youtube": "YouTube (测试版)", + "shopee": "Shopee", + "line": "Line", + "google": "Google", + "chat_plugin": "Chat Plugin", + "lazada": "Lazada", + "tokopedia": "Tokopedia" } }, "groupOverrides": { diff --git a/ui/web/src/lib/config-flatten.ts b/ui/web/src/lib/config-flatten.ts new file mode 100644 index 00000000..8789f3e6 --- /dev/null +++ b/ui/web/src/lib/config-flatten.ts @@ -0,0 +1,70 @@ +/** + * Flatten/unflatten utilities for channel config objects. + * + * Channel config schemas use dot-notation keys (e.g. "features.comment_reply") + * while the database stores nested JSON ({"features": {"comment_reply": true}}). + * These helpers bridge the two representations. + */ + +/** + * Flatten a nested object to dot-notation keys. + * Arrays are NOT flattened — they are kept as-is. + * + * @example + * flattenConfig({ features: { comment_reply: true }, page_id: "123" }) + * // → { "features.comment_reply": true, "page_id": "123" } + */ +export function flattenConfig( + obj: Record, + prefix = "", +): Record { + const result: Record = {}; + for (const key of Object.keys(obj)) { + const fullKey = prefix ? `${prefix}.${key}` : key; + const value = obj[key]; + if ( + value !== null && + typeof value === "object" && + !Array.isArray(value) + ) { + Object.assign(result, flattenConfig(value as Record, fullKey)); + } else { + result[fullKey] = value; + } + } + return result; +} + +/** + * Unflatten dot-notation keys back to a nested object. + * Non-dot keys are placed at the root level unchanged. + * + * @example + * unflattenConfig({ "features.comment_reply": true, "page_id": "123" }) + * // → { features: { comment_reply: true }, page_id: "123" } + */ +export function unflattenConfig( + flat: Record, +): Record { + const result: Record = {}; + for (const [key, value] of Object.entries(flat)) { + const parts = key.split("."); + let current = result; + for (let i = 0; i < parts.length - 1; i++) { + const part = parts[i]!; + const existing = current[part]; + if ( + existing === undefined || + existing === null || + typeof existing !== "object" || + Array.isArray(existing) + ) { + current[part] = {}; + } + current = current[part] as Record; + } + const last = parts[parts.length - 1]!; + current[last] = value; + } + return result; +} diff --git a/ui/web/src/pages/channels/channel-fields.tsx b/ui/web/src/pages/channels/channel-fields.tsx index 124d5283..1598d3ee 100644 --- a/ui/web/src/pages/channels/channel-fields.tsx +++ b/ui/web/src/pages/channels/channel-fields.tsx @@ -35,7 +35,11 @@ export function ChannelFields({ fields, values, onChange, idPrefix, isEdit, cont // Conditional visibility: skip field if showWhen condition is not met if (field.showWhen) { const depValue = allValues[field.showWhen.key] ?? fields.find((f) => f.key === field.showWhen!.key)?.defaultValue; - if (String(depValue) !== field.showWhen.value) return null; + const depStr = depValue !== undefined && depValue !== null ? String(depValue) : ""; + const match = Array.isArray(field.showWhen.value) + ? field.showWhen.value.includes(depStr) + : depStr === field.showWhen.value; + if (!match) return null; } // Check disabledWhen condition let disabled = false; diff --git a/ui/web/src/pages/channels/channel-instance-form-dialog.tsx b/ui/web/src/pages/channels/channel-instance-form-dialog.tsx index 9f53e4b8..26aaf1fe 100644 --- a/ui/web/src/pages/channels/channel-instance-form-dialog.tsx +++ b/ui/web/src/pages/channels/channel-instance-form-dialog.tsx @@ -17,6 +17,7 @@ import { wizardAuthSteps, wizardConfigSteps } from "./channel-wizard-registry"; import { CHANNEL_TYPES } from "@/constants/channels"; import { channelInstanceSchema, type ChannelInstanceFormData } from "@/schemas/channel.schema"; import { ChannelInstanceFormStep } from "./channel-instance-form-step"; +import { flattenConfig, unflattenConfig } from "@/lib/config-flatten"; type WizardStep = "form" | "auth" | "config"; @@ -91,7 +92,7 @@ export function ChannelInstanceFormDialog({ for (const f of schema) { if (f.defaultValue !== undefined) defaults[f.key] = f.defaultValue; } - const merged: Record = { ...defaults, ...(instance?.config ?? {}) }; + const merged: Record = { ...defaults, ...flattenConfig((instance?.config ?? {}) as Record) }; const boolSelectKeys = new Set( schema.filter((f: FieldDef) => f.type === "select" && f.options?.some((o) => o.value === "true")).map((f: FieldDef) => f.key), ); @@ -151,6 +152,19 @@ export function ChannelInstanceFormDialog({ Object.entries(configValues).filter(([, v]) => v !== undefined && v !== "" && v !== null), ); coerceBoolSelects(cleanConfig, configSchema[values.channelType] ?? []); + + // Config required check (create-only): validate after cleanConfig is built so empty strings are caught. + if (!instance) { + const cfgSchema = configSchema[values.channelType] ?? []; + const missingCfg = cfgSchema.filter( + (f: FieldDef) => f.required && (cleanConfig[f.key] === undefined || cleanConfig[f.key] === "" || cleanConfig[f.key] === null), + ); + if (missingCfg.length > 0) { + setError(t("form.errors.requiredFields", { fields: missingCfg.map((f: FieldDef) => f.label).join(", ") })); + return; + } + } + const cleanCreds = Object.fromEntries( Object.entries(credsValues).filter(([, v]) => v !== undefined && v !== "" && v !== null), ); @@ -163,7 +177,7 @@ export function ChannelInstanceFormDialog({ display_name: values.displayName?.trim() || undefined, channel_type: values.channelType, agent_id: values.agentId, - config: Object.keys(cleanConfig).length > 0 ? cleanConfig : undefined, + config: Object.keys(cleanConfig).length > 0 ? unflattenConfig(cleanConfig) : undefined, enabled: values.enabled, }; if (Object.keys(cleanCreds).length > 0) data.credentials = cleanCreds; @@ -198,7 +212,7 @@ export function ChannelInstanceFormDialog({ setLoading(true); setError(""); try { - await onUpdate(createdInstanceId, { config: cleanConfig }); + await onUpdate(createdInstanceId, { config: unflattenConfig(cleanConfig) }); onOpenChange(false); } catch (err: unknown) { setError(err instanceof Error ? err.message : t("form.errors.failedSaveConfig")); diff --git a/ui/web/src/pages/channels/channel-instance-form-step.tsx b/ui/web/src/pages/channels/channel-instance-form-step.tsx index 8adebc12..d749fedb 100644 --- a/ui/web/src/pages/channels/channel-instance-form-step.tsx +++ b/ui/web/src/pages/channels/channel-instance-form-step.tsx @@ -1,4 +1,4 @@ -import { useCallback } from "react"; +import { useCallback, useState } from "react"; import { Controller } from "react-hook-form"; import type { UseFormReturn } from "react-hook-form"; import { useTranslation } from "react-i18next"; @@ -13,6 +13,7 @@ import { SelectValue, } from "@/components/ui/select"; import { Button } from "@/components/ui/button"; +import { ChevronDown, ChevronRight } from "lucide-react"; import type { ChannelInstanceData } from "./hooks/use-channel-instances"; import type { AgentData } from "@/types/agent"; import { slugify } from "@/lib/slug"; @@ -56,6 +57,11 @@ export function ChannelInstanceFormStep({ const cfgFields = configSchema[channelType] ?? []; const formCfgFields = excludeSet.size > 0 ? cfgFields.filter((f: FieldDef) => !excludeSet.has(f.key)) : cfgFields; const hasWizard = !instance && !!wizard; + const normalCfgFields = formCfgFields.filter((f: FieldDef) => !f.advanced); + const advancedCfgFields = formCfgFields.filter((f: FieldDef) => f.advanced); + const [showAdvanced, setShowAdvanced] = useState( + () => advancedCfgFields.some((f) => configValues[f.key] !== undefined && configValues[f.key] !== ""), + ); const handleTelegramGroupsChange = useCallback((groups: Record) => { setConfigValues((prev) => ({ @@ -159,8 +165,25 @@ export function ChannelInstanceFormStep({ {formCfgFields.length > 0 && (
{t("form.configuration")} - + {instance && EditConfig && } + {advancedCfgFields.length > 0 && ( +
+ + {showAdvanced && ( +
+ +
+ )} +
+ )}
)} diff --git a/ui/web/src/pages/channels/channel-schemas.test.ts b/ui/web/src/pages/channels/channel-schemas.test.ts new file mode 100644 index 00000000..6bf564e7 --- /dev/null +++ b/ui/web/src/pages/channels/channel-schemas.test.ts @@ -0,0 +1,43 @@ +import { describe, it, expect } from "vitest"; +import { configSchema } from "./channel-schemas"; + +describe("pancake configSchema", () => { + const pancakeConfig = configSchema["pancake"]!; + + it("has a platform field", () => { + expect(pancakeConfig).toBeDefined(); + const platformField = pancakeConfig.find((f) => f.key === "platform"); + expect(platformField).toBeDefined(); + }); + + it("platform field is type select", () => { + const platformField = pancakeConfig.find((f) => f.key === "platform")!; + expect(platformField.type).toBe("select"); + }); + + it("platform field is required", () => { + const platformField = pancakeConfig.find((f) => f.key === "platform")!; + expect(platformField.required).toBe(true); + }); + + it("platform options include all expected platforms", () => { + const platformField = pancakeConfig.find((f) => f.key === "platform")!; + const values = platformField.options!.map((o) => o.value); + expect(values).toContain("facebook"); + expect(values).toContain("instagram"); + expect(values).toContain("tiktok"); + expect(values).toContain("line"); + expect(values).toContain("shopee"); + expect(values).toContain("lazada"); + expect(values).toContain("tokopedia"); + }); + + it("platform options do NOT include natively-supported channels", () => { + const platformField = pancakeConfig.find((f) => f.key === "platform")!; + const values = platformField.options!.map((o) => o.value); + expect(values).not.toContain("telegram"); + expect(values).not.toContain("zalo"); + expect(values).not.toContain("whatsapp"); + expect(values).not.toContain("zalo_oa"); + }); +}); diff --git a/ui/web/src/pages/channels/channel-schemas.ts b/ui/web/src/pages/channels/channel-schemas.ts index 3dc65d4d..a24654fb 100644 --- a/ui/web/src/pages/channels/channel-schemas.ts +++ b/ui/web/src/pages/channels/channel-schemas.ts @@ -10,10 +10,12 @@ export interface FieldDef { defaultValue?: string | number | boolean | string[]; options?: { value: string; label: string }[]; help?: string; - /** Only show this field when another field has a specific value */ - showWhen?: { key: string; value: string }; + /** Only show this field when another field has a specific value (or one of several values) */ + showWhen?: { key: string; value: string | string[] }; /** Disable this field when another field has a specific value */ disabledWhen?: { key: string; value: string; hint?: string }; + /** Hide in an "Advanced" collapsible section — for rarely-needed fields */ + advanced?: boolean; } // --- Shared option lists --- @@ -81,6 +83,22 @@ export const credentialsSchema: Record = { ], }; +// --- Pancake platform options --- + +const pancakePlatformOptions = [ + { value: "facebook", label: "Facebook" }, + { value: "instagram", label: "Instagram" }, + { value: "threads", label: "Threads (Beta)" }, + { value: "tiktok", label: "TikTok" }, + { value: "youtube", label: "YouTube (Beta)" }, + { value: "shopee", label: "Shopee" }, + { value: "line", label: "Line" }, + { value: "google", label: "Google" }, + { value: "chat_plugin", label: "Chat Plugin" }, + { value: "lazada", label: "Lazada" }, + { value: "tokopedia", label: "Tokopedia" }, +]; + // --- Config schemas --- export const configSchema: Record = { @@ -177,10 +195,12 @@ export const configSchema: Record = { { key: "block_reply", label: "Block Reply", type: "select", options: blockReplyOptions, defaultValue: "inherit" }, ], pancake: [ - { key: "page_id", label: "Page ID", type: "text", required: true, help: "Pancake page ID (numeric, from Pancake dashboard)" }, - { key: "platform", label: "Platform (auto-detected)", type: "text", placeholder: "Leave empty — resolved at startup", help: "facebook / zalo / instagram / tiktok / whatsapp / line. Auto-detected if empty." }, + { key: "page_id", label: "Page ID", type: "text", required: true, help: "Pancake internal page ID (numeric, from Pancake dashboard)" }, + { key: "webhook_page_id", label: "Webhook Page ID", type: "text", help: "Only needed when the native platform page ID in webhooks differs from the Pancake page ID above (rare). Leave empty if both are the same.", advanced: true }, + { key: "platform", label: "Platform", type: "select", required: true, defaultValue: "", options: pancakePlatformOptions, help: "Select the platform this Pancake page serves." }, { key: "features.inbox_reply", label: "Inbox Auto-Reply", type: "boolean", defaultValue: true }, - { key: "features.comment_reply", label: "Comment Reply", type: "boolean", defaultValue: false }, + { key: "features.comment_reply", label: "Comment Reply", type: "boolean", defaultValue: false, + showWhen: { key: "platform", value: ["facebook", "instagram", "threads", "tiktok", "youtube"] } }, { key: "allow_from", label: "Allowed Users", type: "tags", help: "Sender IDs to whitelist. Empty = accept all." }, { key: "block_reply", label: "Block Reply", type: "select", options: blockReplyOptions, defaultValue: "inherit" }, ],