Skip to content

Conversation

@daniel-lxs
Copy link
Contributor

Testing webhooks

@github-advanced-security
Copy link
Contributor

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

Copy link

@roo-code-dan roo-code-dan bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found some low-priority issues that need attention. See inline comments for details.

- [繁體中文](locales/zh-TW/README.md)
- ...
</details>
</details>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Indentation of the closing is deeper than the opening

. Align for consistency.

Suggested change
</details>
</details>

README.md Outdated
**Enjoy Roo Code!** Whether you keep it on a short leash or let it roam autonomously, we can’t wait to see what you build. If you have questions or feature ideas, drop by our [Reddit community](https://www.reddit.com/r/RooCode/) or [Discord](https://discord.gg/roocode). Happy coding!
**Enjoy Roo Code!** Whether you keep it on a short leash or let it roam autonomously, we can't wait to see what you build. If you have questions or feature ideas, drop by our [Reddit community](https://www.reddit.com/r/RooCode/) or [Discord](https://discord.gg/roocode). Happy coding!
<!-- Test webhook change -->
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Temporary test artifact. Remove before merging to keep README clean.

Suggested change
<!-- Test webhook change -->

Copy link

@roo-code-dan roo-code-dan bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found a few low-priority issues related to temporary testing artifacts and formatting. See inline comments.

README.md Outdated
- [繁體中文](locales/zh-TW/README.md)
- ...
</details>
</details>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Indentation of the closing doesn't match the opening

. Align for consistency.

Suggested change
</details>
</details>

README.md Outdated
- Answer Questions about your codebase
- Automate repetitive tasks
- Utilize MCP Servers
- Test webhooks and automated reviews
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Temporary test artifact in README features. Remove before merging to keep user-facing docs clean.

README.md Outdated
**Enjoy Roo Code!** Whether you keep it on a short leash or let it roam autonomously, we can’t wait to see what you build. If you have questions or feature ideas, drop by our [Reddit community](https://www.reddit.com/r/RooCode/) or [Discord](https://discord.gg/roocode). Happy coding!
**Enjoy Roo Code!** Whether you keep it on a short leash or let it roam autonomously, we can't wait to see what you build. If you have questions or feature ideas, drop by our [Reddit community](https://www.reddit.com/r/RooCode/) or [Discord](https://discord.gg/roocode). Happy coding!
<!-- Test webhook change -->
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Temporary test HTML comment. Remove before merge.

Copy link

@roo-code-dan roo-code-dan bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found some maintainability issues to address. See inline comments for details.

* Check if a port is open on a given host
*/
export async function isPortOpen(host: string, port: number, timeout = 1000): Promise<boolean> {
export async function isPortOpen(host: string, port: number, timeout = 2000): Promise<boolean> {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Timeout default increased to 2000ms. Consider centralizing into a constant (e.g., DEFAULT_DISCOVERY_TIMEOUT_MS) or making it configurable to balance reliability vs responsiveness in different environments.

try {
console.log(`Trying to connect to Chrome at: ${chromeHostUrl}/json/version`)
await axios.get(`${chromeHostUrl}/json/version`, { timeout: 1000 })
await axios.get(`${chromeHostUrl}/json/version`, { timeout: 2000 })
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Hardcoded axios timeout at 2000ms. Mirror the socket timeout via a shared constant/config so both paths remain consistent and tunable, and consider retries with exponential backoff instead of a single long wait.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants