fix(feishu): keep default appSecret refs active by WilliamK112 · Pull Request #96933 · openclaw/openclaw

WilliamK112 · 2026-06-26T04:24:44Z

What Problem This Solves

When Feishu has top-level appId/appSecret credentials and named accounts that each override appSecret, the secrets runtime can incorrectly mark channels.feishu.appSecret inactive. That leaves the implicit default Feishu account with an unresolved top-level SecretRef at startup.

Why This Change Was Made

Feishu account resolution treats top-level credentials as an implicit default account even when named accounts exist. The Feishu secret contract now mirrors that account-listing behavior for appSecret: top-level appSecret refs remain active when top-level credentials define the implicit default account, while named account appSecret refs still use their existing enabled-account rules.

The change stays Feishu-scoped instead of changing the shared channel secret helper semantics for Discord, Slack, IRC, or other account-scoped channels. It also keeps stale top-level appSecret refs inactive when an explicit accounts.default entry overrides or disables the default account, because no enabled Feishu account inherits that top-level value in those configs.

User Impact

Users can keep a top-level Feishu appSecret SecretRef for the default account while adding named Feishu accounts with their own inline or referenced app secrets. The default account no longer fails startup because its top-level SecretRef was skipped as inactive.

Configs with an explicit accounts.default.appSecret override or disabled accounts.default do not start resolving stale top-level channels.feishu.appSecret SecretRefs that no enabled runtime account reads.

Evidence

Runtime proof from local terminal on 867bc4ba48a74c28b041dde3e34ef1a830f4c45c using only synthetic/redacted secrets. OPENCLAW_BUNDLED_PLUGINS_DIR=<repo>/extensions points the production bundled-plugin public-surface loader at this checkout's Feishu secret-contract-api.ts, so the command exercises the PR head source instead of the previously built dist artifact.

OPENCLAW_BUNDLED_PLUGINS_DIR=<repo>/extensions node --import tsx --input-type=module <runtime-proof-script>

Output:

[runtime-proof] Feishu implicit default account SecretRef resolved
[runtime-proof] channels.feishu.appSecret=default-secret-redacted
[runtime-proof] channels.feishu.accounts.resource.appSecret=resource-secret-redacted
[runtime-proof] warningCount=0

OPENCLAW_BUNDLED_PLUGINS_DIR=<repo>/extensions node --import tsx --input-type=module <explicit-default-stale-ref-proof-script>

Output:

[runtime-proof] explicit default overrides top-level appSecret
[runtime-proof] topLevelAppSecretStillSecretRef=true
[runtime-proof] resourceAccountAppSecret=resource-secret-redacted
[runtime-proof] warnings=channels.feishu.appSecret
[runtime-proof] explicit default disabled
[runtime-proof] topLevelAppSecretStillSecretRef=true
[runtime-proof] resourceAccountAppSecret=resource-secret-redacted
[runtime-proof] warnings=channels.feishu.appSecret

node scripts/run-vitest.mjs src/secrets/runtime-external-channel-audit.test.ts extensions/feishu/src/accounts.test.ts src/secrets/runtime-discord-surface.test.ts -- --reporter=verbose
- Passed: resolves Feishu top-level appSecret refs for the implicit default account.
- Passed: skips stale Feishu top-level appSecret refs when explicit default overrides them.
- Passed: skips stale Feishu top-level appSecret refs when explicit default is disabled.
- Result: secrets shard Test Files 2 passed (2), Tests 18 passed (18); Feishu extension shard Test Files 1 passed (1), Tests 24 passed (24).
corepack pnpm format:check -- extensions/feishu/src/secret-contract.ts src/secrets/runtime-external-channel-audit.test.ts extensions/feishu/src/accounts.test.ts
- Result: All matched files use the correct format.
git diff --check
- Result: no whitespace errors.

CI note: GitHub Real behavior proof is green on this head. The remaining red CI job is checks-node-compact-small-7, which timed out in src/gateway/server-startup-session-migration.test.ts on the PR merge ref. That file/test comes from current main on the merge ref, not from this Feishu secret-contract diff; rerun was attempted with gh run rerun --failed but GitHub rejected it because this account does not have repository admin rights.

Signed-off-by: WilliamK112 <[email protected]>

clawsweeper · 2026-06-26T04:28:03Z

Codex review: needs maintainer review before merge. Reviewed June 26, 2026, 10:41 PM ET / 02:41 UTC.

Summary
The PR changes Feishu secret-contract collection so top-level channels.feishu.appSecret SecretRefs remain active for implicit default accounts and adds regression coverage for implicit-default and explicit-default stale-ref cases.

PR surface: Source +36, Tests +130. Total +166 across 3 files.

Reproducibility: yes. source-level: current main and v2026.6.10 combine Feishu implicit default account listing with shared inactive-surface filtering, so a top-level appSecret SecretRef is skipped when all named accounts override appSecret. I did not run local tests in this read-only review, but the PR body now includes terminal runtime proof on the head.

Review metrics: 1 noteworthy metric.

SecretRef activation surface: 1 credential path changed. The PR changes when existing channels.feishu.appSecret SecretRefs are considered active, so reviewers should focus on upgrade and credential-resolution behavior.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #96929
Summary: This PR is a candidate fix for the canonical Feishu top-level appSecret SecretRef inactive-surface startup bug; sibling PRs target the same root cause but do not make this branch obsolete.

Members:

canonical: SecretRef on top-level Feishu appSecret skipped when sub-accounts have their own secrets (SECRETS_REF_IGNORED_INACTIVE_SURFACE) #96929 - The PR body uses closing syntax for this issue, and the issue describes the same Feishu top-level appSecret SecretRef inactive-surface startup failure.
same_root_cause: fix(feishu): keep top-level appSecret SecretRef active for the implicit default account (fixes #96929) #96965 - This open sibling PR also fixes the Feishu implicit-default appSecret SecretRef bug and has proof, but it is a separate landing candidate rather than a merged superseding fix.
same_root_cause: fix(secrets): resolve top-level SecretRef when all channel accounts override the field (fixes #96929) (AI-assisted) #96935 - This closed unmerged sibling targeted the same issue through a broader shared-helper change, so it is related history but not a viable canonical landing path.
adjacent_distinct: [Bug]: Feishu MEDIA/image upload fails when appSecret uses SecretRef (Keychain) #89338 - This open issue also involves Feishu appSecret SecretRefs, but it targets media/outbound credential resolution rather than the startup inactive-surface mismatch fixed here.

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: 🐚 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:

[P2] Rerun or resolve checks-node-compact-small-7 before merge.
Choose one landing candidate between this PR and fix(feishu): keep top-level appSecret SecretRef active for the implicit default account (fixes #96929) #96965.

Risk before merge

[P2] Required CI still has one gateway session-migration timeout on the PR merge ref; logs point outside the Feishu diff, but the check should be rerun or resolved before merge.
[P1] There are two open candidate PRs for the same Feishu startup bug, so maintainers should choose one landing path and avoid merging duplicate activation changes.

Maintainer options:

Decide the mitigation before merge
Land one Feishu-scoped fix that keeps top-level appSecret active only for the implicit default account and preserves explicit-default stale-ref warnings, after required checks are green.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

No automated repair lane is needed because there are no concrete code findings; maintainers need to choose the Feishu fix candidate and get the remaining CI shard green.

Security
Cleared: The diff changes Feishu SecretRef activation logic and tests only; it does not introduce dependencies, workflow changes, lockfile changes, install scripts, or new code-execution surfaces.

Review details

Best possible solution:

Land one Feishu-scoped fix that keeps top-level appSecret active only for the implicit default account and preserves explicit-default stale-ref warnings, after required checks are green.

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

Yes, source-level: current main and v2026.6.10 combine Feishu implicit default account listing with shared inactive-surface filtering, so a top-level appSecret SecretRef is skipped when all named accounts override appSecret. I did not run local tests in this read-only review, but the PR body now includes terminal runtime proof on the head.

Is this the best way to solve the issue?

Yes, this is the best fix shape: Feishu-owned activation matches Feishu account listing and is narrower than changing the shared simple collector for every account-scoped channel. The remaining question is which sibling candidate to land, not whether this repair belongs in Feishu.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 56259606d15f.

Label changes

Label justifications:

P1: The PR fixes a linked Feishu channel startup failure for SecretRef-backed appSecret credentials in a real channel workflow.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes copied terminal runtime output showing after-fix implicit-default SecretRef resolution and explicit-default stale-ref warning behavior with synthetic redacted secrets.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied terminal runtime output showing after-fix implicit-default SecretRef resolution and explicit-default stale-ref warning behavior with synthetic redacted secrets.

Evidence reviewed

PR surface:

Source +36, Tests +130. Total +166 across 3 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	44	8	+36
Tests	2	130	0	+130
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	3	174	8	+166

What I checked:

Repository policy read: Root AGENTS.md and scoped extensions/AGENTS.md were read fully; their plugin-boundary and SecretRef/config compatibility guidance shaped this review. (AGENTS.md:20, 56259606d15f)
Current main still skips the reported Feishu top-level appSecret path: Current main still delegates channels.feishu.appSecret to collectSimpleChannelFieldAssignments with the same inactive-surface warning text from the linked issue. (extensions/feishu/src/secret-contract.ts:92, 56259606d15f)
Current main still creates an implicit Feishu default account: Feishu account listing treats configured top-level appId and appSecret as an implicit default account even when named accounts exist. (extensions/feishu/src/accounts.ts:21, 56259606d15f)
Shared active-surface helper explains the mismatch: With explicit accounts present, the shared helper only keeps a top-level field active when an enabled account lacks that field, which misses Feishu's implicit default account when all named accounts override appSecret. (src/secrets/channel-secret-basic-runtime.ts:88, 56259606d15f)
PR head implementation is Feishu-scoped: PR head adds a Feishu-local implicit-default check, excludes explicit default-account ownership, and then collects the top-level appSecret assignment only when active. (extensions/feishu/src/secret-contract.ts:97, 867bc4ba48a7)
PR head regression coverage: PR head covers resolving the implicit-default top-level SecretRef and keeping stale top-level refs inactive when accounts.default overrides or disables the default account. (src/secrets/runtime-external-channel-audit.test.ts:404, 867bc4ba48a7)

Likely related people:

steipete: Peter Steinberger authored account-helper and Feishu account-resolution refactors that preserve implicit default account behavior, which this PR mirrors in Feishu secret activation. (role: implicit default account contributor; confidence: high; commits: 3c071a397fca, bddb6fca7b8d, ba95d43e3ce1; files: extensions/feishu/src/accounts.ts, src/channels/plugins/account-helpers.ts)
vincentkoc: Vincent Koc authored the split of the channel secret runtime helper family used by Feishu's current appSecret collection path. (role: shared SecretRef helper contributor; confidence: high; commits: dfb6c9c92079; files: src/secrets/channel-secret-basic-runtime.ts, src/plugin-sdk/channel-secret-basic-runtime.ts, extensions/feishu/src/secret-contract.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.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fcec0fe82b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

WilliamK112 · 2026-06-27T02:36:13Z

@clawsweeper re-review

clawsweeper · 2026-06-27T02:36:15Z

🦞🧹
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.

openclaw-barnacle · 2026-07-12T04:31:48Z

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

openclaw-barnacle · 2026-07-19T04:36:02Z

Closing due to inactivity.
If you believe this PR should be revived, post in #clawtributors on Discord to talk to a maintainer.
That channel is the escape hatch for high-quality PRs that get auto-closed.

fix(feishu): keep default appSecret refs active

fcec0fe

Signed-off-by: WilliamK112 <[email protected]>

WilliamK112 requested a review from a team as a code owner June 26, 2026 04:24

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

chatgpt-codex-connector Bot reviewed Jun 26, 2026

View reviewed changes

Comment thread extensions/feishu/src/secret-contract.ts

clawsweeper Bot mentioned this pull request Jun 26, 2026

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

Closed

1 task

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

🦞 OpenClaw 生态日报 2026-06-26 litang9/big_model_radar#125

Open

clawsweeper Bot mentioned this pull request Jun 26, 2026

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

Merged

1 task

fix(feishu): respect explicit default appSecret overrides

867bc4b

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

openclaw-barnacle Bot closed this Jul 19, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

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

fix(feishu): keep default appSecret refs active#96933
WilliamK112 wants to merge 2 commits into
openclaw:mainfrom
WilliamK112:codex/feishu-default-secretref-active

WilliamK112 commented Jun 26, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 26, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

WilliamK112 commented Jun 27, 2026

Uh oh!

clawsweeper Bot commented Jun 27, 2026 •

edited

Loading

Uh oh!

openclaw-barnacle Bot commented Jul 12, 2026

Uh oh!

openclaw-barnacle Bot commented Jul 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

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

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

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!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

WilliamK112 commented Jun 27, 2026

Uh oh!

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

Uh oh!

Uh oh!

openclaw-barnacle Bot commented Jul 12, 2026

Uh oh!

openclaw-barnacle Bot commented Jul 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

WilliamK112 commented Jun 26, 2026 •

edited

Loading

clawsweeper Bot commented Jun 26, 2026 •

edited

Loading

clawsweeper Bot commented Jun 27, 2026 •

edited

Loading