Skip to content

convert the final reactors to CommandPost - #294

Merged
rjbs merged 6 commits into
fastmail:mainfrom
rjbs:more-commandpost
Sep 4, 2026
Merged

convert the final reactors to CommandPost#294
rjbs merged 6 commits into
fastmail:mainfrom
rjbs:more-commandpost

Conversation

@rjbs

@rjbs rjbs commented Sep 2, 2026

Copy link
Copy Markdown
Member

No description provided.

rjbs and others added 5 commits September 2, 2026 15:23
The guard meant to detect the two-argument form was testing $_ instead
of $text, so "help NAME => {...} => TEXT" would shift the option hashref
into the help text and drop the real text on the floor.

Co-Authored-By: Claude <noreply@anthropic.com>
The emoji spellings of "highfive" don't survive being split into a
command name, so this is a responder rather than a command.  The
highfive method itself is left alone, because the HTTP endpoint calls it
directly.

Co-Authored-By: Claude <noreply@anthropic.com>
"duty" and "rotors" become commands.  The rest ("replan rotors", "USER
is unavailable on DATE", "assign rotor ...") don't lead with a command
word, so they stay responders, documented under "rotors" as before.

The handlers are now async subs that await their replies, so the hub
gets a future back instead of logging a non-Future result.

Co-Authored-By: Claude <noreply@anthropic.com>
"hours" becomes a command whose parser throws a public Synergy::X when
it can't find a name to look up, so a bare "hours" now gets usage text
instead of "I don't know who that is."  The others keep their multi-word
triggers, so they're responders.

The clock-out matcher is now the only place that pattern is written; it
uses the case-insensitive form the old handler used, rather than the
case-sensitive form the old predicate used.

Co-Authored-By: Claude <noreply@anthropic.com>
Every trigger began with "vesta", so the twelve listeners collapse into
one "vesta" command that dispatches on a table of subcommand patterns,
the way InABox's "box" does.  The handlers now take their arguments
instead of re-parsing the event text.  Yes, this is weird.

Two consequences worth naming: the twelve help entries all titled
"vesta" become one, and "vesta something-else" now gets a "I don't know
that vesta command" reply where it used to go unanswered.

_pay_to_post_payload now returns a future on every path, so the posting
handlers have something to await.

Co-Authored-By: Claude <noreply@anthropic.com>
@rjbs
rjbs requested a review from Davis-A September 2, 2026 05:25

@Davis-A Davis-A left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approved with please fix those awaits on error replies

Comment thread lib/Synergy/Reactor/Vestaboard.pm Outdated
if ($lock && $lock->{locked_by} ne $user->username) {
$event->error_reply("Sorry, the board can't be changed right now!");
return;
return $event->error_reply("Sorry, the board can't be changed right now!");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this should be awaited. Surprised me that it's not an async sub but if you follow it down you end up at

  my $future = $send_cb->($text, $alts);

  $self->from_channel->note_reply($self, $future, $args);
  return $future;

ditto all the others

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This doesn't need to be awaited. _pay_to_post_payload is not an async sub, so it returns a future immediately so that the called can await it or otherwise sequence it. If you look at its callers, they await it, as is right and proper.

ditto all the others ;-)

That said, my response is basically "My code is correct, and you should understand both ways of doing this at all times." A better response might be, "My code is correct, but I should stick to doing these things in a single way."

So I will apply that "reformatting" which will not fix a bug, and ship thusly.

It returned its futures for the caller to await, which worked but meant
you had to go read the callers to know that.  Awaiting in place says so
locally.  The failed-post branch now logs the error it swallows.

Co-Authored-By: Claude <noreply@anthropic.com>
@rjbs
rjbs merged commit de932c0 into fastmail:main Sep 4, 2026
1 check passed
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