-
Notifications
You must be signed in to change notification settings - Fork 0
Fix 5 runtime bugs found during debug pass #26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
1. Redirect console.log to stderr to prevent conflict with MCP StdioServerTransport which hijacks stdout for protocol messages, corrupting both agent output and MCP communication. 2. Wire up the unused lastMentionId field to pass since_id on the mentions API call, avoiding redundant refetches of the same tweets every polling cycle. 3. Add null guard in fetchThread before accessing response.data to prevent null dereference crashes when the API returns unexpected data. 4. Copy the tweets array before sorting in parseThread to avoid mutating the caller's data via Array.sort() in-place side effect. 5. Cap processedMentions Set at 10,000 entries and prune oldest to prevent unbounded memory growth in long-running deployments. https://claude.ai/code/session_015bNhpqRa1sDd4NPLPuZqxV
Summary of ChangesHello @groupthinking, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses several runtime stability and performance issues identified during a debugging pass. The changes focus on robust error handling for API responses, optimizing data fetching to reduce redundant calls, preventing unintended data mutations, and implementing memory management strategies to ensure the application remains stable and efficient over time. These fixes collectively enhance the agent's reliability and resource utilization. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces several important runtime bug fixes that improve the agent's stability, efficiency, and correctness. The changes address potential crashes from null data, prevent a memory leak by capping processed mentions, avoid redundant API calls by using since_id, and prevent data mutation side effects by correctly copying an array before sorting. My review includes a couple of suggestions to further improve code robustness and maintainability.
| let mentionsUrl = `https://api.twitter.com/2/users/${userId}/mentions?max_results=10&expansions=author_id&tweet.fields=created_at,conversation_id,in_reply_to_user_id,referenced_tweets`; | ||
| if (this.lastMentionId) { | ||
| mentionsUrl += `&since_id=${this.lastMentionId}`; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Building URLs via string concatenation can be fragile and hard to read. Using URLSearchParams provides a more robust and maintainable way to construct URLs with query parameters, as it handles proper encoding and separation of parameters automatically.
const params = new URLSearchParams({
max_results: '10',
expansions: 'author_id',
'tweet.fields': 'created_at,conversation_id,in_reply_to_user_id,referenced_tweets',
});
if (this.lastMentionId) {
params.set('since_id', this.lastMentionId);
}
const mentionsUrl = `https://api.twitter.com/2/users/${userId}/mentions?${params.toString()}`;There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR addresses several runtime/operational issues in MyXstack’s polling + MCP runtime by preventing stdout corruption, reducing redundant mention refetching, hardening thread parsing, and bounding in-memory state growth.
Changes:
- Redirects
console.logoutput away from stdout to avoid MCP stdio protocol corruption. - Adds
since_idpagination vialastMentionIdto reduce redundant mention refetches. - Hardens thread parsing (null-guard) and avoids in-place mutation when sorting thread tweets; caps processed mention tracking to prevent unbounded memory growth.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/services/xapi.ts | Adds since_id support, improves thread response guarding, and avoids mutating tweet arrays during sort. |
| src/services/agent.ts | Caps processedMentions to 10,000 entries and prunes oldest IDs. |
| src/index.ts | Redirects console.log to stderr to avoid MCP stdout interference. |
| if (!response || !response.data) { | ||
| console.warn('Invalid response from X API (thread)'); | ||
| return null; | ||
| } | ||
|
|
||
| return this.parseThread(response.data); |
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fetchThread() now only checks response.data for truthiness, but parseThread() expects an array and will still throw if response.data is a non-array object (which can happen with unexpected API shapes). Consider guarding with Array.isArray(response.data) (and returning null otherwise) before calling parseThread().
| // Redirect console.log to stderr so it doesn't conflict with | ||
| // MCP StdioServerTransport which uses stdout for protocol messages | ||
| const origLog = console.log; | ||
| console.log = (...args: any[]) => console.error(...args); | ||
|
|
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
origLog is assigned but never used. Either remove it to avoid dead code, or use it to restore console.log during shutdown/after MCP server init (depending on the intended lifecycle).
src/services/agent.ts
Outdated
| const excess = this.processedMentions.size - AutonomousAgent.MAX_PROCESSED_MENTIONS; | ||
| const iter = this.processedMentions.values(); | ||
| for (let i = 0; i < excess; i++) { | ||
| this.processedMentions.delete(iter.next().value as string); |
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pruning loop deletes iter.next().value as string, which relies on a type assertion and would silently pass undefined if iteration were ever exhausted. Prefer reading { value, done } from iter.next() and breaking if done, which keeps the code type-safe and avoids the assertion.
| this.processedMentions.delete(iter.next().value as string); | |
| const { value, done } = iter.next(); | |
| if (done) { | |
| break; | |
| } | |
| this.processedMentions.delete(value); |
src/index.ts
Outdated
| // Redirect console.log to stderr so it doesn't conflict with | ||
| // MCP StdioServerTransport which uses stdout for protocol messages | ||
| const origLog = console.log; | ||
| console.log = (...args: any[]) => console.error(...args); |
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unexpected any. Specify a different type.
| console.log = (...args: any[]) => console.error(...args); | |
| console.log = (...args: unknown[]) => console.error(...args); |
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: Copilot <[email protected]>
|
@copilot open a new pull request to apply changes based on the comments in this thread |
|
@groupthinking I've opened a new pull request, #27, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
Prompt Yourself: Solution for failing job 62936698644 in MyXstack repo (PR #26) Problem:
Solution Path:
Actionable Fix: permissions:
pull-requests: write
contents: readExample minimal edit: name: PR Checks
on:
pull_request:
types: [opened, synchronize, reopened]
permissions:
pull-requests: write
contents: read
jobs:
# ... (rest of your job definitions)If you're using the default GITHUB_TOKEN, this permissions block grants write access for posting comments or status updates on PRs. If PRs come from forks, also ensure you’re not using secrets that skip steps on forked runs. References:
Prompt Yourself Actions:
Summary: Set pull-requests: write in workflow permissions. Re-run the workflow. This should resolve the Resource not accessible by integration error. |
Co-authored-by: groupthinking <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| console.warn('Invalid response from X API (thread)'); | ||
| return null; | ||
| } | ||
|
|
Copilot
AI
Feb 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fetchThread returns early when response.data is falsy, but it still passes through cases where response.data is present but not an array (e.g., an object from an unexpected API shape). In that case parseThread() will throw when it reads tweets.length. Please validate response.data with Array.isArray(...) (and handle the non-array case) before calling parseThread.
| if (!Array.isArray(response.data)) { | |
| console.warn('Unexpected response shape from X API (thread): data is not an array'); | |
| return null; | |
| } |
| // Prune oldest entries to prevent unbounded memory growth | ||
| if (this.processedMentions.size > AutonomousAgent.MAX_PROCESSED_MENTIONS) { | ||
| const excess = this.processedMentions.size - AutonomousAgent.MAX_PROCESSED_MENTIONS; | ||
| const iter = this.processedMentions.values(); | ||
| for (let i = 0; i < excess; i++) { | ||
| this.processedMentions.delete(iter.next().value as string); | ||
| } |
Copilot
AI
Feb 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pruning logic deletes the first values in the Set, which are the earliest inserted IDs, not necessarily the oldest mention by time. Since mentions appear to be processed in the API response order (and fetchMentions treats index 0 as “newest”), this can end up pruning newer mentions first. Either process mentions oldest→newest before inserting into processedMentions, or change the wording/approach so the eviction policy matches the intended behavior.
| private parseThread(tweets: { created_at: string; [key: string]: unknown }[]): XThread | null { | ||
| if (tweets.length === 0) return null; |
Copilot
AI
Feb 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parseThread’s parameter type still uses an index signature with any, which weakens type safety under strict mode and makes it easier for unexpected shapes to slip through (especially since this method assumes an array-like input). Consider using unknown instead of any for the extra fields (or define a minimal tweet shape type) to avoid accidentally relying on untyped properties.
|
@copilot open a new pull request to apply changes based on the comments in this thread |
|
@groupthinking I've opened a new pull request, #28, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@copilot open a new pull request to apply changes based on the comments in this thread |
|
@groupthinking I've opened a new pull request, #29, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@copilot open a new pull request to apply changes based on the comments in this thread |
|
@groupthinking I've opened a new pull request, #30, to work on those changes. Once the pull request is ready, I'll request review from you. |
…y.isArray guard, safe iterators, chronological processing Co-authored-by: groupthinking <[email protected]>
Addresses unresolved review comments from PRs #6, #13, #25, #26: - src/index.ts: Add console.log→stderr redirect with unknown[] type - src/services/xapi.ts: Use URLSearchParams, add Array.isArray guard, add since_id pagination, use unknown type, avoid mutating sort - src/services/agent.ts: Add bounded memory pruning with safe iterator, process mentions oldest-first for chronological ordering - .github/workflows: Add permissions blocks, check labels exist before adding, use pull_request_target for fork PR support - README.md: Add Contributing section with absolute paths - Add CONTRIBUTING.md, PR_TITLE_GUIDE.md, pull_request_template.md Co-authored-by: groupthinking <[email protected]>
PR #26 fixes (with review feedback incorporated): - Redirect console.log to stderr for MCP stdout (unknown[] type) - Use URLSearchParams for mentions URL + wire up since_id - Add Array.isArray() guard in fetchThread - Copy tweets array before sorting + unknown type in parseThread - Cap processedMentions with safe {value, done} iterator - Process mentions oldest-first for chronological pruning PR #6 fix (with review feedback): - Add mentionPostId param to analyzeAndDecide for reply targeting - Replace any type with specific type in Grok response parsing Co-authored-by: groupthinking <[email protected]>
… safety, Array.isArray guards, Set pruning Co-authored-by: groupthinking <[email protected]>
…rams, Array.isArray guards, safe iterators, chronological pruning, README fixes Co-authored-by: groupthinking <[email protected]>
…RLSearchParams, Array.isArray guard, safe iterators, reply targeting Co-authored-by: groupthinking <[email protected]>
This commit applies all actionable review feedback from multiple pending PRs: ## PR #26 - Console.log redirect and type improvements - Added console.log redirect to stderr in main() to prevent MCP protocol conflicts - Used unknown[] instead of any[] for type safety - Refactored fetchMentions() to use URLSearchParams for cleaner URL construction - Added lastMentionId tracking for efficient pagination - Added Array.isArray guard in fetchThread() for safer data handling - Changed parseThread() to use proper typed parameters and copy array before sort ## PR #6 - Pass mention post ID for accurate replies - Added mentionPostId parameter to analyzeAndDecide() and simulateAnalysis() - Updated all callers to pass mention.post.id - Fixed response.json() type with proper type assertion - Fixed parseGrokResponse to use mentionPostId instead of root_post.id - Updated examples.ts to pass mention.post.id ## PR #13 - GitHub Actions workflows with review feedback - Created .github/workflows/auto-label.yml for PR auto-labeling - Created .github/workflows/pr-checks.yml for CI builds - Created .github/workflows/issue-triage.yml for automatic issue triage - Created .github/labeler.yml configuration for auto-labeling - All workflows use secure permissions and latest action versions ## Additional improvements from review feedback - Added MAX_PROCESSED_MENTIONS constant (10000) to prevent unbounded memory growth - Reversed mention processing order (oldest-first) for better chronological handling - Added safe pruning logic using iterator pattern to cap Set size - Improved type safety throughout with unknown[] and proper type assertions All changes compile successfully with TypeScript. No tests to run.
Co-authored-by: groupthinking <[email protected]>
- xapi.ts: Use URLSearchParams for mentions URL with since_id support - xapi.ts: Add Array.isArray() guard before parseThread() in fetchThread - xapi.ts: Add Array.isArray() guard in searchTweets - xapi.ts: Change any to unknown in parseThread parameter type - agent.ts: Process mentions oldest-first and add safe iterator pruning - grok.ts: Replace any with typed response, fix as any cast - auto-label.yml: Add permissions: pull-requests: write - issue-triage.yml: Add permissions: issues: write - pr-checks.yml: Add permissions: pull-requests: write, fix syntax check - README.md: Fix double space and improve wording Co-authored-by: groupthinking <[email protected]>
- Add Array.isArray guard in fetchThread (xapi.ts) - Use URLSearchParams for mentions URL construction (xapi.ts) - Add since_id pagination tracking for mentions (xapi.ts) - Add console.log→stderr redirect for MCP transport (index.ts) - Add memory pruning with safe iterator for processedMentions (agent.ts) - Process mentions oldest-first for chronological pruning (agent.ts) - Replace any with unknown/proper types (xapi.ts, grok.ts) - Fix CODEOWNERS invalid usernames to @groupthinking - Add permissions blocks to all workflow files - Add label existence check in issue-triage workflow Co-authored-by: groupthinking <[email protected]>
- Add console.log→stderr redirect with unknown[] type (PR #26) - Use URLSearchParams for mentions URL + since_id pagination (PR #26) - Add Array.isArray() guard in fetchThread() (PR #26) - Add Set pruning with safe { value, done } iterator (PR #26) - Process mentions oldest-first for chronological pruning (PR #26) - Use [key: string]: unknown in parseThread, spread sort (PR #26) - Add mentionPostId param for correct reply targeting (PR #6) - Fix any type in Grok response parsing (PR #6) - Fix README broken list numbering (PR #4) Co-authored-by: groupthinking <[email protected]>
Address code review feedback: strengthen type guards and remove unsafe assertions
Redirect console.log to stderr to prevent conflict with MCP StdioServerTransport which hijacks stdout for protocol messages, corrupting both agent output and MCP communication.
Wire up the unused lastMentionId field to pass since_id on the mentions API call, avoiding redundant refetches of the same tweets every polling cycle.
Add null guard in fetchThread before accessing response.data to prevent null dereference crashes when the API returns unexpected data.
Copy the tweets array before sorting in parseThread to avoid mutating the caller's data via Array.sort() in-place side effect.
Cap processedMentions Set at 10,000 entries and prune oldest to prevent unbounded memory growth in long-running deployments.
https://claude.ai/code/session_015bNhpqRa1sDd4NPLPuZqxV