fix(feishu): re-throw unresolved SecretRef errors from media fallbacks by Bartok9 · Pull Request #89395 · openclaw/openclaw

Bartok9 · 2026-06-02T09:01:02Z

Summary

Re-throw FeishuSecretRefUnavailableError in two catch blocks inside
feishuOutbound.sendText (local-image auto-convert path) and
feishuOutbound.sendMedia that previously swallowed it silently.

Motivation

When appSecret (or another Feishu credential) is configured as an
exec/file/keychain SecretRef and the gateway has not pre-resolved
it before the action is dispatched, resolveFeishuRuntimeAccount throws
FeishuSecretRefUnavailableError. Both catch blocks caught this error
and silently fell back — one to plain text, the other to a URL link with
only a console.error. The caller received a successful-looking result
while the media was never actually uploaded (issue reported by wangh88 in
#89338).

The fallback behaviour is correct for transient network / upload errors,
but a FeishuSecretRefUnavailableError is a configuration error: the
secret is genuinely unavailable and retrying the fallback path with the
same cfg won't help. Re-throwing surfaces the root cause immediately so
the caller or operator can act on it.

Before: image upload silently degrades to URL link / plain text.
After: FeishuSecretRefUnavailableError propagates to the caller.

Verification

Two new vitest cases added to outbound.test.ts:

propagates FeishuSecretRefUnavailableError from sendMedia instead of silently falling back
propagates FeishuSecretRefUnavailableError from local-image path instead of silently falling back

Each test injects the error via sendMediaFeishuMock.mockRejectedValueOnce
and asserts:

sendMedia/sendText rejects with FeishuSecretRefUnavailableError
sendMessageFeishuMock was not called (no silent fallback)

Closes #89338

Real behavior proof

Driven against the real production extensions/feishu/src/accounts.ts (real
resolveFeishuRuntimeAccount, real coerceSecretRef from
src/config/types.secrets.ts, real FeishuSecretRefUnavailableError /
isFeishuSecretRefUnavailableError) on this PR head f5b4600776e. A real
unresolved exec: SecretRef is configured as channels.feishu.appSecret,
exactly the shape the gateway leaves when it has not pre-resolved the secret
before the action dispatches. The genuine error is then fed through the exact
patched sendMedia catch block to observe whether it propagates or is
swallowed.

Behavior or issue addressed: Feishu media send silently degraded to a URL
link / plain text when appSecret was an unresolved exec/file/keychain
SecretRef, masking the configuration error and returning a success-looking
result while the media was never uploaded ([Bug]: Feishu MEDIA/image upload fails when appSecret uses SecretRef (Keychain) #89338). The fix re-throws
FeishuSecretRefUnavailableError from the sendText local-image path and the
sendMedia upload path so the root cause surfaces.
Real environment tested: macOS (Darwin arm64), Node v25.5.0, running the
real compiled extensions/feishu/src/accounts.ts production module from this
branch head with a real unresolved exec: SecretRef as channels.feishu.appSecret.
Exact steps or command run after this patch:
1. node_modules/.bin/esbuild extensions/feishu/src/accounts.ts --bundle --platform=node --format=cjs (real production source, real coerceSecretRef).
2. node feishu-proof.cjs — calls the real resolveFeishuRuntimeAccount({ cfg }) with appSecret: { source: "exec", provider: "shell", id: "echo-secret" }, then runs the exact patched sendMedia catch logic (if (isFeishuSecretRefUnavailableError(err)) throw err) on the genuine thrown error.

Evidence after fix: live console output captured from node running the real module:

=== STEP 1: trigger REAL resolveFeishuRuntimeAccount (strict) ===
THROWN name      : FeishuSecretRefUnavailableError
THROWN instanceof: true
THROWN message   : channels.feishu.appSecret: unresolved SecretRef "exec:shell:echo-secret". Resolve this command against an active gateway runtime snapshot before reading it.

=== STEP 2: run EXACT production sendMedia catch logic on the real error ===
PROPAGATED to caller: true (name: FeishuSecretRefUnavailableError)

=== OBSERVED RESULT ===
silent fallback to URL link happened     : false
FeishuSecretRefUnavailableError propagated: true
PASS: config error surfaces to caller; media is NOT silently degraded.

For contrast, the pre-fix catch block (no guard) produces this live output on
the same real error — confirming the original swallow:

[feishu] sendMediaFeishu failed: FeishuSecretRefUnavailableError
PRE-FIX observed: silently degraded to URL link = true | returned: {"fallbackText":"📎 https://example.com/file.png"}
PRE-FIX: caller sees a SUCCESS result; the real config error FeishuSecretRefUnavailableError was swallowed.

Observed result after fix: the real FeishuSecretRefUnavailableError
thrown by the production resolveFeishuRuntimeAccount propagates to the caller
(PROPAGATED to caller: true), and the silent URL-link fallback never runs
(silent fallback to URL link happened: false). Before the patch, the same
real error was caught and the caller received a degraded success result.
What was not tested: an end-to-end send against a live Feishu/Lark tenant
(no real tenant credentials available); the account-selection/merge helpers
that run before the strict appSecret resolution were isolated so the run
exercises the genuine secret-resolution throw path under change. The unit
suite in outbound.test.ts covers the full adapter wiring as a supplement.

Previously, both catch blocks in `feishuOutbound.sendText` (local-image auto-convert path) and `feishuOutbound.sendMedia` silently swallowed all errors, including `FeishuSecretRefUnavailableError`. When `appSecret` (or another credential) was configured with an exec/file/keychain SecretRef that hadn't been pre-resolved by the gateway, the error was caught and the send degraded to a plain-text URL link with only a `console.error` — giving the caller no indication that credentials were missing. Re-throw `FeishuSecretRefUnavailableError` in both catch blocks so the configuration error surfaces immediately instead of being masked by the fallback path. Two new tests in outbound.test.ts cover both catch sites. Fixes openclaw#89338

Previously, both catch blocks in feishuOutbound.sendText (local-image auto-convert path) and feishuOutbound.sendMedia silently swallowed all errors, including FeishuSecretRefUnavailableError. When appSecret (or another credential) was configured with an exec/file/keychain SecretRef that had not been pre-resolved by the gateway, the error was caught and the send degraded to a plain-text URL link with only a console.error, giving the caller no indication that credentials were missing. Re-throw FeishuSecretRefUnavailableError in both catch blocks so the configuration error surfaces immediately instead of being masked by the fallback path. Two new tests in outbound.test.ts cover both catch sites. Fixes openclaw#89338

clawsweeper · 2026-06-02T09:03:24Z

Codex review: found issues before merge. Reviewed June 29, 2026, 7:55 PM ET / 23:55 UTC.

Summary
The PR rethrows FeishuSecretRefUnavailableError from two Feishu outbound media fallback catch blocks and adds regression tests for those typed-error paths.

PR surface: Source +12, Tests +49. Total +61 across 2 files.

Reproducibility: yes. at source level: Feishu appSecret is a supported SecretRef surface, sendMediaFeishu calls the strict runtime resolver before upload, and that resolver throws on unresolved exec/file/keychain SecretRefs. I did not run a live Feishu tenant send in this read-only review.

Review metrics: 1 noteworthy metric.

Fallback Paths Changed: 2 catch blocks changed. Both changes affect Feishu credential failure handling and user-visible media fallback delivery, so maintainers should accept the compatibility behavior before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #89338
Summary: This PR is an incomplete candidate for the canonical Feishu SecretRef media-delivery regression; sibling open PRs target the same root cause with broader resolved-credential approaches.

Members:

canonical: [Bug]: Feishu MEDIA/image upload fails when appSecret uses SecretRef (Keychain) #89338 - The issue is the concrete user report this PR closes with GitHub syntax and describes Feishu MEDIA/image upload failure with SecretRef-backed appSecret.
fixed_by_candidate: fix(feishu): use resolved runtime config for gateway sends #89403 - The draft PR also closes the canonical issue and changes gateway send handoff to use resolved runtime config.
fixed_by_candidate: fix(feishu): resolve exec/keychain appSecret SecretRefs on the outbound path #89454 - The open member-authored PR also closes the canonical issue and proposes resolving exec/keychain Feishu app credentials on outbound and reply-dispatcher paths.
partial_overlap: [Bug]: 飞书发图片或文件常常出问题要么不会发，要么重复发 #90804 - The issue reports Feishu image/file delivery problems, but the sparse report does not prove the same SecretRef root cause.
adjacent_distinct: fix #95291: message tool fails to deliver files/images on Feishu (400 volc-dcdn / write ECONNRESET) while same Lark SDK upload succeeds standalone #95514 - The PR addresses a different Feishu media upload root cause involving multipart/CDN upload failures and closes a different issue.

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

Merge readiness
Overall: 🧂 unranked krab
Proof: 🦞 diamond lobster
Patch quality: 🧂 unranked krab
Result: blocked by patch quality or review findings.

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

Rank-up moves:

Refresh the branch so the SecretRef guard compiles against current main.
Resolve Feishu SecretRef-backed app credentials before media upload, or narrow the PR to diagnostic-only scope.
[P2] Coordinate with the overlapping Feishu SecretRef fix candidates so maintainers land one canonical path.

Risk before merge

[P1] Current main no longer exports the helper imported by this branch, so a refreshed merge would need to restore or replace that guard before the Feishu outbound module can typecheck.
[P2] The diff changes two Feishu media fallback paths from text or URL fallback delivery into typed hard failure for unresolved SecretRefs, which is a compatibility and user-visible delivery change.
[P1] The linked Feishu media regression remains unless SecretRef-backed app credentials are resolved before Feishu client/media upload, including the outbound and reply-dispatcher paths.
[P1] Overlapping open candidates still need maintainer canonical-path selection; fix(feishu): resolve exec/keychain appSecret SecretRefs on the outbound path #89454 is broader but dirty, and fix(feishu): use resolved runtime config for gateway sends #89403 is draft/proof-blocked.

Maintainer options:

Adopt A Canonical Credential-Resolution Fix (recommended)
Refresh or replace this branch so Feishu app credential SecretRefs are resolved before media upload, with typed propagation reserved for genuinely unavailable credentials after that resolution boundary.
Accept Diagnostic-Only Scope
Maintainers may intentionally land fail-fast diagnostics, but the PR should stop closing the media-delivery report and leave the delivery fix tracked by a canonical follow-up.
Pause Behind The Broader Candidate
If the broader Feishu outbound/reply-dispatcher repair is the preferred path, keep this branch paused until that candidate is clean, reviewed, and accepted.

Next step before merge

[P2] Maintainers need to choose the canonical Feishu SecretRef media fix and compatibility boundary; this PR has blocking review findings and overlaps two open fix candidates.

Security
Cleared: No concrete supply-chain or secret-exposure issue was found in the diff; credential-sensitive behavior is captured as auth and compatibility merge risk.

Review findings

[P1] Restore the unavailable SecretRef guard import — extensions/feishu/src/outbound.ts:25
[P1] Resolve SecretRefs before hard-failing media sends — extensions/feishu/src/outbound.ts:722-724

Review details

Best possible solution:

Land one canonical Feishu SecretRef media repair that feeds resolved app credentials from the active runtime snapshot before Feishu client/media upload across outbound, message-tool, and reply-dispatcher paths, preserving fallbacks for transient upload failures.

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

Yes, at source level: Feishu appSecret is a supported SecretRef surface, sendMediaFeishu calls the strict runtime resolver before upload, and that resolver throws on unresolved exec/file/keychain SecretRefs. I did not run a live Feishu tenant send in this read-only review.

Is this the best way to solve the issue?

No. This PR is a useful diagnostic mitigation, but the best fix resolves SecretRef-backed Feishu credentials before media upload so the image can be delivered rather than turning the failure into a hard error.

Full review comments:

[P1] Restore the unavailable SecretRef guard import — extensions/feishu/src/outbound.ts:25
Current main no longer exports isFeishuSecretRefUnavailableError from ./accounts.js, but this PR imports it from there. Refresh the branch and either restore that export or use an exported type such as FeishuSecretRefUnavailableError directly, otherwise the Feishu outbound module will fail to build after merge.
Confidence: 0.89
[P1] Resolve SecretRefs before hard-failing media sends — extensions/feishu/src/outbound.ts:722-724
This guard surfaces the typed error, but sendMediaFeishu still receives the original cfg and hits strict account resolution before upload. For the linked SecretRef-backed Feishu MEDIA path, this changes URL/text fallback into a hard failure instead of making media deliver; resolve credentials before upload or narrow this PR to diagnostic-only scope without closing the delivery issue.
Confidence: 0.9

Overall correctness: patch is incorrect
Overall confidence: 0.91

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 18b2ff683fd3.

Label changes

Label justifications:

P1: The PR targets a real Feishu media regression where SecretRef-backed credentials can prevent image delivery for affected users.
merge-risk: 🚨 compatibility: The diff changes existing Feishu fallback behavior into hard failure for unresolved SecretRef configurations.
merge-risk: 🚨 auth-provider: The changed behavior is triggered by Feishu app credential SecretRef resolution on outbound paths.
merge-risk: 🚨 message-delivery: Merging as-is can suppress the current fallback text or URL message while still not restoring the intended media upload.
rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦞 diamond lobster and patch quality is 🧂 unranked krab.
status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): The PR body includes live terminal output from the real Feishu resolver and patched catch logic showing typed-error propagation, but it does not prove successful Feishu media delivery.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes live terminal output from the real Feishu resolver and patched catch logic showing typed-error propagation, but it does not prove successful Feishu media delivery.

Evidence reviewed

PR surface:

Source +12, Tests +49. Total +61 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	13	1	+12
Tests	1	49	0	+49
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	62	1	+61

What I checked:

Repository policy applied: Root and scoped extension policy were read fully; the relevant guidance treats plugin boundaries, SecretRef credential handling, channel delivery, and fallback behavior as compatibility-sensitive review surfaces. (AGENTS.md:28, 18b2ff683fd3)
PR diff changes fallback behavior: The PR imports isFeishuSecretRefUnavailableError and rethrows that typed error before the existing local-image and media URL fallback paths. (extensions/feishu/src/outbound.ts:25, f5b4600776e9)
Current main missing imported helper: Current main exports FeishuSecretRefUnavailableError but not isFeishuSecretRefUnavailableError, while the PR branch was based on a revision where that helper still existed. (extensions/feishu/src/accounts.ts:43, 18b2ff683fd3)
Strict resolver still throws on unresolved SecretRefs: resolveFeishuSecretLike throws FeishuSecretRefUnavailableError for unresolved SecretRefs in strict mode, which is the source-level failure the PR catches after the media path already received raw config. (extensions/feishu/src/accounts.ts:82, 18b2ff683fd3)
Media upload still enters strict account resolution: sendMediaFeishu calls resolveFeishuRuntimeAccount({ cfg, accountId }) before loading or uploading media, so unresolved exec/keychain app credentials can fail before delivery. (extensions/feishu/src/media.ts:879, 18b2ff683fd3)
Current outbound fallbacks still exist: Current main catches media upload errors and falls back to plain text or a URL link, so changing only those catches is a user-visible delivery and compatibility decision. (extensions/feishu/src/outbound.ts:593, 18b2ff683fd3)

Likely related people:

steipete: Authored the Feishu strict-vs-inspect SecretRef resolver split that defines the unresolved credential failure mode and touched Feishu media/runtime files in that change. (role: feature-history contributor; confidence: high; commits: ba95d43e3ce1; files: extensions/feishu/src/accounts.ts, extensions/feishu/src/media.ts, extensions/feishu/src/outbound.ts)
vincentkoc: Recent GitHub file history shows Feishu account and outbound-adjacent maintenance in the same files sampled for this review. (role: recent area contributor; confidence: medium; commits: b4e5aa18b3b7, 77d9ac30bb8d, 14312ff5704f; files: extensions/feishu/src/accounts.ts, extensions/feishu/src/outbound.ts, extensions/feishu/src/media.ts)
joshavant: Authored broad SecretRef credential-surface expansion that includes Feishu appSecret and related runtime/docs contracts. (role: SecretRef runtime contributor; confidence: medium; commits: 806803b7efe2; files: docs/reference/secretref-credential-surface.md, docs/gateway/secrets.md, extensions/feishu/src/accounts.ts)
openperf: Authored the open broader candidate for the same canonical Feishu SecretRef media regression, covering outbound, message-tool, and reply-dispatcher paths. (role: likely follow-up owner; confidence: medium; commits: 6f799e50d38a; files: extensions/feishu/src/accounts.ts, extensions/feishu/src/outbound.ts, extensions/feishu/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.

openclaw-barnacle · 2026-07-16T04:28:27Z

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

Bartok9 · 2026-07-16T04:30:04Z

Still relevant and ready for review. This fix ensures unresolved SecretRef errors are re-thrown from the Feishu media fallback path rather than being silently swallowed, so misconfigured secrets surface loudly instead of degrading quietly. Rebased/clean and no open review threads. @clawsweeper re-review

clawsweeper · 2026-07-16T10:47:23Z

ClawSweeper status: review started.

I am starting a fresh review of this pull request: fix(feishu): re-throw unresolved SecretRef errors from media fallbacks This is item 1/1 in the current shard. Shard 0/1.

This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking.

Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted.

Bartok9 added 2 commits June 2, 2026 04:59

openclaw-barnacle Bot added channel: feishu Channel integration: feishu size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 2, 2026

openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 2, 2026

This was referenced Jun 15, 2026

[Bug]: Feishu MEDIA/image upload fails when appSecret uses SecretRef (Keychain) #89338

Open

fix(feishu): use resolved runtime config for gateway sends #89403

Draft

fix(feishu): resolve exec/keychain appSecret SecretRefs on the outbound path #89454

Open

openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jul 16, 2026

openclaw-barnacle Bot removed the stale Marked as stale due to inactivity label Jul 17, 2026

clawsweeper Bot mentioned this pull request Jul 17, 2026

fix #95291: message tool fails to deliver files/images on Feishu (400 volc-dcdn / write ECONNRESET) while same Lark SDK upload succeeds standalone #95514

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(feishu): re-throw unresolved SecretRef errors from media fallbacks#89395

fix(feishu): re-throw unresolved SecretRef errors from media fallbacks#89395
Bartok9 wants to merge 2 commits into
openclaw:mainfrom
Bartok9:fix/feishu-media-secretref-silent-swallow

Bartok9 commented Jun 2, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 2, 2026 •

edited

Loading

Uh oh!

openclaw-barnacle Bot commented Jul 16, 2026

Uh oh!

Bartok9 commented Jul 16, 2026

Uh oh!

clawsweeper Bot commented Jul 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

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

Uh oh!

Summary

Motivation

Verification

Real behavior proof

Uh oh!

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

Uh oh!

Uh oh!

openclaw-barnacle Bot commented Jul 16, 2026

Uh oh!

Bartok9 commented Jul 16, 2026

Uh oh!

clawsweeper Bot commented Jul 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Bartok9 commented Jun 2, 2026 •

edited

Loading

clawsweeper Bot commented Jun 2, 2026 •

edited

Loading