Skip to content

fix(agent-client): respect action hooks configuration (load/change)#1541

Open
Scra3 wants to merge 6 commits intomainfrom
fix/respect-action-hooks-config
Open

fix(agent-client): respect action hooks configuration (load/change)#1541
Scra3 wants to merge 6 commits intomainfrom
fix/respect-action-hooks-config

Conversation

@Scra3
Copy link
Copy Markdown
Member

@Scra3 Scra3 commented Apr 10, 2026

Summary

  • loadInitialState was called unconditionally even for actions with hooks.load: false, causing 404 errors on backends that reject the /hooks/load request
  • setFieldValue always called /hooks/change even when hooks.change was empty, causing 404 errors
  • Pass hooks config from the schema through ActionEndpointsByCollectionCollectionFieldFormStates
  • Skip the HTTP calls when hooks are disabled
  • Backward compatible: when hooks is not provided (old schema format), behavior is unchanged

Test plan

  • All 285 agent-client tests pass (4 new tests)
  • All 520 mcp-server tests pass
  • Verify actions with hooks.load: false no longer trigger /hooks/load
  • Verify actions with hooks.change: [] no longer trigger /hooks/change

🤖 Generated with Claude Code

Note

Fix action hooks configuration to skip load/change requests when hooks are absent or empty

  • FieldFormStates.setFieldValue no longer calls the /hooks/change endpoint when hooks.change is empty, avoiding unnecessary requests.
  • FieldFormStates.loadInitialState swallows 404 errors from /hooks/load when hooks.load is false, optionally populating fields from fallbackFields instead of throwing.
  • Collection.action now passes hooks and fields from the action schema through to FieldFormStates, using a new getActionInfo() method that replaces getActionPath().
  • HttpRequester gains a static is404Error(error) helper and normalizes paths missing a leading slash in buildUrl().
  • SchemaConverter.extractActionEndpoints and getActionEndpoints in the MCP server now include hooks and fields in their returned action metadata.

Macroscope summarized 5944ae6.

@qltysh
Copy link
Copy Markdown

qltysh Bot commented Apr 10, 2026

1 new issue

Tool Category Rule Count
qlty Structure Function with many parameters (count = 7): constructor 1

@qltysh
Copy link
Copy Markdown

qltysh Bot commented Apr 10, 2026

Qlty

Coverage Impact

This PR will not change total coverage.

Modified Files with Diff Coverage (3)

RatingFile% DiffUncovered Line #s
Coverage rating: A Coverage rating: A
packages/agent-client/src/action-fields/field-form-states.ts93.3%122
Coverage rating: A Coverage rating: A
packages/agent-client/src/domains/collection.ts100.0%
Coverage rating: A Coverage rating: A
packages/agent-client/src/http-requester.ts100.0%
Total95.5%
🤖 Increase coverage with AI coding...

In the `fix/respect-action-hooks-config` branch, add test coverage for this new code:

- `packages/agent-client/src/action-fields/field-form-states.ts` -- Line 122

🚦 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.

alban bertolini and others added 2 commits April 10, 2026 14:22
When api_endpoint has no trailing slash and action endpoints have no
leading slash, the URL concatenation produces broken paths like
/backoffice/v1forest/actions/... instead of /backoffice/v1/forest/actions/...

Extract a buildUrl() method that ensures the path always starts with /
before concatenation.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
loadInitialState was called unconditionally, even for actions with
hooks.load: false, causing 404 errors. Similarly, setFieldValue always
called /hooks/change even when hooks.change was empty.

Pass hooks config from the schema through to FieldFormStates, and skip
the HTTP calls when the hooks are disabled.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Scra3 Scra3 force-pushed the fix/respect-action-hooks-config branch from 189a6c6 to c8216e9 Compare April 10, 2026 12:36
alban bertolini and others added 3 commits April 10, 2026 14:58
The Node agent responds to POST /hooks/load even when hooks.load is
false (returning static fields). The Ruby agent returns 404.

Instead of skipping the call entirely (which breaks Node), always
attempt it and gracefully catch the error when hooks.load is false.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The catch block was too broad — it swallowed all errors (401, 500,
network failures) when hooks.load was false. Now it only catches 404
specifically, matching the Ruby backend behavior. Other errors are
rethrown so they surface properly.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When hooks.load is false and the Ruby backend returns 404, use the
static fields from the schema as fallback instead of returning an
empty form. This ensures the AI knows which fields to fill.

Also moves is404Error to HttpRequester (where HTTP errors are created)
and narrows the catch to only swallow 404 errors.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Medium

In loadChanges, this.layout.push(...queryResults.layout) spreads queryResults.layout without a null-coalescing fallback. If the /hooks/change endpoint returns a response without a layout property, this throws a TypeError. The same operation in loadInitialState uses queryResults.layout ?? [] — the inconsistency suggests this is an oversight.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file packages/agent-client/src/action-fields/field-form-states.ts around line 167:

In `loadChanges`, `this.layout.push(...queryResults.layout)` spreads `queryResults.layout` without a null-coalescing fallback. If the `/hooks/change` endpoint returns a response without a `layout` property, this throws a TypeError. The same operation in `loadInitialState` uses `queryResults.layout ?? []` — the inconsistency suggests this is an oversight.

Evidence trail:
packages/agent-client/src/action-fields/field-form-states.ts lines 102: `this.layout.push(...(queryResults.layout ?? []));` in loadInitialState method. packages/agent-client/src/action-fields/field-form-states.ts line 170: `this.layout.push(...queryResults.layout);` in loadChanges method. Both methods query similar ResponseBody type from HTTP requests to /hooks/load and /hooks/change endpoints respectively.

ForestSchemaAction.fields is typed as { field: string }[] but the
actual JSON contains type, isRequired, defaultValue. Cast to the
expected type until the ForestSchemaAction type is updated.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.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.

1 participant