feat(cli): add --actor-name and --actor-summary options to 'fedify inbox' command #285
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @crohasang, 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 new command-line interface options for the fedify inbox command, enabling users to customize the ephemeral ActivityPub actor's display name and summary. This enhancement provides greater flexibility for testing and demonstration scenarios by allowing dynamic configuration of the temporary actor's identity.
Highlights
- CLI Customization: The
fedify inboxcommand now supports two new options:--actor-nameand--actor-summary. These allow users to customize the display name and summary of the ephemeral ActivityPub actor created by the command. - Dynamic Actor Properties: The
Applicationobject, which represents the ephemeral actor, now uses the values provided by the new CLI options for itsnameandsummaryproperties, replacing previously hardcoded strings. Sensible default values are provided if the options are not specified. - Code Structure: A new
actorOptionsglobal object has been introduced incli/inbox.tsxto store and manage the custom actor name and summary values, which are then passed to theApplicationconstructor.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and 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 to provide feedback.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
crohasang
changed the title
github-actions
bot
added
component/cli
crohasang
changed the title
There was a problem hiding this comment.
Code Review
This pull request introduces the --actor-name and --actor-summary options to the fedify inbox command, allowing users to customize the actor's display name and summary. The changes are well-structured and include appropriate default values and documentation.
cli/inbox.tsx
Outdated
| actorOptions.name = options.actorName; | ||
| actorOptions.summary = options.actorSummary; |
There was a problem hiding this comment.
Assigning options.actorName and options.actorSummary to the module-level actorOptions object here means that these values are effectively global state. This can lead to unexpected behavior if the command is ever used in a context where it's called multiple times, as the actorOptions would retain the values from the last call. Consider passing options.actorName and options.actorSummary directly to the Application constructor to avoid this global state.
// actorOptions.name = options.actorName;
// actorOptions.summary = options.actorSummary;There was a problem hiding this comment.
I’ve refactored the code to eliminate the module-level global variable. Instead, I used a factory function to generate the necessary handlers, so actorName and actorSummary are now passed explicitly and safely scoped per invocation.
I’ve also updated the PR description to reflect these changes.
Please check latest commits that I pushed. Thank you!
dahlia
left a comment
dahlia
left a comment
There was a problem hiding this comment.
Thanks for your first contribution!
cli/inbox.tsx
Outdated
| actorOptions.name = options.actorName; | ||
| actorOptions.summary = options.actorSummary; |
|
The latest push to this pull request has been published to JSR and npm as a pre-release:
|
|
The docs for this pull request have been published: |
There was a problem hiding this comment.
Pull Request Overview
Adds --actor-name and --actor-summary options to the fedify inbox command, refactors out global state using factory functions, and replaces hardcoded actor metadata with dynamic values.
- Introduce CLI options with sensible defaults for actor name and summary.
- Refactor
createContext, fetch, and delete handlers to acceptContextData. - Update
Applicationinstantiation to useactorNameandactorSummary.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| cli/inbox.tsx | Add options, define ContextData, convert federation and handlers to use dynamic metadata. |
| CHANGES.md | Document new CLI options and link issue/PR references. |
Comments suppressed due to low confidence (3)
cli/inbox.tsx:81
- [nitpick] The local variable name
fetchshadows the globalfetchfunction and may cause confusion. Consider renaming it to something likehandleRequestorrequestHandler.
const fetch = createFetchHandler(options);
cli/inbox.tsx:38
- Consider adding unit tests for
createFetchHandlerandcreateSendDeleteToPeersto verify that theactorNameandactorSummaryoptions are correctly passed through the federation context.
interface ContextData {
CHANGES.md:34
- There's an extra
[before#262in the changelog entry ([[#262], [#285]). Remove the redundant bracket to correct the markdown links.
- Added options to customize the temporary actor information when running
cli/inbox.tsx
Outdated
| function createSendDeleteToPeers(actorOptions: { | ||
| actorName: string; | ||
| actorSummary: string; | ||
| }): (server: TemporaryServer) => Promise<void> { |
There was a problem hiding this comment.
[nitpick] The inline type { actorName: string; actorSummary: string } is duplicated in both factory functions. Extract it into a shared ActorOptions type to reduce duplication and improve clarity.
| function createSendDeleteToPeers(actorOptions: { | |
| actorName: string; | |
| actorSummary: string; | |
| }): (server: TemporaryServer) => Promise<void> { | |
| function createSendDeleteToPeers(actorOptions: ActorOptions): (server: TemporaryServer) => Promise<void> { |
There was a problem hiding this comment.
Thanks for the feedback! I've applied your recommendations.
|
There seem some conflicts. Could you rebase it on the current main? |
- Use --actor-name and --actor-summary values in Application object - Add actorOptions global object to store custom values - Replace hardcoded actor name and summary with dynamic values
- Document --actor-name and --actor-summary options for Issue fedify-dev#262
…nnotations - Remove hardcoded actorOptions - Extract ActorOptions interface to reduce inline type duplication in factory functions - Change action handler to use InboxOptions & ActorOptions intersection type - Update ContextData and ActorOptions annotation
crohasang
force-pushed
the
feature/262-inbox-actor-customization
branch
from
9232906 to
131fe7a
Compare
Thank you for pointing that out! |
dahlia
left a comment
dahlia
left a comment
There was a problem hiding this comment.
I feel like we could abstract this better than we currently have, but I can't think of how. Let's just merge it for now!
2 participants
Description
closes #262
Changes
Use --actor-name and --actor-summary values in Application object
--actor-name <name>: Customize actor display name"Fedify Ephemeral Inbox"--actor-summary <summary>: Customize actor description"An ephemeral ActivityPub inbox for testing purposes."- Add actorOptions global object to store custom valuesChecklist
Tests
deno task check-all— all checks passed.deno fmt— no formatting changes were needed.--actor-nameand--actor-summaryoptions appear in the help output (deno task cli inbox --help).deno task testand encountered an error.Based on the error message, it appears unrelated to the changes introduced in this PR.
Additional Notes
Update(KST 20250713 1628)
Refactored implementation to eliminate global state using function factory pattern for improved code maintainability and following Builder Pattern established in the codebase.
Ran
deno task check-all— all checks passed.Executed
deno fmt— no formatting changes were needed.Ran
deno task cli inboxdeno task cli inbox --actor-name "My Test Actor" --actor-summary "A temporary actor for testing"Update(KST 20250713 1058)