Summary: Fixes T7111. Also nails out the TODO to show the username
Test Plan: fired up the ole phabot, chatted "P123" and saw "P123: https://secure.phabricator.com/P123 - Masterwork From Distant Lands by epriestley"...!
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7111
Differential Revision: https://secure.phabricator.com/D11664
Summary: Ref T6822. This method is only called from `PhutilDaemon::execute()` and can be made `protected`.
Test Plan: See D11404.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6822
Differential Revision: https://secure.phabricator.com/D11405
Summary: Fixes T5884. Macro images are no longer public on most installs. We could generate tokens for them, but this (using Conduit to pull the file data) is easier and more correct.
Test Plan: Logged a bot into IRC and had it spam part of a macro before being killed for flooding.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5884
Differential Revision: https://secure.phabricator.com/D10274
Summary: Fixes T5883. The first time we hit an error we'll continue forward; we only bail after the second time. Instead, check for an error immediately
Test Plan: HA HA HA DID NOT TEST HA HA HA HA
Reviewers: btrahan, cburroughs
Reviewed By: cburroughs
Subscribers: epriestley
Maniphest Tasks: T5883
Differential Revision: https://secure.phabricator.com/D10265
Summary: Fixes T3173. This doesn't actually fix T3173 but I'm going to redirect that. It does make the bot quit IRC gracefully, with a nicer quit message, which can be customized.
Test Plan: Got a bot to quit IRC with nice messages.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T3173
Differential Revision: https://secure.phabricator.com/D10257
Summary:
Fixes T5855. Adds a `--graceful N` flag to `phd stop` and `phd restart`.
`phd` will send SIGINT, wait `N` seconds, SIGTERM, wait 15 seconds, and SIGKILL. By default, `N` is 15.
Test Plan:
- Ran `bin/phd debug ...` and used `^C` to interrupt daemons. Saw graceful shutdown behavior, and abrupt termination on multiple `^C`.
- Ran `bin/phd start`, `bin/phd stop` and `bin/phd restart` with `--graceful` set to various things, notably `0`. Saw graceful shutdowns on the CLI and in the web UI. With `0`, abrupt shutdowns.
Reviewers: btrahan, hach-que
Reviewed By: hach-que
Subscribers: epriestley
Maniphest Tasks: T5855
Differential Revision: https://secure.phabricator.com/D10228
Summary:
Restores functionality for Flowdock->Chatbot adapter.
Most likely the result of API changes in the year since the original patch was contributed,
the flowdock adapter no longer worked.
This makes a few tweaks to both the base streaming adapter class and the flowdock adpater. I took care to not disturb the functionality of the campfire adapter, but I don't have any way to test it.
Test Plan: I am new here and I have no idea what to write other than sarcastic things but I'll most like amend this after review.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D10168
Summary: I'm pretty sure that `@group` annotations are useless now... see D9855. Also fixed various other minor issues.
Test Plan: Eye-ball it.
Reviewers: #blessed_reviewers, epriestley, chad
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin, hach-que
Differential Revision: https://secure.phabricator.com/D9859
Summary: This class has been deprecated for a while now (see rP0a8b0d1392bd79b4e88fbf910b176c960d57b4b4). It should be safe to remove.
Test Plan: N/A
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9467
Summary: This class has been deprecated for a while now (see rPdad7c65bf56384480be7c18e02fdc01ea67cf1ff). It should be safe to remove.
Test Plan: N/A
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9468
Summary: This class has been deprecated for a while (see rP574bc3ba31cca2767bafe7844d7f854d90d6be1c). It should be safe to remove now.
Test Plan: N/A
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9469
Summary: Applied some more linter fixes that I previously missed because my global `arc` install was out-of-date.
Test Plan: Will run `arc unit` on another host.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9443
Summary: Ran `arc lint --apply-patches --everything` over rP, mainly to change double quotes to single quotes where appropriate. These changes also validate that the `ArcanistXHPASTLinter::LINT_DOUBLE_QUOTE` rule is working as expected.
Test Plan: Eyeballed it.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin, hach-que
Differential Revision: https://secure.phabricator.com/D9431
Summary: Someone stole my bot's name, so the bot couldn't (re)connect. This tries adding some text to the name if it encounters a 433 'nick in use' error
Test Plan: Started a ton of bots: F154744
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9123
Summary: Adds support for custom SSL certs in the IRC bot config, same as in .arcconfig
Test Plan: Bot wouldn't connect before. Added this code and corresponding line in bot config, now it does.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Differential Revision: https://secure.phabricator.com/D8393
Summary: `What's new` has been broken for awhile, I've updated it to use the `feed.query` text view.
Test Plan: Start up a bot and say "What's new?"
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley
CC: fas, epriestley, aran, Kage, demo
Differential Revision: https://secure.phabricator.com/D8118
Summary:
Previously, if there were no macros, we would ping conduit for a list of macros until we got something. Now we cache false when there are no results.
T3045
Test Plan: Ensure the init doesn't call the ##macro.query## conduit method more than once during the PhabricatorBot's lifetime.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran
Maniphest Tasks: T3045
Differential Revision: https://secure.phabricator.com/D6671
Summary: Dump everything to the debug log for `phd debug phabricatorbot ...`
Test Plan: Ran `phd debug phabricatorbot ...`
Reviewers: chad, AnhNhan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5239
Summary: Fixes T2651. This could be futher generalized but it's a bit out of the way.
Test Plan: See chatlog.
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2651
Differential Revision: https://secure.phabricator.com/D5236
Summary:
PhabricatorBotMessage->getSender returns a PhabricatorBotUser object (which potentially can be null)
So check null and then use getName to get actual name of the sender
Test Plan: Run phabot and add myself to ignore list
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5036
Summary: Mostly applies a new call spacing rule; also a few things that have slipped through via pull requests and such
Test Plan: `find src/ -type f -name '*.php' | xargs -n16 arc lint --output summary --apply-patches`
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5002
Summary: First pass. Flowdock supports interesting message types (like replies to messages), but for now implementing a standard messaging interface.
Test Plan: Ran both a Flowdock bot and a Campfire bot. Made sure both still connected and responded properly to the Object Handler.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4983
Summary:
Clearly silly to have a separate handler for this. I also made most of the protocol stuff direct writes so we don't need to ship them through handlers, and made the adapter ignore message it does not understand by default instead of sending them to IRC, and added PASTE "support".
We could still let handlers react to these messages by emitting them all as 'RAWIRC' or similar, but there's currently no need for that so I didn't bother.
Also fix an issue in D4924 with nickpass.
Test Plan: Had bot join IRC, talked to it.
Reviewers: indiefan
Reviewed By: indiefan
CC: aran
Differential Revision: https://secure.phabricator.com/D4925
Summary:
Make users/channels/rooms into objects, so we can later sort out stuff like Campfire user IDs, Phabricator vs chat accounts, etc.
The only change here is that I removed output buffering from the macro handler. We should move throttling/buffering to adapters instead and have it apply globally.
Test Plan: Ran IRC and Campfire bots and interacted with them.
Reviewers: indiefan
Reviewed By: indiefan
CC: aran
Differential Revision: https://secure.phabricator.com/D4924
Summary:
- Reduce visibiliy of config.
- Add a typehint.
Test Plan: Ran campfire/irc bots and chatted with them.
Reviewers: indiefan
Reviewed By: indiefan
CC: aran, amerigomasini
Differential Revision: https://secure.phabricator.com/D4923
Test Plan: Ran the bot with a handler that sends sound commands.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4922
Summary: Also added sender to the campfire adapter. This isn't extremely useful as it's just a numeric id, but it allows us to add ignores (specifically having the bot ignore itself).
Test Plan: Ran the bot, ignored itself.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4893
Summary:
- Use PhutilURI to correct for specifying "https://yourname.campfire.com/" instead of "https://yourname.campfire.com".
- Use HTTPSFuture to get logging via `--trace` and error detection (CA stuff should be OK since 37signals has real certs).
- On destruction, only try to leave rooms we've actually joined.
Test Plan: Setup a bot, had it join a room, talked to it.
Reviewers: indiefan
Reviewed By: indiefan
CC: aran
Differential Revision: https://secure.phabricator.com/D4849
Summary:
Decided the best approach for refactoring the message/command stuff would be to actually start implementing the campfire adapter to get a better idea of what the abstractions should look like. It feels awkward and unwieldy trying to maintain the irc command interface (notice the message instantiation in the `processReadBuffer()` method. However, i'm still not clear what the best approach is without requiring a re-write of nearly all the existing handlers and defining essentially a custom dsl on top of irc's.
I suppose given that alternative, implementing to irc's dsl doesn't sound all that bad. Just feels like poor coupling.
Also, I know that there is some http stuff in libphutil's futures library, but the https future is shit and I need to do some custom curlopt stuff I wasn't sure how to do with that. But if you think this should be refactored, let me know.
I tested this with the ObjectHandler (messages with DXXX initiate the bot to respond with the title/link just as with irc), but beyond that, I haven't tried any of the other handlers, so if there are complications you think i'm going to run into, just let me know (this is one of the reasons for requesting review early on).
Also, this diff is against my last one, even though that hasn't been merged down yet. It was starting to get large and I'd prefer to keep to two conversations separate.
Fixing some lint issues.
Test Plan: Ran the bot with the Object Handler in campfire and observed it behaving properly.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2462
Differential Revision: https://secure.phabricator.com/D4830
Summary:
Ugh, just wrote out a huge message, only to lose it with a fat-fingered ctrl-c. Le sigh.
First pass at decoupling the bot from the protocol. Noticeably absent is the command/message coupling. After this design pass I'll give that a go. Could use some advice, thinking that handlers should only create messages (which can be public or private) and not open ended, undefined 'commands'. The problem being that there needs to be some consistant api if we want handlers to be protocol agnostic. Perhaps that's a pipedream, what are your thoughts?
Secondly, a few notes, design review requests on the changes i did make:
# Config. For now i'm passing config through to the adapter. This was mainly to remain backwards compatible on the config. I was thinking it should probably be namespaced into it's own subobject though to distinguish the adapter config from the bot config.
# Adapter selection. This flavor is the one-bot-daemon, config specified protocol version. The upside is that in the future they won't have to run different daemons for this stuff, just have different config, and the door is open for multiple protocol adapters down the road if need be. The downside is that I had to rename the daemon (non-backwards compatible change) and there will need to be some sort of runtime evaluation for instatiation of the adapter. For now I just have a crude switch, but I was thinking of just taking the string they supply as the class name (ala `try { new $clasName(); } catch...`) so as to allow for homegrown adapters, but I wasn't sure how such runtime magic would go over. Also, an alternative would be to make the PhabricatorBot class a non-abstract non-final base class and have the adapters be accompanied by a bot class that just defines their adapter as a property. The upside of which is backwards compatibility (welcome back PhabricatorIRCBot) and perhaps a little bit clearer plugin path for homegrowners.
# Logging. You'll notice I commented out two very important logging lines in the irc adapter. This isn't intended to remain commented out, but I'm not sure what the best way is to get logging at this layer. I'm wary of just composing the daemon back down into the adapter (bi-directional object composition makes my skin crawl), but something needs to happen, obviously. Advice?
That's it. After the feedback on the above, you can either merge down, or wait until i finish the command/message refactor if you don't think the diff will grow too large. Up to you, this all functions as is.
Test Plan: Ran an irc bot, connected, read input, and wrote output including handler integration.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2462
Differential Revision: https://secure.phabricator.com/D4757
