fix(zalo): accept string chat ids for outbound sends by goutamadwant · Pull Request #93689 · openclaw/openclaw

goutamadwant · 2026-06-16T15:51:01Z

Summary

What problem does this PR solve?

Fixes Zalo proactive outbound sends rejecting valid Bot API chat_id string values before delivery.
Replaces the Zalo plugin's numeric-only target check with a Zalo-specific predicate that treats the normalized nonblank chat_id as opaque, matching Zalo's documented String field.
Keeps the change plugin-local; shared outbound target heuristics and other channels are intentionally unchanged.

Why does this matter now?

Zalo channel rejects valid non-numeric chat_id values for proactive outbound sends #57594 is still open and source-reproducible on current main.
The current Zalo resolver rejects valid IDs like 3becaa50ae12474c1e03 and documented string examples like abc.xyz, which blocks proactive notifications even though Zalo Bot API delivery can work with string chat_id values.

What is the intended outcome?

openclaw message send --channel zalo --target <chat_id> resolves any nonblank normalized Zalo target as a direct Zalo chat_id instead of failing with Unknown target because OpenClaw cannot infer the upstream ID grammar.

What is intentionally out of scope?

Live Zalo Bot API delivery changes.
Zalo auth, token, webhook, pairing, group policy, or directory behavior.
Any shared outbound resolver behavior for other channels.

What does success look like?

Bare, zalo:, and zl: Zalo string chat IDs pass target resolution.
Existing numeric and reported hex-like Zalo chat IDs continue to pass.
Blank and prefix-only Zalo targets remain rejected.

What should reviewers focus on?

Whether treating Zalo chat_id as opaque is the right boundary given Zalo documents it as String.
Whether the behavior is acceptably scoped to Zalo only.
Whether the tests cover numeric, reported hex-like, documented dotted, prefixed, and blank target shapes.

Summary guidance

This PR description is the contributor's durable explanation of the change. Write it for human maintainers first; ClawSweeper and Barnacle use the same text to understand intent, proof, risk, and current review state.

Describe the intent and outcome in 2-5 bullets. Avoid restating the diff; reviewers and bots can read the changed files.

If this PR fixes a plugin beta-release blocker, title it fix(<plugin-id>): beta blocker - <summary> and link the matching Beta blocker: <plugin-name> - <summary> issue labeled beta-blocker. Contributors cannot label PRs, so the title is the PR-side signal for maintainers and automation.

Linked context

Which issue does this close?

Closes #57594

Which issues, PRs, or discussions are related?

Related #84599

Was this requested by a maintainer or owner?

The review on #93689 requested avoiding an undocumented regex grammar and adding coverage for Zalo's documented abc.xyz string chat_id example.

Linked context guidance

Link the issue, PR, discussion, maintainer request, or owner request that explains why this PR should exist. Maintainer context helps reviewers and automation distinguish intended work from drive-by churn.

Real behavior proof (required for external PRs)

Behavior or issue addressed: Zalo proactive outbound sends rejected valid Bot API chat_id string values before send target resolution.
Real environment tested: Local OpenClaw checkout on the updated branch, Node 24.14.0, CLI dry-run mode, no live Zalo credentials.
Exact steps or command run after this patch: openclaw message send --channel zalo --target abc.xyz --message test --dry-run --json

Evidence after fix (screenshot, recording, terminal capture, console output, redacted runtime log, linked artifact, or copied live output): copied live output:

{
  "action": "send",
  "channel": "zalo",
  "dryRun": true,
  "handledBy": "core",
  "payload": {
    "channel": "zalo",
    "to": "abc.xyz",
    "via": "direct",
    "mediaUrl": null,
    "dryRun": true
  }
}

Observed result after fix: The documented dotted string Zalo chat_id resolves to a direct outbound target instead of failing as an unknown target.
What was not tested: Live Zalo Bot API delivery with real credentials.
Proof limitations or environment constraints: Dry-run proves OpenClaw's pre-send target resolution path without contacting Zalo. Live delivery still depends on a valid bot token and upstream Zalo API access.
Before evidence (optional but encouraged): On origin/main, extensions/zalo/src/channel.ts wired targetResolver.looksLikeId to isNumericTargetId; that helper only accepts /^\d{3,}$/, excluding reported hex-like IDs and documented dotted string IDs.

Real behavior proof guidance

External contributors must show after-fix evidence from a real OpenClaw setup. Unit tests, mocks, lint, typechecks, snapshots, and CI are supplemental only.

Screenshots are encouraged even for CLI, console, text, or log changes. Terminal screenshots, copied live output, redacted runtime logs, recordings, and linked artifacts count.

If your environment cannot produce the ideal proof, explain that under Proof limitations or environment constraints so reviewers and ClawSweeper can direct the next step properly.

Be mindful of private information like IP addresses, API keys, phone numbers, non-public endpoints, or other private details when providing evidence.

Tests and validation

Which commands did you run?

node scripts/run-vitest.mjs extensions/zalo/src/channel.directory.test.ts src/infra/outbound/target-normalization.test.ts src/infra/outbound/target-resolver.test.ts
node scripts/run-vitest.mjs extensions/zalo/src src/infra/outbound/outbound-session.test.ts
./node_modules/.bin/oxfmt --check --threads=1 extensions/zalo/src/channel.ts extensions/zalo/src/channel.directory.test.ts
./node_modules/.bin/oxlint extensions/zalo/src/channel.ts extensions/zalo/src/channel.directory.test.ts
git diff --check
OPENCLAW_BUILD_ALL_NO_PNPM=1 node scripts/build-all.mjs qaRuntime

What regression coverage was added or updated?

Added Zalo plugin coverage for numeric, reported hex-like, documented dotted, zalo:-prefixed, and zl:-prefixed chat IDs.
Added blank/prefix-only coverage so empty normalized Zalo targets are still rejected.

What failed before this fix, if known?

Current main uses isNumericTargetId for Zalo direct target detection, so reported Zalo chat_id values like 3becaa50ae12474c1e03 and documented string examples like abc.xyz do not qualify as id-like targets.

If no test was added, why not?

Focused regression coverage was added.

Testing guidance

List focused commands, not every incidental check. CI is useful support, but external PRs still need real behavior proof above when behavior changes.

Risk checklist

Did user-visible behavior change? (Yes/No)

Yes

Did config, environment, or migration behavior change? (Yes/No)

No

Did security, auth, secrets, network, or tool execution behavior change? (Yes/No)

No

What is the highest-risk area?

The Zalo target-id heuristic now treats any nonblank normalized Zalo target as a direct chat_id, so ordinary-looking strings can be sent directly instead of being rejected as unknown targets.

How is that risk mitigated?

The behavior is scoped only to Zalo.
Zalo documents chat_id as String and shows a dotted string example, so OpenClaw should not invent a narrower grammar.
Blank and prefix-only inputs still fail normalization.
Tests cover numeric, reported hex-like, documented dotted, prefixed, ordinary nonblank, and blank target shapes.

Risk guidance

Use this for author judgment that is not obvious from the diff. ClawSweeper can see touched files, but it cannot know which behavior you think is risky, why the risk is acceptable, or what mitigation reviewers should verify.

Current review state

What is the next action?

Maintainer re-review.

What is still waiting on author, maintainer, CI, or external proof?

Waiting on GitHub CI and maintainer re-review. Live Zalo delivery proof would require valid Zalo Bot API credentials; the included dry-run proof covers OpenClaw target resolution without sending a live message.

Which bot or reviewer comments were addressed?

Addressed Vincent Koc's review on fix(zalo): accept string chat ids for outbound sends #93689 by removing the undocumented hex-only grammar, treating normalized nonblank Zalo chat_id values as opaque strings, and adding abc.xyz plus prefixed variants to the resolver test.

Review state guidance

Keep this as the durable state for review progress. If useful information appears in comments, fold the current next action or blocker back here so maintainers and ClawSweeper do not need to reconstruct state from comment history.

clawsweeper · 2026-06-16T18:56:29Z

Codex review: needs maintainer review before merge. Reviewed July 1, 2026, 2:13 AM ET / 06:13 UTC.

Summary
This PR replaces Zalo's numeric-only outbound target predicate with a Zalo-local nonblank string chat_id predicate and adds resolver coverage for numeric, hex-like, dotted, prefixed, ordinary, and blank targets.

PR surface: Source +2, Tests +17. Total +19 across 2 files.

Reproducibility: yes. Source inspection shows current main and v2026.6.11 use the numeric-only Zalo looksLikeId helper, and the shared resolver only accepts direct targets when that predicate returns true.

Review metrics: 1 noteworthy metric.

Target Heuristic Changed: 1 channel-specific predicate changed. This predicate decides whether Zalo outbound sends treat user input as a direct chat_id before directory lookup or unknown-target fallback.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #57594
Summary: The open issue is the canonical user report for nonnumeric Zalo chat_id rejection; this PR is its active closing fix candidate, while the older hex-only PR is closed unmerged and superseded.

Members:

canonical: Zalo channel rejects valid non-numeric chat_id values for proactive outbound sends #57594 - It reports the exact Zalo proactive-send rejection and this PR uses closing syntax for that issue.
superseded: fix(zalo): accept hex-like chat_id values in targetResolver #90859 - It attempted a narrower hex-like fix, was closed unmerged, and does not cover the official dotted string example that this PR now covers.

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

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

Ask the prior reviewer or another maintainer to clear or refresh the CHANGES_REQUESTED review after the opaque-string follow-up commit.
[P2] Rerun or assess the failed network-platform CI check before merge.

Risk before merge

[P1] The patch intentionally changes Zalo so any nonblank normalized target, including name-like strings, is treated as a direct chat_id instead of falling through to directory lookup or unknown-target rejection.
[P1] GitHub still reports the earlier CHANGES_REQUESTED review and one failed network-platform check, so maintainers need to refresh review state and rerun or accept that check before merge.
[P1] The supplied proof covers OpenClaw CLI dry-run target resolution, not credentialed live Zalo Bot API delivery; that is enough for the changed pre-send gate unless maintainers want upstream delivery proof.

Maintainer options:

Accept opaque Zalo chat IDs (recommended)
Merge after maintainer re-review if the team accepts that Zalo chat_id values are opaque upstream strings and nonblank normalized Zalo targets should bypass narrower local grammar checks.
Request credentialed Zalo proof
Ask for a redacted live Zalo send artifact if maintainers want upstream delivery confidence beyond OpenClaw dry-run target resolution.
Pause on target semantics
Hold or close the PR if maintainers do not want ordinary nonblank Zalo strings to become direct outbound chat_id targets without directory lookup.

Next step before merge

[P2] The remaining action is maintainer re-review and CI handling; there is no narrow automated code repair to make on the PR branch.

Security
Cleared: The diff only changes a Zalo target predicate and tests; it does not touch dependencies, workflows, secrets, package metadata, or code-execution surfaces.

Review details

Best possible solution:

Land the Zalo-local opaque chat_id predicate after the prior review is cleared and current CI is green, while leaving shared outbound target heuristics unchanged.

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

Yes. Source inspection shows current main and v2026.6.11 use the numeric-only Zalo looksLikeId helper, and the shared resolver only accepts direct targets when that predicate returns true.

Is this the best way to solve the issue?

Yes, with maintainer acceptance of the wider delivery heuristic. A Zalo-local opaque string predicate is the right owner boundary; changing shared target heuristics or encoding a guessed hex grammar would be worse.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 0c7bac34ae68.

Label changes

Label justifications:

P2: The PR fixes a bounded Zalo outbound-send bug that blocks proactive sends for valid nonnumeric chat_id values.
merge-risk: 🚨 message-delivery: The widened Zalo predicate changes whether nonblank targets are sent directly instead of rejected or resolved through directory lookup.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes copied live CLI dry-run JSON from a real OpenClaw setup showing abc.xyz resolves as a direct Zalo target, which exercises the changed pre-send resolver path.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied live CLI dry-run JSON from a real OpenClaw setup showing abc.xyz resolves as a direct Zalo target, which exercises the changed pre-send resolver path.

Evidence reviewed

PR surface:

Source +2, Tests +17. Total +19 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	7	5	+2
Tests	1	17	0	+17
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	24	5	+19

What I checked:

Repository policy read: Read the full root policy and scoped extensions policy; the plugin-boundary guidance applies because the change stays inside the bundled Zalo plugin and uses plugin-sdk surfaces. (AGENTS.md:1, 0c7bac34ae68)
Current main still rejects nonnumeric Zalo IDs: Current main normalizes Zalo prefixes but wires Zalo messaging.targetResolver.looksLikeId to isNumericTargetId, so nonnumeric chat_id strings miss the direct-target path. (extensions/zalo/src/channel.ts:237, 0c7bac34ae68)
Numeric helper contract: isNumericTargetId only accepts trimmed strings matching three or more digits, excluding the reported hex-like IDs and the documented dotted string example. (src/plugin-sdk/reply-payload.ts:389, 0c7bac34ae68)
Outbound resolver gate: resolveMessagingTarget only builds the normalized direct target before directory lookup when looksLikeTargetId passes; otherwise it can fall through to unknownTargetError. (src/infra/outbound/target-resolver.ts:433, 0c7bac34ae68)
Zalo send payload already uses string chat_id: The Zalo send path trims and forwards context.chatId into the Bot API payload as chat_id, so the failure is the pre-send target-resolution gate rather than the send payload shape. (extensions/zalo/src/send.ts:169, 0c7bac34ae68)
PR head implements opaque Zalo chat IDs: The PR head adds looksLikeZaloChatId, accepts any nonblank normalized Zalo target, and wires that predicate into Zalo messaging.targetResolver. (extensions/zalo/src/channel.ts:74, e90bdb95bb69)

Likely related people:

steipete: Git history shows Peter Steinberger introduced the Zalo plugin, centralized message target resolution, and later carried the Zalo chat-plugin builder refactor that preserved the current resolver wiring. (role: feature-history owner; confidence: high; commits: 5abe3c21456e, 331141ad77e3, 6ba9764b0f68; files: extensions/zalo/src/channel.ts, extensions/zalo/src/send.ts, src/infra/outbound/target-resolver.ts)
gumadeiras: Gustavo Madeira Santana moved target-resolution heuristics and Zalo outbound session routing behind plugin-owned seams, which is the boundary this PR uses. (role: target-resolution seam contributor; confidence: medium; commits: e93412b5ce13, 33bcf11c3fdc; files: src/infra/outbound/target-resolver.ts, extensions/zalo/src/session-route.ts, extensions/zalo/src/channel.ts)
vincentkoc: Vincent Koc recently worked on outbound target-normalization seams and authored the live review that requested the opaque Zalo chat_id direction now implemented here. (role: recent area contributor and reviewer; confidence: medium; commits: 08ca24837809, ae3d731810f6, 6eb04c8aab; files: src/infra/outbound/target-normalization.ts, src/infra/outbound/target-resolver.ts, extensions/zalo/src/channel.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 (1 earlier review cycle)

reviewed 2026-06-23T16:03:53.763Z sha e90bdb9 :: needs maintainer review before merge. :: none

vincentkoc · 2026-06-16T19:37:59Z

Clownfish 🐠 reef update

Thanks for the contribution here. Clownfish gave this branch a little current boost so the original PR can stay the canonical swim lane instead of opening a replacement.

Source PR: #93689
Validation: pnpm -s vitest run extensions/zalo/src/channel.directory.test.ts; pnpm check:changed
Contributor credit stays right here with this PR's history and changelog context. no lost treasure, no mystery bubbles.

fish notes: model gpt-5.5, reasoning medium; reviewed against 824baed.

vincentkoc

P2: the new regex still rejects Zalo's documented string chat ID form. The official sendMessage docs define chat_id as String and use abc.xyz in the request example, but zaloChatIdPattern accepts only digits or a 12-plus-character hex string. Please avoid encoding an undocumented grammar here: treat the normalized nonblank Zalo chat ID as opaque, or prove the complete upstream format, and add abc.xyz plus a prefixed variant to the resolver test.

goutamadwant · 2026-06-16T23:58:28Z

Thanks for the review @vincentkoc . I updated the branch/code to remove the undocumented Zalo chat_id regex and now treat normalized nonblank Zalo targets as opaque string chat_id values, matching the upstream docs.

I added resolver coverage for abc.xyz, zalo:abc.xyz, and zl:abc.xyz, while keeping existing numeric/reported hex-like IDs covered and preserving rejection for blank/prefix-only targets.

Local validation are good too.

The remaining failed CI check appears to be runner infrastructure. checks-node-core-runtime-infra-process never reached the Node test shard; it stopped during Setup Node environment, and the annotation says the self-hosted runner lost communication with GitHub. All other build/test/proof checks are passing. Could your or someone with permission rerun that failed job? Tx!

steipete · 2026-07-07T09:59:27Z

Landed the same final two-file fix in #101548 as commit 6b76a30.

Thank you, @goutamadwant. Your Co-authored-by credit is preserved on the landed commit. I used a replacement maintainer branch because this fork branch was several weeks behind current main; updating it through the safe maintainer path would have carried more than 10,000 unrelated upstream files instead of keeping the patch reviewable.

Verification on the landed patch included 34 focused tests, a real openclaw message send --dry-run path for abc.xyz, exact-head CI run 28857111252, and a clean final autoreview. Closing this superseded PR now that the credited fix is on main.

openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. channel: zalo Channel integration: zalo size: XS labels Jun 16, 2026

vincentkoc force-pushed the fix/57594-zalo-hex-chat-id branch 2 times, most recently from 450c06d to 96397f4 Compare June 16, 2026 18:34

vincentkoc force-pushed the fix/57594-zalo-hex-chat-id branch 2 times, most recently from 05d8f27 to d3ded44 Compare June 16, 2026 19:21

vincentkoc force-pushed the fix/57594-zalo-hex-chat-id branch from d3ded44 to 824baed Compare June 16, 2026 19:37

clawsweeper Bot mentioned this pull request Jun 16, 2026

Zalo channel rejects valid non-numeric chat_id values for proactive outbound sends #57594

Closed

vincentkoc force-pushed the fix/57594-zalo-hex-chat-id branch from 824baed to 1955870 Compare June 16, 2026 22:17

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 16, 2026

vincentkoc requested changes Jun 16, 2026

View reviewed changes

goutamadwant changed the title ~~fix(zalo): accept hex chat ids for outbound sends~~ fix(zalo): accept string chat ids for outbound sends Jun 16, 2026

goutamadwant added 2 commits June 17, 2026 17:01

fix(zalo): accept hex chat ids for outbound sends

af9c536

fix(zalo): treat chat ids as opaque strings

e90bdb9

vincentkoc force-pushed the fix/57594-zalo-hex-chat-id branch from c66c802 to e90bdb9 Compare June 17, 2026 17:09

clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 19, 2026

steipete self-assigned this Jul 7, 2026

steipete mentioned this pull request Jul 7, 2026

fix(zalo): accept opaque string chat IDs #101548

Merged

steipete closed this Jul 7, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(zalo): accept string chat ids for outbound sends#93689

fix(zalo): accept string chat ids for outbound sends#93689
goutamadwant wants to merge 2 commits into
openclaw:mainfrom
goutamadwant:fix/57594-zalo-hex-chat-id

goutamadwant commented Jun 16, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 16, 2026 •

edited

Loading

Uh oh!

vincentkoc commented Jun 16, 2026

Uh oh!

vincentkoc left a comment

Uh oh!

goutamadwant commented Jun 16, 2026

Uh oh!

steipete commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

goutamadwant commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Linked context

Real behavior proof (required for external PRs)

Tests and validation

Risk checklist

Current review state

Uh oh!

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

Uh oh!

Uh oh!

vincentkoc commented Jun 16, 2026

Uh oh!

vincentkoc left a comment

Choose a reason for hiding this comment

Uh oh!

goutamadwant commented Jun 16, 2026

Uh oh!

steipete commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

goutamadwant commented Jun 16, 2026 •

edited

Loading

clawsweeper Bot commented Jun 16, 2026 •

edited

Loading