Skip to content

fix: 676 ux dropdown messages - #26

Merged
daveomri merged 13 commits into
apify:fix/668-pipedream-retest-fixesfrom
daveomri:fix/676-ux-dropdown-messages
Sep 3, 2026
Merged

fix: 676 ux dropdown messages#26
daveomri merged 13 commits into
apify:fix/668-pipedream-retest-fixesfrom
daveomri:fix/676-ux-dropdown-messages

Conversation

@daveomri

@daveomri daveomri commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Why

First-run UX fixes from Pipedream retest #612: empty Actor dropdown for new accounts, inconsistent "actor"/"Actor" casing, confusing "Run Asynchronously" label, unhelpful no-build error.

What changed

apify.app.mjs

  • Empty "Recently used" list now shows a hint row pointing to Apify Store instead of a blank dropdown.
  • Clearer actorId / actorSource descriptions.

run-actor.mjs (0.0.7 > 0.0.8)

  • ⚠️ Breaking: runAsynchronously > waitForFinish (key renamed, boolean inverted, default flips to wait, matching Run Task). Existing steps fall back to the new default; deployed steps stay pinned until updated.
  • Selecting the hint row shows an info alert instead of erroring.
  • No-build error now names the Actor and points to Console Source > Code > Build / CLI / API.
  • actor > Actor casing fixes.

sources/common/base.mjs + new-finished-actor-run-instant (stays 0.0.8, bumped by umbrella branch)

  • activate() throws before webhook creation if the condition target is empty, so the hint row can't register a webhook on an empty Actor ID. Message overridable via getEmptyConditionMessage().
  • Same actorSource description tweak.

README.md

  • actor > Actor casing sweep.

Testing

  • npm run lint:fix clean (pre-existing action-annotations errors in get-kvs-record/run-task are unrelated).
  • Build terminology confirmed against Apify docs.
  • Live pd publish smoke test: empty-account hint, wait on/off, new no-build message.

Closes #676

@daveomri
daveomri marked this pull request as ready for review July 29, 2026 17:09

@JanHranicky JanHranicky left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM ✅

@drobnikj

Copy link
Copy Markdown
Member

Net effect: steps that relied on the old async default will start waiting for completion instead. Output Record Key is gated to the wait path, so it now shows by default.

Not sure how the breaking changes work in pipedream, maybe they have some process for it.

type: "string",
label: "Search Actors from",
description: "Where to search for Actors. Valid options are Store and Recently used Actors.",
description: "Where to search for Actors. Choose **Apify Store Actors** to browse the public [Apify Store](https://apify.com/store), or **Recently used Actors** to pick from Actors you've run before.",

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.

Important: this component's content changed but version is still 0.0.8 (same as develop), while the apify_oauth mirror was bumped to 0.0.4 — the description tweak won't ship for the non-OAuth source. Needs a bump too.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

thanks, will do in next commit

Comment thread components/apify/apify.app.mjs Outdated
limit: LIMIT,
});

if (actorSource !== "store" && page === 0 && items.length === 0) {

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.

Note: the fallback is gated to page === 0, so scrolling past the first page of the Store list re-queries listUserActors and comes back empty — the dropdown looks like it ran out after one page. Consider tracking that the fallback is active so later pages also page through the Store.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I thought about this and wonder if this logic is even necessary; this is only an edge case for new users who haven't run any actors yet, but it adds complexity to the code (due to limitations of pipedream fallback logic)

So in the next logic, I introduce a sentinel value when the list is empty, informing users to select "from store" instead. If they select this sentinel value, then an error will be raised with an invalid Actor being selected.

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.

Agree, let's do it with sentinel value makes more sence.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

it will look like this
obrazek

and if they select that value, it will show a message to choose the "from store"
obrazek

reloadProps: true,
optional: true,
},
runAsynchronously: {

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.

Note: About breaking change, can we just keep the field, but just make it hidden somehow?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Hiding the old field would only preserve behavior by making the new "Wait for Finish" toggle silently inert for existing steps, so I think that clean rename is the better trade.

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.

OK, it that case we need to do breaking change, can you check what this means, maybe we would need to bump version?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

yeah, we resolve the breaking change via version bump, 0.0.7 -> 0.0.8 (which is already covered)

existing deployed steps stay pinned to their old version, so they keep working fine

@daveomri
daveomri changed the base branch from develop to fix/668-pipedream-retest-fixes August 25, 2026 12:51
@daveomri daveomri self-assigned this Aug 25, 2026
@daveomri
daveomri requested a review from drobnikj August 25, 2026 13:05

@drobnikj drobnikj 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.

Two more comments

if (page === 0 && items.length === 0 && actorSource !== "store") {
return [
{
label: "No recent Actors, switch to \"Apify Store Actors\" above",

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.

Important: getActorOptions is shared — new-finished-actor-run-instant.mjs:42 calls it too, so this hint row appears in the trigger's Actor dropdown as well, but only run-actor.mjs guards on actorId === "". Picking it there yields getCondition() -> { actorId: "" } and a webhook on an empty Actor ID; the sentinel needs handling where it's consumed, or it shouldn't live in the shared method.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Handled it in base.mjs activate(), which now throws a ConfigurationError before creating the webhook if the condition target is empty. Thanks

reloadProps: true,
optional: true,
},
runAsynchronously: {

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.

OK, it that case we need to do breaking change, can you check what this means, maybe we would need to bump version?

@daveomri
daveomri requested a review from drobnikj August 28, 2026 09:57

@drobnikj drobnikj 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.

Last note, but pre approve

Comment thread components/apify/sources/common/base.mjs
@daveomri
daveomri merged commit 5482671 into apify:fix/668-pipedream-retest-fixes Sep 3, 2026
drobnikj pushed a commit that referenced this pull request Sep 3, 2026
…HQ#21493)

* feat(slack_v2): eval-driven fixes for the MCP tool surface

Iterated against the MCP eval suite (pd-connect-eval-monster/evals/slack_v2).
Suite went 23/35 -> 33/35 on Sonnet 5 (pass^2); every change below is tied to a
specific red-to-green flip or a measured payload reduction.

- list-channels: evals #2/#10/#11/#16/#19/PipedreamHQ#35 all failed the same way — the
  action returned every field of every channel with numPages defaulting to 1, so
  a 62-channel workspace produced ~55k chars, blew the 25k-token MCP ceiling, and
  the model was handed a file path instead of data (#2 burned all 20 turns
  re-calling it). Added an additive `fields` projection, a `cursor` prop, and
  `has_more`/`next_cursor` so truncation is visible instead of silent. Payload
  55k -> 5.9k chars; all six evals pass.  [minor]

- slack_v2.app.mjs: `assistantSearch` called `sdk().apiCall()` directly — the one
  path in the app that bypassed `_withRetries` — while the client is built with
  `rejectRateLimitedCalls: true`, so every 429 rejected instantly. 8 of 12 search
  calls errored in one run (#24/#26/#27). Routed through `_withRetries`. Also adds
  `resolveUserId` (id / email / display name) for the invite fix below.

- delete-message: eval #20 ("post a note then take it back") failed every run with
  cant_delete_message — post-message sends no `as_user` so it posts as the USER,
  while this action defaulted `as_user: false`, which routes to the BOT token. A
  message just posted could never be deleted. Now retries with the other identity
  on cant_delete_message; the default is unchanged, so existing workflows are
  unaffected. Description also gained confirmation guidance for the destructive
  path (PipedreamHQ#33).  [minor]

- invite-user-to-channel: eval #10 failed with user_not_found — the agent passed
  the email from the prompt. Now resolves user id / email / display name, and the
  channel by name.  [minor]

- get-channel-details, list-members-in-channel, set-channel-topic: every
  AI-optimized action in this app resolves channel NAMES server-side and these did
  not, so agents that read "#seinfeld-general" from a prompt got channel_not_found
  (#19, #22) or a bare internal_error (#10). All three now use resolveChannelId.
  [minor]

- get-channel-history, get-thread-replies: outputs measured at 17k chars average
  (worst 49k) and 25k average respectively — every call burned that much of the
  agent's context. Added the same additive `fields` projection.  [minor]

- get-user-details, get-current-user: two tools answer "who am I" and the agent
  chose the legacy one in 10/10 trials (#1). Descriptions now differentiate them.
  NOTE: this did NOT change routing — the agent still picks get-current-user. The
  actual fix is removing one of the two from the /v3 component allowlist; these
  edits only make the intended split legible.  [patch]

App package.json bumped 0.6.1 -> 0.7.0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(slack_v2): satisfy CI — lint formatting, dependent version bumps, UI-centric copy

- Lint Code Base: 9 errors, all in the files this branch touches and all
  formatting. Broke the `.split().map().filter()` chains across lines
  (newline-per-chained-call), split the `has_more` spread ternary and its object
  literal (multiline-ternary, object-curly-newline), and expanded the
  `users.list` destructure in resolveUserId (object-curly-newline).

- Ensure component commits modify component versions: this branch edits
  slack_v2.app.mjs, so every component importing it needs a version bump.
  Patch-bumped the 48 actions/sources that were not already versioned in the
  previous commit.

- Removed UI-centric wording from two descriptions, which read oddly for an
  agent calling the tool over MCP and contradicted the resolution behaviour
  added in the previous commit:
  - get-channel-details: "by selecting it or providing an ID" -> "specified by
    ID or by name" (it had claimed both "selecting it" and "Accepts a channel
    ID or NAME" in the same sentence).
  - set-channel-topic: "a selected channel" -> "a channel, specified by ID or
    by name".

Verified locally with ESLint 9 against the repo's own rule options for the three
failing rules: 0 violations across components/slack_v2.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(slack_v2): restore comma-separated users on invite, fix delete identity fallback

invite-user-to-channel: conversations.invite documents `users` as comma-separated and
the prop is a plain string, so "U01ABCDEF,U02GHIJKL" was a working input. The new
resolveUserId() matches neither the ID regex (no comma) nor the email regex, then
falls through to an exhaustive paged users.list scan that cannot match, and throws.
Adds resolveUserIds() (plural), which splits on commas — or accepts an array —
resolves each token independently, and rejoins. An all-IDs input still makes zero
extra API calls, so the common path is unchanged in cost as well as result.

delete-message: makeRequest() routes to the bot token only on `as_user === false`, so
`!this.as_user` made the retry a no-op whenever as_user arrived nullish — undefined
and true both route to the user token, so it retried as the identity that had just
been refused. Changed to `this.as_user === false`. Attempt 1 still passes the
configured value verbatim.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(slack_v2): reattach resolveChannelId's JSDoc to its function

resolveUserId/resolveUserIds were inserted between resolveChannelId's doc comment
and resolveChannelId itself, leaving two stacked doc blocks above resolveUserId and
no doc on resolveChannelId. Comment move only; no code change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* refactor(slack_v2): move the fields projection into common/utils.mjs

Addresses review feedback on PipedreamHQ#21493: `pickFields` was copy-pasted identically into
get-channel-history, get-thread-replies and list-channels.

Moved to components/slack_v2/common/utils.mjs, alongside the CSV-or-array `fields`
normalizer that was duplicated in the same three places — both halves of one feature,
so splitting them across files would just invite the next divergence.

`projectFields(records, fields)` composes the two and keeps the additive contract:
with no `fields`, it returns the ORIGINAL array, so a caller that omits the prop gets
exactly what these actions have always returned.

Behavior-identical to the three removed copies, verified against them across
undefined / empty array / empty string / array / CSV / CSV-with-spaces / unknown
field / all-unknown inputs, including array identity on the no-fields path.

No version bumps: all three components are already ahead of master on this branch,
so the PR diff still shows a version change for each.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(slack_v2): address review feedback on delete-message, resolveUserId, invite-user-to-channel

- delete-message: only retry with the other identity when a bot token exists,
  since otherwise both attempts hit the same token and just repeat the error.
- resolveUserId: scan all users.list pages and require an exact, unambiguous
  name match; throw ConfigurationError instead of silently picking the first.
- invite-user-to-channel: treat already_in_channel as success only for a
  single user; bulk invites are now sent per-user with individual results so
  failures aren't hidden behind a successful summary.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(slack_v2): guard invite-user-to-channel against empty resolved user list

resolveUserIds() returns "" when every comma-separated token is blank, and
"".split(",") yields [""] rather than [], which slipped through the
single-user path and sent an empty users value to Slack. Filter empty tokens
and throw ConfigurationError when none remain.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(slack_v2): resolve channel name in delete-message, treat self-invite as no-op

- delete-message now calls resolveChannelId() before deleting, like
  every other AI-optimized action in this app. Previously it passed
  `conversation` straight through, so a channel NAME (rather than ID)
  failed with channel_not_found.
- invite-user-to-channel: Slack refuses to let an identity invite
  itself (cant_invite_self) before it would ever reach an
  already_in_channel check. Treat that the same way — a no-op, not an
  error — since to the caller it means the same thing: the user has no
  further action needed on that channel.

Found via eval-driven regression testing on PR PipedreamHQ#21493.

* chore(slack_v2): revert version bumps on delete-message, invite-user-to-channel

Keep at 0.2.0/0.1.0 rather than bumping per fix.

---------

Co-authored-by: Dylan Sather <Dylan Sather>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Michelle Bergeron <michelle.bergeron@workday.com>
Co-authored-by: michelle0927 <michelle0927@users.noreply.github.com>
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.

4 participants