fix(discord): clarify startup readiness log#51425
fix(discord): clarify startup readiness log#51425scoootscooob merged 2 commits intoopenclaw:mainfrom
Conversation
openclaw-barnacle
bot
added
channel: discord
Greptile SummaryThis PR fixes a misleading Discord startup log that previously claimed the bot was "logged in" before the gateway was actually ready. It extracts a small
Confidence Score: 5/5
Prompt To Fix All With AIThis is a comment left during a code review.
Path: extensions/discord/src/monitor/startup-status.test.ts
Line: 4-30
Comment:
**Missing test case: `gatewayReady: true` without `botIdentity`**
The test suite covers three of the four meaningful input combinations but is missing the case where the gateway is ready but `botIdentity` is absent. Exercising that path ensures the `identitySuffix` branch in the ready state is also verified, keeping coverage symmetric with the non-ready cases.
```suggestion
it("reports logged-in status only after the gateway is ready", () => {
expect(
formatDiscordStartupStatusMessage({
gatewayReady: true,
botIdentity: "bot-1 (Molty)",
}),
).toBe("logged in to discord as bot-1 (Molty)");
});
it("reports logged-in status without identity suffix when identity is absent", () => {
expect(
formatDiscordStartupStatusMessage({
gatewayReady: true,
}),
).toBe("logged in to discord");
});
```
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: "Discord: clarify sta..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| describe("formatDiscordStartupStatusMessage", () => { | ||
| it("reports logged-in status only after the gateway is ready", () => { | ||
| expect( | ||
| formatDiscordStartupStatusMessage({ | ||
| gatewayReady: true, | ||
| botIdentity: "bot-1 (Molty)", | ||
| }), | ||
| ).toBe("logged in to discord as bot-1 (Molty)"); | ||
| }); | ||
|
|
||
| it("reports client initialization while gateway readiness is still pending", () => { | ||
| expect( | ||
| formatDiscordStartupStatusMessage({ | ||
| gatewayReady: false, | ||
| botIdentity: "bot-1 (Molty)", | ||
| }), | ||
| ).toBe("discord client initialized as bot-1 (Molty); awaiting gateway readiness"); | ||
| }); | ||
|
|
||
| it("handles missing identity without awkward punctuation", () => { | ||
| expect( | ||
| formatDiscordStartupStatusMessage({ | ||
| gatewayReady: false, | ||
| }), | ||
| ).toBe("discord client initialized; awaiting gateway readiness"); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
gatewayReady: true without botIdentity
The test suite covers three of the four meaningful input combinations but is missing the case where the gateway is ready but botIdentity is absent. Exercising that path ensures the identitySuffix branch in the ready state is also verified, keeping coverage symmetric with the non-ready cases.
| describe("formatDiscordStartupStatusMessage", () => { | |
| it("reports logged-in status only after the gateway is ready", () => { | |
| expect( | |
| formatDiscordStartupStatusMessage({ | |
| gatewayReady: true, | |
| botIdentity: "bot-1 (Molty)", | |
| }), | |
| ).toBe("logged in to discord as bot-1 (Molty)"); | |
| }); | |
| it("reports client initialization while gateway readiness is still pending", () => { | |
| expect( | |
| formatDiscordStartupStatusMessage({ | |
| gatewayReady: false, | |
| botIdentity: "bot-1 (Molty)", | |
| }), | |
| ).toBe("discord client initialized as bot-1 (Molty); awaiting gateway readiness"); | |
| }); | |
| it("handles missing identity without awkward punctuation", () => { | |
| expect( | |
| formatDiscordStartupStatusMessage({ | |
| gatewayReady: false, | |
| }), | |
| ).toBe("discord client initialized; awaiting gateway readiness"); | |
| }); | |
| }); | |
| it("reports logged-in status only after the gateway is ready", () => { | |
| expect( | |
| formatDiscordStartupStatusMessage({ | |
| gatewayReady: true, | |
| botIdentity: "bot-1 (Molty)", | |
| }), | |
| ).toBe("logged in to discord as bot-1 (Molty)"); | |
| }); | |
| it("reports logged-in status without identity suffix when identity is absent", () => { | |
| expect( | |
| formatDiscordStartupStatusMessage({ | |
| gatewayReady: true, | |
| }), | |
| ).toBe("logged in to discord"); | |
| }); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/discord/src/monitor/startup-status.test.ts
Line: 4-30
Comment:
**Missing test case: `gatewayReady: true` without `botIdentity`**
The test suite covers three of the four meaningful input combinations but is missing the case where the gateway is ready but `botIdentity` is absent. Exercising that path ensures the `identitySuffix` branch in the ready state is also verified, keeping coverage symmetric with the non-ready cases.
```suggestion
it("reports logged-in status only after the gateway is ready", () => {
expect(
formatDiscordStartupStatusMessage({
gatewayReady: true,
botIdentity: "bot-1 (Molty)",
}),
).toBe("logged in to discord as bot-1 (Molty)");
});
it("reports logged-in status without identity suffix when identity is absent", () => {
expect(
formatDiscordStartupStatusMessage({
gatewayReady: true,
}),
).toBe("logged in to discord");
});
```
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: f7da963fa8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| runtime.log?.( | ||
| formatDiscordStartupStatusMessage({ | ||
| gatewayReady: lifecycleGateway?.isConnected === true, | ||
| botIdentity: botIdentity || undefined, | ||
| }), |
There was a problem hiding this comment.
When lifecycleGateway.isConnected is still false here, this path now logs discord client initialized ...; awaiting gateway readiness and never logs the old logged in to discord ... message later if READY arrives a few seconds afterwards. I checked extensions/discord/src/monitor/provider.lifecycle.ts, and the later READY path only pushes statusSink updates; it does not emit any follow-up success log. On slow-but-healthy startups, operators therefore lose the only positive Discord-ready marker they are still told to look for in docs/install/fly.md:133-138, so log-based startup verification regresses on exactly the scenario this change is trying to clarify.
Useful? React with 👍 / 👎.
|
Merged via squash.
Thanks @scoootscooob! |
🔒 Aisle Security AnalysisWe found 1 potential security issue(s) in this PR:
1. 🔵 Discord startup log message allows CRLF/terminal escape injection via unsanitized bot identity
DescriptionThe Discord monitor formats a startup status log line by directly concatenating If
Vulnerable code: const identitySuffix = params.botIdentity ? ` as ${params.botIdentity}` : "";
return `logged in to discord${identitySuffix}`;Data flow (as implemented in
RecommendationSanitize/neutralize untrusted identity text before embedding it into log/terminal output. For example, strip ANSI escapes and remove/escape control characters before concatenation: function sanitizeForLog(input: string): string {
// Remove ANSI escape sequences
const noAnsi = input.replace(/\x1b\[[0-9;]*[A-Za-z]/g, "");
// Escape common whitespace controls and drop remaining control chars
return noAnsi
.replace(/\r/g, "\\r")
.replace(/\n/g, "\\n")
.replace(/\t/g, "\\t")
.replace(/[\x00-\x1F\x7F-\x9F]/g, "");
}
export function formatDiscordStartupStatusMessage(params: {
gatewayReady: boolean;
botIdentity?: string;
}): string {
const safeIdentity = params.botIdentity ? sanitizeForLog(params.botIdentity) : "";
const identitySuffix = safeIdentity ? ` as ${safeIdentity}` : "";
return params.gatewayReady
? `logged in to discord${identitySuffix}`
: `discord client initialized${identitySuffix}; awaiting gateway readiness`;
}Alternatively, expose and reuse the repo's existing terminal-safe sanitizer ( Analyzed PR: #51425 at commit Last updated on: 2026-03-21T05:15:14Z |
Merged via squash. Prepared head SHA: 390986d Co-authored-by: scoootscooob <[email protected]> Co-authored-by: scoootscooob <[email protected]> Reviewed-by: @scoootscooob
Merged via squash. Prepared head SHA: 390986d Co-authored-by: scoootscooob <[email protected]> Co-authored-by: scoootscooob <[email protected]> Reviewed-by: @scoootscooob
Merged via squash. Prepared head SHA: 390986d Co-authored-by: scoootscooob <[email protected]> Co-authored-by: scoootscooob <[email protected]> Reviewed-by: @scoootscooob
Merged via squash. Prepared head SHA: 390986d Co-authored-by: scoootscooob <[email protected]> Co-authored-by: scoootscooob <[email protected]> Reviewed-by: @scoootscooob
1 participant
Summary
Describe the problem and fix in 2-5 bullets:
logged in to discordbefore the gateway was actually READY.discord client initialized ...; awaiting gateway readinessorlogged in to discord ...based on actual gateway readiness.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
User-visible / Behavior Changes
discord client initialized ...; awaiting gateway readiness.Security Impact (required)
No)No)No)No)No)Yes, explain risk + mitigation:Repro + Verification
Environment
Steps
Expected
discord client initialized ...; awaiting gateway readinesslogged in to discord ...Actual
logged in to discord...even when the gateway was not yet ready.Evidence
Attach at least one:
Human Verification (required)
What you personally verified (not just CI), and how:
pnpm test -- extensions/discord/src/monitor/startup-status.test.tspnpm test -- extensions/discord/src/monitor/provider.lifecycle.test.tsnode --import tsxfor both helper outputsReview Conversations
If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers.
Compatibility / Migration
Yes)No)No)Failure Recovery (if this breaks)
extensions/discord/src/monitor/provider.tsextensions/discord/src/monitor/startup-status.tsRisks and Mitigations
List only real risks for this PR. Add/remove entries as needed. If none, write
None.