fix(feishu): keep top-level appSecret SecretRef active for the implicit default account (fixes #96929) by zw-xysk · Pull Request #96965 · openclaw/openclaw

zw-xysk · 2026-06-26T06:50:51Z

What Problem This Solves

When a Feishu channel has both a top-level appSecret using SecretRef format and sub-accounts (e.g. accounts.resource-shrimp) with their own inline appSecret, the secrets resolver skips resolving the top-level SecretRef. This causes the [default] channel to fail on startup with:

channels.feishu.appSecret: unresolved SecretRef "env:default:FEISHU_APP_SECRET".
Resolve this command against an active gateway runtime snapshot before reading it.

The resolver logs SECRETS_REF_IGNORED_INACTIVE_SURFACE because it only checks whether any explicit account inherits the field — but the implicit [default] channel itself always uses the top-level value.

Why This Change Was Made

isBaseFieldActiveForChannelSurface returns true only when at least one enabled account does NOT override the field. When all accounts provide their own inline values, the top-level SecretRef was incorrectly marked as inactive.

Feishu account listing (extensions/feishu/src/accounts.ts) always creates an implicit default account from top-level appId + appSecret, even when named accounts exist. This fix detects that implicit default account and keeps the top-level appSecret active accordingly.

The change stays Feishu-scoped instead of modifying the shared collectSimpleChannelFieldAssignments helper, avoiding any risk of affecting sibling channel semantics (Discord, Slack, IRC, etc.).

User Impact

Users with multi-account Feishu configurations where all accounts provide their own appSecret can now use SecretRef format for the top-level appSecret without the [default] channel failing to start. Previously, the only workaround was replacing the SecretRef with an inline secret string.

Changes Made

extensions/feishu/src/secret-contract.ts: Replaced collectSimpleChannelFieldAssignments with inline collectSecretInputAssignment logic that checks for the implicit default account via hasConfiguredSecretInputValue(appId) && hasConfiguredSecretInputValue(appSecret).
src/secrets/runtime-external-channel-audit.test.ts: Added regression test verifying top-level SecretRef is resolved when all accounts override the field.

Evidence

Unit tests: node scripts/run-vitest.mjs run src/secrets/runtime-external-channel-audit.test.ts — 8/8 passed
Regression (Discord): node scripts/run-vitest.mjs run src/secrets/runtime-discord-surface.test.ts — 8/8 passed
Regression (Feishu accounts): node scripts/run-vitest.mjs run extensions/feishu/src/accounts.test.ts — 23/23 passed

Real behavior proof

Config tested: Top-level appSecret as env:default:FEISHU_APP_SECRET with one sub-account overriding appSecret inline.

Environment: OpenClaw 2026.6.10 (7e4a082 — this fix), Linux, Node 24.13.1

secrets audit result:

{
  "resolution": {
    "refsChecked": 1,
    "skippedExecRefs": 0,
    "resolvabilityComplete": true
  },
  "summary": {
    "plaintextCount": 1,
    "unresolvedRefCount": 0,
    "shadowedRefCount": 0,
    "legacyResidueCount": 0
  }
}

✅ Top-level SecretRef resolved (refsChecked=1, unresolvedRefCount=0)
✅ No SECRETS_REF_IGNORED_INACTIVE_SURFACE warning emitted
✅ Gateway started cleanly — no errors, no crashes
✅ Sub-account inline secret handled correctly (plaintextCount=1 is expected for inline secrets)

What was not tested: Live Feishu platform message delivery (not required — fix is in config/secret resolution layer).

AI-assisted

…it default account When a Feishu channel has top-level appId/appSecret using SecretRef format and sub-accounts with their own inline appSecret, the secrets resolver marks the top-level SecretRef as inactive because isBaseFieldActiveForChannelSurface only checks whether any explicit account inherits the field. Feishu account listing always creates an implicit default account from top-level credentials. The fix detects this implicit default account and keeps the top-level appSecret active accordingly, without changing the shared channel secret helper semantics for other channels. Fixes openclaw#96929 Signed-off-by: 赵旺0668001248 <[email protected]>

zw-xysk · 2026-06-26T06:51:01Z

@clawsweeper re-review

clawsweeper · 2026-06-26T06:51:04Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

clawsweeper · 2026-06-26T06:52:14Z

Codex review: needs maintainer review before merge. Reviewed June 26, 2026, 2:56 AM ET / 06:56 UTC.

Summary
The PR changes Feishu secret-contract handling so a top-level Feishu appSecret SecretRef remains active for the implicit default account when named accounts override appSecret.

PR surface: Source +34, Tests +34. Total +68 across 2 files.

Reproducibility: yes. source-level: current main combines Feishu implicit default account creation with top-level appSecret inactive-surface filtering when every explicit account overrides the field. I did not run tests or a live gateway startup because this review is read-only.

Review metrics: 1 noteworthy metric.

SecretRef activation surface: 1 changed. The PR changes when the existing Feishu credential path channels.feishu.appSecret is considered active for configs with named accounts.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #96929
Summary: This PR is a candidate fix for the canonical Feishu top-level appSecret SecretRef startup issue; an older open sibling PR targets the same root cause but is not a reason to close this proof-backed PR.

Members:

canonical: SecretRef on top-level Feishu appSecret skipped when sub-accounts have their own secrets (SECRETS_REF_IGNORED_INACTIVE_SURFACE) #96929 - The issue describes the same Feishu top-level appSecret SecretRef inactive-surface failure and is referenced with closing syntax by this PR.
same_root_cause: fix(feishu): keep default appSecret refs active #96933 - The sibling PR changes the same Feishu appSecret activation path for the same issue but previously lacked real behavior proof, so it is not a safer superseding target.

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

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🦞 diamond lobster
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:

none.

Next step before merge

No automated repair lane is needed; this PR is ready for normal maintainer review and required check gating.

Security
Cleared: The diff changes Feishu SecretRef activation logic and a regression test only; no dependency, workflow, lockfile, install script, or new code-execution surface was introduced.

Review details

Best possible solution:

Land a narrow Feishu-owned fix that mirrors Feishu account listing for top-level appSecret activation while preserving shared inactive-surface filtering for sibling channels.

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

Yes, source-level: current main combines Feishu implicit default account creation with top-level appSecret inactive-surface filtering when every explicit account overrides the field. I did not run tests or a live gateway startup because this review is read-only.

Is this the best way to solve the issue?

Yes: the Feishu-owned active check is the narrowest maintainable fix because changing the shared helper would alter sibling channel credential semantics. The PR preserves named-account handling and adds focused regression coverage.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 81e53202f25f.

Label changes

Label justifications:

P1: The PR fixes a linked Feishu channel startup failure for SecretRef-backed credentials in a real channel workflow.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes copied after-fix secrets audit output and a gateway-startup result for the reported config, which is enough real behavior proof for this non-visual config-resolution fix.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied after-fix secrets audit output and a gateway-startup result for the reported config, which is enough real behavior proof for this non-visual config-resolution fix.

Evidence reviewed

PR surface:

Source +34, Tests +34. Total +68 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	42	8	+34
Tests	1	35	1	+34
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	77	9	+68

What I checked:

Root policy read: Read the full root policy; its plugin-boundary and compatibility-sensitive SecretRef/config review guidance applies to this PR. (AGENTS.md:1, 81e53202f25f)
Scoped extension policy read: Read extensions/AGENTS.md fully; the PR keeps production changes inside the Feishu plugin boundary and imports through the plugin SDK subpath. (extensions/AGENTS.md:1, 81e53202f25f)
Current-main source reproduction: Current main still delegates channels.feishu.appSecret to collectSimpleChannelFieldAssignments with the inactive reason reported by the issue. (extensions/feishu/src/secret-contract.ts:92, 81e53202f25f)
Implicit default account invariant: Feishu account listing creates an implicit default account when top-level appId and appSecret are configured, even with named accounts present. (extensions/feishu/src/accounts.ts:21, 81e53202f25f)
Shared helper mismatch: The shared active check only keeps a top-level field active with explicit accounts when an enabled account lacks that field, which explains the skipped top-level appSecret when all named accounts override it. (src/secrets/channel-secret-basic-runtime.ts:88, 81e53202f25f)
Runtime caller path: Gateway startup lists plugin account IDs, resolves each account, then passes the runtime config to startAccount; Feishu startAccount resolves runtime credentials in strict mode. (src/gateway/server-channels.ts:435, 81e53202f25f)

Likely related people:

steipete: Remote path history shows steipete authored the implicit default account preservation work and recent Feishu account behavior that this PR mirrors in the secret contract. (role: Feishu account-resolution contributor; confidence: high; commits: 16ef041b5d86, 0ee5f47fbabb; files: extensions/feishu/src/accounts.ts, src/channels/plugins/account-helpers.ts)
vincentkoc: Remote path history shows vincentkoc split and narrowed the channel secret runtime helper family used by Feishu's secret contract. (role: shared SecretRef helper contributor; confidence: high; commits: dfb6c9c92079, 78639eff76af; files: src/secrets/channel-secret-basic-runtime.ts, src/plugin-sdk/channel-secret-basic-runtime.ts, extensions/feishu/src/secret-contract.ts)
Ayaan Zaidi: Local blame in this checkout maps the current implicated lines to the available graft boundary commit, useful as checkout provenance but weaker than the remote path history. (role: graft-boundary committer; confidence: low; commits: dc12b998da; files: extensions/feishu/src/secret-contract.ts, extensions/feishu/src/accounts.ts, src/secrets/channel-secret-basic-runtime.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.

…it default account (openclaw#96965) When a Feishu channel has top-level appId/appSecret using SecretRef format and sub-accounts with their own inline appSecret, the secrets resolver marks the top-level SecretRef as inactive because isBaseFieldActiveForChannelSurface only checks whether any explicit account inherits the field. Feishu account listing always creates an implicit default account from top-level credentials. The fix detects this implicit default account and keeps the top-level appSecret active accordingly, without changing the shared channel secret helper semantics for other channels. Fixes openclaw#96929 Signed-off-by: 赵旺0668001248 <[email protected]>

…it default account (openclaw#96965) When a Feishu channel has top-level appId/appSecret using SecretRef format and sub-accounts with their own inline appSecret, the secrets resolver marks the top-level SecretRef as inactive because isBaseFieldActiveForChannelSurface only checks whether any explicit account inherits the field. Feishu account listing always creates an implicit default account from top-level credentials. The fix detects this implicit default account and keeps the top-level appSecret active accordingly, without changing the shared channel secret helper semantics for other channels. Fixes openclaw#96929 Signed-off-by: 赵旺0668001248 <[email protected]> (cherry picked from commit 4e7992c)

zw-xysk requested a review from a team as a code owner June 26, 2026 06:50

openclaw-barnacle Bot added channel: feishu Channel integration: feishu size: S labels Jun 26, 2026

This was referenced Jun 27, 2026

fix(secrets): resolve top-level SecretRef when all channel accounts override the field (fixes #96929) (AI-assisted) #96935

Closed

fix(feishu): keep default appSecret refs active #96933

Closed

vincentkoc merged commit 4e7992c into openclaw:main Jun 29, 2026
138 of 145 checks passed

github-actions Bot mentioned this pull request Jun 29, 2026

📡 Upstream Digest — 2026-06-29 02:42 UTC curtismercier/openclaw-mods#1171

Open

clawsweeper Bot mentioned this pull request Jul 5, 2026

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

Draft

vincentkoc mentioned this pull request Jul 6, 2026

chore(jobs): prune resolved OpenClaw inbox jobs openclaw/clownfish#221

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(feishu): keep top-level appSecret SecretRef active for the implicit default account (fixes #96929)#96965

fix(feishu): keep top-level appSecret SecretRef active for the implicit default account (fixes #96929)#96965
vincentkoc merged 1 commit into
openclaw:mainfrom
zw-xysk:fix/issue-96929-feishu-secretref-default-account

zw-xysk commented Jun 26, 2026

Uh oh!

zw-xysk commented Jun 26, 2026

Uh oh!

clawsweeper Bot commented Jun 26, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 26, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

zw-xysk commented Jun 26, 2026

What Problem This Solves

Why This Change Was Made

User Impact

Changes Made

Evidence

Real behavior proof

Uh oh!

zw-xysk commented Jun 26, 2026

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

clawsweeper Bot commented Jun 26, 2026 •

edited

Loading

clawsweeper Bot commented Jun 26, 2026 •

edited

Loading