feat(actions): resolve select-all selections for approval-required actions

[EnkiP]

Summary

Ruby port of ForestAdmin/agent-nodejs#1847 — enabling bulk actions with approval on "select all" selections.

• When an approval-required action is triggered with all_records: true, the route resolves the selection (caller filter, primary keys only, cap+1 fetch) and hands the ids back through CustomActionRequiresApprovalError → errors[0].data.recordIds, so the frontend stores the full target set in the approval request.
• New max_records_for_approval setting (default 500, matching the Forest server's authoritative cap on approval record ids); above it the trigger is rejected with ApprovalSelectionTooLargeError (422). Normal executes are never capped.
• The resolver is threaded route → can_smart_action? → SmartActionChecker and invoked at raise time, mirroring the agent-nodejs design.
• The error data now also carries roleIdsAllowedToApprove — the key the frontend actually reads (it was only sent as user_approval_enabled before, which the frontend ignores).

Related PRs

• agent-nodejs: feat(agent): resolve select-all selections for approval-required actions agent-nodejs#1847
• Frontend (uses data.recordIds, with a fetch-based backport for agents without this feature): ForestAdmin/forestadmin#9924
• Server (authoritative 500 cap): ForestAdmin/forestadmin-server#8462

fixes PRD-934

Test plan

• smart_action_checker specs: resolver ids in the error data, no ids without resolver, approver-roles key (28 pass)
• rubocop clean on changed files
• Note: no route-level spec — actions_spec.rb fails 8/8 on main (pre-existing breakage in route construction); the resolution logic is covered through the checker specs and mirrors the tested agent-nodejs implementation.

🤖 Generated with Claude Code

Note

Resolve select-all record IDs for approval-required actions in SmartActionChecker

• Approval-required smart actions triggered with all_records: true now resolve targeted record IDs from the database when the action scope is not GLOBAL. The resolved IDs are included in the CustomActionRequiresApprovalError details.
• Adds resolve_select_all_record_ids to SmartActionChecker, which fetches primary keys up to a configurable limit. Adds max_records_for_approval to ForestAdminRails config, defaulting to 500 and clamped to a hard cap of 500.
• Duplicates approver role IDs under roleIdsAllowedToApprove in the approval error details for frontend consumption.
• Behavioral Change: Select-all approval actions targeting more than MAX_RECORDS_FOR_APPROVAL (500) records now raise ApprovalSelectionTooLargeError instead of triggering the approval flow.

^{Macroscope summarized d92d810.}

[linear-code]

PRD-934

[qltysh]

All good ✅

[qltysh]

Coverage Impact

⬆️ Merging this pull request will increase total coverage on main by 0.1%.

Modified Files with Diff Coverage (1)

Rating	File	% Diff	Uncovered Line #s
	..._agent/lib/forest_admin_agent/services/smart_action_checker.rb	100.0%
	Total	100.0%

🚦 See full report on Qlty Cloud »

🛟 Help

• Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.

• Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.

• File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)

  • Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

[EnkiP]

8d7e211 ports the Global-scope fix from agent-nodejs#1847: a global approval-required action triggered with all_records no longer resolves (and caps) the selection — it targets no specific records, so the 422 above the cap / whole-collection snapshot below it were regressions. The schema scope rides along with the permission data into SmartActionChecker; new spec asserts no recordIds in the error and no list call.

[matthv]

Review findings — select-all approval on agent-ruby

Ruby port of agent-nodejs#1847, cross-checked against the whole PRD-934 chain. No critical issues — the cap itself can't be silently bypassed (Ruby's > doesn't have the NaN failure mode we found on the Node side). Important findings below.

🟠 Important

• smart_action_checker.rb:104-116 + forest_admin_rails.rb:41 — max_records_for_approval is read raw, no validation. A negative value reaches Page.new(limit: max + 1) and gets passed straight to the datasource: on SQLite a negative LIMIT is treated as unlimited, so the query fetches the whole table before the cap check ever rejects it — real perf/DoS risk on a large collection from a config typo. A String/true value raises a bare ArgumentError, uncaught, surfacing as a generic 500 "Unexpected error" on every select-all trigger with no indication max_records_for_approval is the cause.
• permissions.rb:204 (collection.schema[:actions][action['name']]&.scope) — this is what feeds the global-action skip guard, but it's never exercised at its real integration point: every existing spec sets scope by hand directly on the checker. A lookup-miss here silently degrades to scope = nil (not global), which would silently reintroduce the exact bug the global-action fix (8d7e211) was written to close, with nothing catching it.
• The PR's stated reason for skipping a route-level spec ("actions_spec.rb fails 8/8 on main, pre-existing") doesn't hold — ran it on both main and this branch, 8/8 pass on both. Worth either fixing that claim or adding the route-level test it was meant to justify skipping.

Happy to open follow-up tickets if useful.

[EnkiP]

Addressed the review in d92d810:

Unvalidated max_records_for_approval — fixed: anything but a positive Integer now falls back to the 500 default, closing both failure modes (negative value reaching the datasource as an unlimited LIMIT, and String/true raising an uncaught ArgumentError as a 500 on every select-all trigger). Spec covers -5, a String, and true, asserting the fetch stays at limit 501.

Schema-scope merge unexercised — fixed: a new can_smart_action? spec drives the enabled path and asserts the checker receives the scope read from collection.schema[:actions] (merged with the permission data), so a silent lookup-miss regression on the global-action guard would now fail a test at its real integration point.

actions_spec.rb claim — you're right, and the claim is withdrawn: it passes 12/12 on this branch locally too. The failure I hit earlier was an artifact of the working-tree state I ran it in, not of main. That said, the route-level test it was meant to justify is now moot in its original form — resolution has since moved out of the route into SmartActionChecker (5ed850b), and the route-level line left is a plain can_smart_action? call; the integration point that needed coverage is the permissions merge, which the new spec above covers.

Full suite: 1163 examples, 0 failures; rubocop clean.

[forest-bot]

🎉 This PR is included in version 1.40.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀