fix: 4 bug fixes for feishu, health, models, and skills#63924
fix: 4 bug fixes for feishu, health, models, and skills#63924hubbyxiao-cpu wants to merge 4 commits into
Conversation
…N Schema The hand-written JSON Schema in feishu channel.ts only listed 11 properties for accounts, while the Zod schema (FeishuAccountConfigSchema) defines 36+ properties via FeishuSharedConfigShape. This caused 'must NOT have additional properties' errors when valid config keys like dmPolicy, allowFrom, etc. were present. Switch to buildChannelConfigSchema(FeishuConfigSchema) to auto-generate the JSON Schema from the Zod schema, keeping them in sync automatically. This matches the pattern used by all other channel plugins (telegram, discord, slack, signal, etc.).
…apshot getHealthSnapshot() used loadConfig() which returns a stale runtimeConfigSnapshot in the gateway process. This caused health checks to show channels as 'not configured' even when the config file had been updated. Now reads directly from file via readBestEffortConfig() with loadConfig() as fallback.
…ProviderId normalizeProviderId() did not normalize volcengine-plan to volcengine or byteplus-plan to byteplus, causing contextWindow lookups to fail for coding-plan providers. The qualified cache key volcengine-plan/ark-code-latest could not match volcengine/ark-code-latest in the model cache. Simplify normalizeProviderIdForAuth to delegate to normalizeProviderId now that the mapping is centralized.
resolveContainedSkillPath() skipped skills when their realpath resolved outside the configured root (e.g. pnpm symlinks, macOS /var -> /private/var). This caused valid skills to be silently dropped. Now falls back to the original (unresolved) path when it is inside the root, logging a warning instead of skipping entirely.
openclaw-barnacle
Bot
added
commands
Greptile SummaryThis PR bundles four bug fixes: (1) Feishu channel config schema auto-generated from Zod schema, (2) health snapshot reads fresh config via Fixes 1, 3, and 4 are correct and low-risk. Fix 2 (health) has a logic error: Confidence Score: 4/5Safe to merge for fixes 1, 3, 4; fix 2 (health) should be revisited — it does not regress behavior but leaves the described bug unfixed. Three of the four fixes are correct and verified against existing patterns. Fix 2 has a P1 logic issue: src/commands/health.ts — the
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const rawCfg = await readBestEffortConfig(); | ||
| const cfg = rawCfg ?? loadConfig(); |
There was a problem hiding this comment.
readBestEffortConfig still returns stale config in gateway
readBestEffortConfig is implemented as:
// src/config/io.ts:1494
export async function readBestEffortConfig(): Promise<OpenClawConfig> {
const snapshot = await readConfigFileSnapshot();
return snapshot.valid ? loadConfig() : snapshot.config;
}When the config file is valid (the normal case), it returns loadConfig(). In the gateway process loadConfig() returns the in-memory runtimeConfigSnapshot, which is exactly the stale value the PR says it wants to avoid. The fix is a no-op for the described problem in the gateway path.
Additionally, readBestEffortConfig returns Promise<OpenClawConfig> (never null or undefined), so the ?? loadConfig() fallback is unreachable dead code and will never be evaluated.
In the unusual edge case where the config file is temporarily malformed, the new code returns snapshot.config (an empty or partially-parsed object) instead of the stale-but-valid runtime snapshot — a potential regression.
To actually read a fresh config in the gateway, something like snapshot.config from readConfigFileSnapshot() (when snapshot.valid) would need to be used directly, bypassing loadConfig().
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/commands/health.ts
Line: 353-354
Comment:
**`readBestEffortConfig` still returns stale config in gateway**
`readBestEffortConfig` is implemented as:
```ts
// src/config/io.ts:1494
export async function readBestEffortConfig(): Promise<OpenClawConfig> {
const snapshot = await readConfigFileSnapshot();
return snapshot.valid ? loadConfig() : snapshot.config;
}
```
When the config file is valid (the normal case), it returns `loadConfig()`. In the gateway process `loadConfig()` returns the in-memory `runtimeConfigSnapshot`, which is exactly the stale value the PR says it wants to avoid. The fix is a no-op for the described problem in the gateway path.
Additionally, `readBestEffortConfig` returns `Promise<OpenClawConfig>` (never `null` or `undefined`), so the `?? loadConfig()` fallback is unreachable dead code and will never be evaluated.
In the unusual edge case where the config file is temporarily malformed, the new code returns `snapshot.config` (an empty or partially-parsed object) instead of the stale-but-valid runtime snapshot — a potential regression.
To actually read a fresh config in the gateway, something like `snapshot.config` from `readConfigFileSnapshot()` (when `snapshot.valid`) would need to be used directly, bypassing `loadConfig()`.
How can I resolve this? If you propose a fix, please make it concise.
|
Thanks for the context here. I did a careful shell check against current This stale, conflicting PR should close because current main already has the useful Feishu config-schema generation, provider-plan auth/catalog handling, runtime health config path, and explicit trusted symlink support; the remaining branch diff is obsolete and riskier than current main. So I’m closing this older PR as already covered on Review detailsBest possible solution: Keep the current main implementations and close this stale branch; any remaining reproducible bug should come back as a narrow PR against the current files with real behavior proof. Do we have a high-confidence way to reproduce the issue? No high-confidence current-main failing reproduction was established. Source inspection shows current main covers the useful behavior with newer paths, while the stale PR branch itself remains conflicting and lacks real behavior proof. Is this the best way to solve the issue? No. Current main is the better solution shape: Feishu schema generation is in the plugin, plan-provider credentials use manifest aliases, health uses runtime snapshots, and symlinked skills require explicit trusted targets. Security review: Security review needs attention: The stale branch includes a concrete security-boundary concern in skill symlink loading, which current main handles more narrowly with explicit trusted target roots.
AGENTS.md: found and applied where relevant. What I checked:
Likely related people:
Codex review notes: model internal, reasoning high; reviewed against c78f9376d929; fix evidence: commit 7fb0d45b48e1, main fix timestamp 2026-06-14T10:49:26+08:00. |
clawsweeper
Bot
added
rating: 🧂 unranked krab
openclaw-barnacle
Bot
added
the
triage: needs-real-behavior-proof
|
ClawSweeper PR egg 🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat. Where did the egg go?
|
|
This pull request has been automatically marked as stale due to inactivity. |
clawsweeper
Bot
added
rating: 🌊 off-meta tidepool
clawsweeper
Bot
added
rating: 🧂 unranked krab
|
ClawSweeper applied the proposed close for this PR.
|
1 participant
Summary
4 bug fixes addressing stability issues in OpenClaw 2026.4.9.
1. fix(feishu): use buildChannelConfigSchema instead of hand-written JSON Schema
must NOT have additional propertieserrors when valid config keys likedmPolicy,allowFrom, etc. were present.buildChannelConfigSchema(FeishuConfigSchema)to auto-generate the JSON Schema from the Zod schema, keeping them in sync. This matches the pattern used by all other channel plugins.2. fix(health): use readBestEffortConfig for fresh config in getHealthSnapshot
getHealthSnapshot()usedloadConfig()which returns a staleruntimeConfigSnapshotin the gateway process. This caused health checks to show channels as "not configured" even when the config file had been updated.await readBestEffortConfig()withloadConfig()as fallback.3. fix(models): normalize volcengine-plan and byteplus-plan in normalizeProviderId
normalizeProviderId()did not normalizevolcengine-plantovolcengineorbyteplus-plantobyteplus, causingcontextWindowlookups to fail for coding-plan providers.normalizeProviderId()and simplifynormalizeProviderIdForAuth()to delegate to it.4. fix(skills): use original path for symlinked skills instead of skipping
resolveContainedSkillPath()skipped skills when their realpath resolved outside the configured root (e.g. pnpm symlinks, macOS/var->/private/var).Test plan
pnpm buildpasses (only pre-existingglobal-singleton.tserror)normalizeProviderIdtests passnormalizeProviderIdForAuthtests passcontext.lookuptests passir.blockquote-spacingtests pass