Skip to content

feat(agent-bff): expose the action form endpoint#1748

Open
nbouliol wants to merge 4 commits into
mainfrom
feature/prd-673-expose-the-action-form-endpoint
Open

feat(agent-bff): expose the action form endpoint#1748
nbouliol wants to merge 4 commits into
mainfrom
feature/prd-673-expose-the-action-form-endpoint

Conversation

@nbouliol

@nbouliol nbouliol commented Jul 9, 2026

Copy link
Copy Markdown
Member

Fixes PRD-673

What

Adds POST /v1/:collection/actions/:action/form to agent-bff. It loads a Smart Action form without failing on unknown UI-supplied values and returns the fields a UI needs to render (including the layout). No activity log is written.

Response shape:

{ fields: [{ name, type, value, isRequired, enumValues? }], canExecute, requiredFields, skippedFields, layout }

How

  • Action resolution via the read-model action allow-list (isActionAllowed). Since the endpoint map is the allow-list, an absent action cannot be told from a known-but-disallowed one → every non-exposed action maps to unknown_action (404); action_not_allowed (403) has no local trigger, mirroring the collection/relation precedent.
  • Form load reuses agent-client (createRemoteAgentClient().collection().action()), so the stateful /hooks/load + /hooks/change logic is not reimplemented. tryToSetFields skips unknown submitted values and returns them as skippedFields.
  • Error mapping (PRD-670): each agent-hitting call (loadActionForm, tryToSetFields) is wrapped on its own; field/layout reads and mapping stay outside so a local bug surfaces as a 500. Fields and layout are read after tryToSetFields because a change hook rebuilds them.
  • Shared read-model store: extracted src/http/agent-route-helpers.ts (token guard, read-model resolution, callAgent, decodeSegment) so the new action middleware and the existing data middleware share one cache.

Decisions / deviations from the ticket

See the planning-decisions comment on PRD-673 for the full list. Highlights:

  • layout = raw ForestServerActionFormLayoutElement[] array. The spec is self-contradictory (prose says "ActionLayoutRoot serialized", example shows {type,component,elements}, actual JSON.stringify yields {layout:[...]}). We return the raw element array. ⚠️ To confirm with Anthony — trivial to change.
  • enumValues present only for Enum fields (matches the MCP tool + AC enumValues?; the spec example shows null everywhere).
  • recordIds: [] accepted for global/bulk actions (spec AC only covers missing recordIds); non-array → 400 invalid_request with no agent call.
  • Timezone limitation: form hooks use agent-client’s hardcoded Europe/Paris (cannot forward the resolved timezone without an agent-client change, forbidden here). Tracked in PRD-768.
  • "No activity log" test is behavioral: no activity-log writer exists in agent-bff yet (PRD-637); the test asserts the action is never executed / only form hooks are called.

Tests

test/action/action-routes-middleware.test.ts + test/action/action-form-mapper.test.ts: full response shape incl. layout, skipped values, canExecute true/false, post-change-hook read ordering, recordIds missing/non-array/[], values non-object, unknown_action 404, agent failures on load and tryToSetFields → PRD-670, 401/503.

Lint ✓ · build ✓ · 565/565 tests ✓.

Stacked on the PRD-672 branch — merge #1746 first, then this rebases onto main.

🤖 Generated with Claude Code

Note

Expose POST action form endpoint in agent-bff

  • Adds a POST /agent/v1/:collection/actions/:action/form route in action-routes-middleware.ts that loads an action form from the agent, applies submitted values via tryToSetFields, and returns fields, canExecute, requiredFields, skippedFields, and layout.
  • Adds action-form-mapper.ts to serialize ActionForm to a response object, including enumValues only for Enum-typed fields.
  • Adds agent-action-client.ts as a thin client that calls the remote agent to load action forms.
  • Extracts shared helpers (decodeSegment, resolveReadModel, requireAgentToken, callAgent) into agent-route-helpers.ts, replacing duplicated logic in the data routes middleware.
  • Updates cli-core.ts to register both data and action route middlewares sharing a single read-model store.

Macroscope summarized ae6772e.

@linear-code

linear-code Bot commented Jul 9, 2026

Copy link
Copy Markdown

PRD-673

Comment thread packages/agent-bff/src/action/action-routes-middleware.ts Outdated
@qltysh

qltysh Bot commented Jul 9, 2026

Copy link
Copy Markdown

Qlty


Coverage Impact

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

Modified Files with Diff Coverage (6)

RatingFile% DiffUncovered Line #s
Coverage rating: A Coverage rating: A
packages/agent-bff/src/cli-core.ts100.0%
Coverage rating: A Coverage rating: A
packages/agent-bff/src/data/data-routes-middleware.ts100.0%
New Coverage rating: A
packages/agent-bff/src/action/agent-action-client.ts100.0%
New Coverage rating: A
packages/agent-bff/src/http/agent-route-helpers.ts100.0%
New Coverage rating: A
packages/agent-bff/src/action/action-form-mapper.ts100.0%
New Coverage rating: A
packages/agent-bff/src/action/action-routes-middleware.ts100.0%
Total100.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.

@nbouliol nbouliol force-pushed the feature/prd-672-expose-relation-list-and-count-with-foreign-collection branch from d10c98b to a65f89e Compare July 10, 2026 08:35
@nbouliol nbouliol force-pushed the feature/prd-673-expose-the-action-form-endpoint branch from 636b46b to 5a335a9 Compare July 10, 2026 09:13
Base automatically changed from feature/prd-672-expose-relation-list-and-count-with-foreign-collection to main July 10, 2026 12:31
nbouliol and others added 3 commits July 10, 2026 14:34
Add POST /v1/:collection/actions/:action/form: resolve the action via
the read-model action allow-list, load its form through agent-client
(tryToSetFields skips unknown submitted values), and return
{ fields, canExecute, requiredFields, skippedFields, layout }. No
activity log is written.

Extract shared token-guard/read-model/callAgent/decodeSegment helpers so
the new action middleware and the data middleware share one read-model
store.

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

agent-client filters recordIds with .filter(Boolean) before serializing,
so a valid numeric primary key of 0 was silently dropped and the form
loaded as an unscoped/global action. Coerce ids to strings on the way in.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ry, read-model rethrow

- action-routes-middleware: non-POST and non-matching path fall through to next
- agent-action-client: real factory wires createRemoteAgentClient().collection().action()
- agent-route-helpers: resolveReadModel rethrows a non-schema error unchanged

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@nbouliol nbouliol force-pushed the feature/prd-673-expose-the-action-form-endpoint branch from 5a335a9 to a916daa Compare July 10, 2026 12:35
Comment thread packages/agent-bff/src/data/data-routes-middleware.ts
@ShohanRahman

Copy link
Copy Markdown
Contributor

Review note — extractRawLayout "drift guard" test is missing

packages/agent-bff/src/action/agent-action-client.ts:36-39

export function extractRawLayout(action: ActionForm): ForestServerActionFormLayoutElement[] {
  const root = action.getLayout() as { layout?: ForestServerActionFormLayoutElement[] };
  return root.layout ?? [];
}

The comment at line 35 states "A layout-shape test guards against drift." — but no such test exists:

  • extractRawLayout is never referenced by name in any test.
  • Every test stubs getLayout() to return { layout: [...] } directly, so they exercise the BFF's own consumption of that shape, never agent-client's real ActionLayoutRoot (whose element array is a protected field reached here via cast).
  • The ?? [] fallback branch is uncovered (branch coverage sits at ~83%).

Not a live bug — agent-client currently always populates that field with an array. But if that internal field is ever renamed, extractRawLayout silently returns [] (empty layout in the response) with zero test failures to catch it — which is exactly what the comment promises is guarded.

Suggested fix (either):

  • Add a real test that instantiates agent-client's ActionLayoutRoot with a known layout and asserts extractRawLayout unwraps it correctly (closes the gap the comment claims), or
  • Soften the comment to state the guard doesn't exist yet.

🤖 From a multi-agent review pass (code-reviewer, comment-analyzer, silent-failure-hunter, pr-test-analyzer all converged on this).

…tionLayoutRoot

The comment claimed a layout-shape test guarded against agent-client
renaming ActionLayoutRoot's element field, but none existed. Add a test
that builds a real ActionLayoutRoot and asserts extractRawLayout unwraps
it, plus the empty-layout fallback branch.

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

Copy link
Copy Markdown
Member Author

Addressed in ae6772e — took the first option (real drift guard).

Added test/action/extract-raw-layout.test.ts:

  • builds a real agent-client ActionLayoutRoot and asserts extractRawLayout unwraps its element array — so if that internal field is renamed, .layout reads undefined, the fallback returns [], and the assertion fails. That is the drift the comment promised.
  • covers the ?? [] fallback branch explicitly.

Also tightened the code comment to name the guarding test. Full suite green (618 tests, run in-band).

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