-
-
Notifications
You must be signed in to change notification settings - Fork 6
[WIP] Support multiple bots per instance #17
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
Add new types for supporting multiple bots per instance: - BotInfo: Interface for bot identity information - BotProfile<TContextData>: Profile configuration for creating bots - Instance<TContextData>: Server instance interface that can host multiple bots - BotDispatcher<TContextData>: Function type for dynamic bot resolution - CreateInstanceOptions<TContextData>: Options for creating an instance - createInstance(): Factory function (placeholder implementation) 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add createScopedPrefixes() and createLegacyPrefixes() functions - Update SqliteRepository to support botId scoping - Add bot_id column to all SQLite tables with auto-migration - All queries now filter by bot_id for data isolation 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add BotInfo import to session.ts - Add botInfo property to Session interface with docs - Implement botInfo getter in SessionImpl - Update text.test.ts mock to include botInfo 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Implement InstanceImpl class that manages multiple bots sharing a single Federation instance. The class supports: - Static bot creation with fixed identifier and profile - Dynamic bot creation with dispatcher function for on-demand resolution - Shared dispatchers that route to the correct bot based on identifier - Inbox routing to delegate activities to the appropriate bot handlers - Repository scanning for object ownership lookup 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add options to BotImpl for integration with Instance: - federation: Accept an existing Federation instead of creating new one - skipInitialize: Skip dispatcher/listener registration when managed by Instance Update InstanceImpl to use these options when creating bots, ensuring all bots share the same Federation instance. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Connect createInstance() to InstanceImpl, enabling multi-bot instance creation. The function returns a wrapper object for Deno serve compatibility. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Document the new multi-bot features in CHANGES.md: - createInstance() function and Instance interface - Bot profile and dispatcher types - Session.botInfo property - Repository scoping utilities - SQLite bot_id scoping with auto-migration 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary of ChangesHello @moreal, 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 introduces a significant architectural enhancement, enabling the hosting of multiple ActivityPub bots within a single application instance. It provides new APIs to create and manage these bots, ensuring they can share common infrastructure while maintaining isolated data storage and handling requests appropriately. This change aims to simplify deployment and resource management for developers running several bots. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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
|
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
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 significant changes to support hosting multiple bots on a single instance. A new Instance abstraction is added to manage shared infrastructure, and bots can be created statically or dynamically. The SQLite repository is updated with bot_id columns and migrations for data isolation. The KvRepository is also updated to support per-bot data scoping via prefixes.
The changes are well-structured and the new APIs are clearly documented. I've found a couple of minor issues: an empty catch block in the SQLite migration logic that could hide errors, and an unused repository option in CreateInstanceOptions. Overall, this is a great step towards multi-tenancy for BotKit.
| } catch { | ||
| // Table might have been just created with bot_id, ignore | ||
| } |
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 empty catch block here is concerning as it can swallow unexpected errors during the database migration, making debugging difficult. While the comment suggests this is to ignore errors on newly created tables, it's safer to log any caught errors. This ensures that if an unexpected issue occurs (e.g., a syntax error, permissions issue), it won't fail silently.
} catch (error) {
// Swallowing errors can hide potential issues. It's better to log them,
// even if they are expected in some cases. This helps with debugging.
logger.warn(
`Failed to check for 'bot_id' column on table '${tableName}'. This might be okay if the table was just created.`,
{ error },
);
}| /** | ||
| * The underlying repository to use for storing data. If omitted, | ||
| * {@link KvRepository} will be used with bot-scoped prefixes. | ||
| */ | ||
| readonly repository?: Repository; |
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 repository option in CreateInstanceOptions seems to be unused in the InstanceImpl implementation. The implementation always creates a new KvRepository for each bot with scoped prefixes.
This option is likely a leftover from CreateBotOptions and is misleading for createInstance. For multi-bot support, a single Repository instance is not suitable. Perhaps a repository factory function like (botId: string) => Repository would be more appropriate if custom repository logic is needed per bot.
I'd recommend either removing this option or implementing it as a factory.
⚒️ This pull request isn't ready to be reviewed.