slightly related: there's some confusing naming in the result for harbormaster.querybuildables:

{
    "id": "13288",
    "phid": "PHID-HMBB-q34gfg4ivtnmtdfaf4sw",
    "monogram": "B13288",
    "uri": "https://secure.phabricator.com/B13288",
    "buildableStatus": "building",
    "buildableStatusName": "Building",
    "buildablePHID": "PHID-DIFF-jvgtdlv2arsrztmgf3me",
    "containerPHID": "PHID-DREV-idtirhwjhzeum3btlryb",
    "isManualBuildable": false
  }

Here buildablePHID appears to be the diff that the buildable is attached to. Calling both of these "buildables" threw me off initially.

(@yelirekim, just a heads up since whatever comes of this might impact you somehow.)

It looks like passing this to the actual query was just overlooked in D16356, presumably because it was added to keep harbormaster.querybuilds compatible but isn't actually used for anything. In the short term I'll just fix that, since it's a couple lines of code.

I agree that the output from harbormaster.querybuildables is confusing, but I'd probably plan to just fix it in the upgrade to harbormaster.buildable.search (probably by calling the thing objectPHID, I guess). We can put a very basic version of this method into the upstream today if you want to start writing against it.

Prefilling Conduit seems sort-of-reasonable, although I wonder if the primary use case is basically just reporting Conduit issues upstream? I can't really recall cases where I would have used this which weren't discussing Conduit behavior in the context of upstream development. I'd also be very vaguely worried about social engineering attacks where you prefill whatever.edit with malicious changes and trick users into clicking the button to cause non-obvious effects ("Hey, does this button work for you? I'm seeing a weird error. https://phabricator.mycompany.com/conduit/user.edit?blahblahadmin=true"). I don't think this is really much of a risk, but if the utility of the feature is also pretty narrow maybe it's not worthwhile overall.

I agree that we should not treat "ids":[] over Conduit the same as omitting the key. A plausible interpretation of this is perhaps "return no results", but I think just treating the constraint as invalid is probably better: it's way more obvious what's happening, it's consistent with the underlying API when writing upstream code in PHP, and I believe most reasonable callers have a meaningful interest in performing the test for no possible results locally (either to save the call or behave differently, like showing the user a more tailored message). I'll check what's involved here and either fix if it's easy or file a task to fix it at some point if it's more involved.

I think fixing "ids":[] is trivial; it just works like that because we use the same Conduit type ("PHIDList") both for constraints and other things which can accept an empty array (like setting subscribers to [] to remove all the subscribers on an object).

I'll just split the types or add a flag so the constraint flavor knows that it shouldn't be empty.

D16396 should make us raise a narrow error for "ids": [], "phids": [], "whateverPHIDs": [], etc., instructing you to pass a nonempty list.

Awesome! I appreciate the quick fixes!

Re: the new API, I am currently using harbormaster.querybuildables to get the buildables for a revision, so if this is changing to harbormaster.buildable.search it would be helpful to use that now instead of later -- but it's not a huge deal either.