fix(config): make status command resilient to unresolved SecretRefs#33213
fix(config): make status command resilient to unresolved SecretRefs#33213Sid-Qin wants to merge 1 commit intoopenclaw:mainfrom
Conversation
Greptile SummaryThis PR wraps the per-account resolution loop in Key concern: The catch block hardcodes Confidence Score: 2/5
Last reviewed commit: 6656ee0 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
src/commands/status-all/channels.ts
Outdated
| } catch { | ||
| const degraded: ChannelAccountSnapshot = { | ||
| accountId, | ||
| enabled: false, | ||
| configured: false, | ||
| lastError: "unresolved SecretRef (gateway unreachable)", | ||
| }; | ||
| accounts.push({ | ||
| accountId, | ||
| account: {}, | ||
| enabled: false, | ||
| configured: false, | ||
| snapshot: degraded, | ||
| }); | ||
| } |
There was a problem hiding this comment.
The catch block unconditionally sets lastError to the hardcoded string "unresolved SecretRef (gateway unreachable)" regardless of what actually threw. This masks unrelated runtime errors inside the try block (e.g., null-pointer in resolveChannelAccountEnabled, timeout in resolveChannelAccountConfigured, unexpected shape in buildChannelAccountSnapshot) and reports them to operators as a SecretRef problem, making real bugs invisible.
Preserve the actual error message instead:
| } catch { | |
| const degraded: ChannelAccountSnapshot = { | |
| accountId, | |
| enabled: false, | |
| configured: false, | |
| lastError: "unresolved SecretRef (gateway unreachable)", | |
| }; | |
| accounts.push({ | |
| accountId, | |
| account: {}, | |
| enabled: false, | |
| configured: false, | |
| snapshot: degraded, | |
| }); | |
| } | |
| } catch (e) { | |
| const errorMessage = e instanceof Error ? e.message : String(e); | |
| const degraded: ChannelAccountSnapshot = { | |
| accountId, | |
| enabled: false, | |
| configured: false, | |
| lastError: errorMessage, | |
| }; | |
| accounts.push({ | |
| accountId, | |
| account: {}, | |
| enabled: false, | |
| configured: false, | |
| snapshot: degraded, | |
| }); | |
| } |
Since lastError will now carry the actual message, the hasDegradedAccount check on line 389 can also be simplified to accounts.some((a) => a.snapshot.lastError != null) rather than string-matching the hardcoded prefix.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/commands/status-all/channels.ts
Line: 341-355
Comment:
The `catch` block unconditionally sets `lastError` to the hardcoded string `"unresolved SecretRef (gateway unreachable)"` regardless of what actually threw. This masks unrelated runtime errors inside the `try` block (e.g., null-pointer in `resolveChannelAccountEnabled`, timeout in `resolveChannelAccountConfigured`, unexpected shape in `buildChannelAccountSnapshot`) and reports them to operators as a SecretRef problem, making real bugs invisible.
Preserve the actual error message instead:
```suggestion
} catch (e) {
const errorMessage = e instanceof Error ? e.message : String(e);
const degraded: ChannelAccountSnapshot = {
accountId,
enabled: false,
configured: false,
lastError: errorMessage,
};
accounts.push({
accountId,
account: {},
enabled: false,
configured: false,
snapshot: degraded,
});
}
```
Since `lastError` will now carry the actual message, the `hasDegradedAccount` check on line 389 can also be simplified to `accounts.some((a) => a.snapshot.lastError != null)` rather than string-matching the hardcoded prefix.
How can I resolve this? If you propose a fix, please make it concise.![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6656ee0e94
ℹ️ 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".
| if (!anyEnabled) { | ||
| return "off"; | ||
| return hasDegradedAccount ? "warn" : "off"; | ||
| } |
There was a problem hiding this comment.
This branch only turns hasDegradedAccount into a warning when all accounts are disabled, so a plugin with mixed outcomes (one account throws during resolveAccount, another account resolves) can still report ok. Because degraded accounts are forced to enabled: false, they are excluded from enabledAccounts and token checks, and plugins that do not implement collectStatusIssues (for example Slack’s status block in extensions/slack/src/channel.ts:391-435) will not emit any warning path; the unresolved SecretRef is silently hidden instead of being surfaced in status output.
Useful? React with 👍 / 👎.
openclaw-barnacle
bot
added
commands
Replace the hardcoded "unresolved SecretRef" error message with the actual caught error message so operators can distinguish between SecretRef failures and other account resolution errors. Closes openclaw#33070
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1092b3dd12
ℹ️ 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".
| if (!anyEnabled) { | ||
| return "off"; | ||
| return hasDegradedAccount ? "warn" : "off"; | ||
| } |
There was a problem hiding this comment.
This logic only considers hasDegradedAccount when !anyEnabled, so a plugin with mixed account outcomes (one account throws during resolveAccount, another account resolves and is enabled) can still report ok and hide the failed account from status output. In that scenario, channels without a collectStatusIssues implementation (such as Slack) have no alternate warning path, so unresolved SecretRef/account-resolution failures are silently masked. Fresh evidence: src/commands/status-all/channels.test.ts currently tests mixed plugin outcomes, but it does not cover mixed account outcomes within the same plugin, leaving this branch unverified.
Useful? React with 👍 / 👎.
2 participants
Summary
openclaw statuscrashed whenchannels.telegram.botTokenused a SecretRef that couldn't be resolved (e.g., gateway unreachable).buildChannelsTablewith try/catch. Unresolved accounts produce a degraded row with a warning instead of crashing the entire command.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
User-visible / Behavior Changes
openclaw statusshows degraded channel state instead of crashing on unresolved SecretRefs.Security Impact (required)
NoNoNoNoNoRepro + Verification
Environment
Steps
botTokenas SecretRef with file provideropenclaw statuswhen gateway is unreachableExpected
Actual
Evidence
3 tests verify: degraded row on resolution failure, isolation between plugins, degraded accounts marked as disabled.
Human Verification (required)
Compatibility / Migration
YesNoNoFailure Recovery (if this breaks)
src/commands/status-all/channels.tsRisks and Mitigations