Skip to content

[5.x] Let asset_folders_list filter to root folders - #20464

Open
brandonkelly wants to merge 2 commits into
5.xfrom
bugfix/mcp-root-asset-folders
Open

brandonkelly wants to merge 2 commits into
5.xfrom
bugfix/mcp-root-asset-folders

Conversation

@brandonkelly

@brandonkelly brandonkelly commented Nov 7, 2026 •

Copy link
Copy Markdown
Member

Summary

asset_folders_list couldn't find a volume's root folder by query, because criteria.parentId only accepted integers. Agents need that folder's ID to list a volume's top-level folders, or to create one with asset_folders_create, which requires parentId.

 asset_folders_list criteria
-  parentId: integer                 // null -> -32602 "Expected `integer`, but received `null`"
+  parentId: integer | null          // null -> volumes' root folders

FolderCriteria ignores a null parentId, so findFolders() would have returned every folder. null is therefore mapped to :empty:, the same match that Assets::getRootFolderByVolumeId() uses:

list(criteria)
  if criteria has parentId: null
    criteria.parentId = ":empty:"
  findFolders(criteria), filtered by viewContents

Combine it with volumeId to get one volume's root folder. Then pass that folder's ID as parentId to list the volume's top-level folders. The parentId description says this.

Craft 6: craftcms/cms 6.x has the same integer-only parentId in src/Mcp/Capabilities/AssetFolders.php, so this should be ported.

Evidence

  • Before: asset_folders_list {criteria: {parentId: null}} →
    Invalid parameters for tool 'asset_folders_list': Property '/criteria/parentId': Invalid type. Expected 'integer', but received 'null'.
    After: it returns only root folders 1000 and 1001 from the volume folder fixtures. The subfolder under 1000 is left out.

New AssetFoldersCapabilityTest::testListFiltersToRootFoldersForNullParentId calls the tool through tools/call on the authenticated server, so the input schema applies:

store subfolder (parentId 1000)
tools/call asset_folders_list {criteria: {parentId: null}} -> ids == [1000, 1001]
  • Before (schema integer-only): -32602 Invalid parameters … Expected 'integer', but received 'null'
    Before (without the :empty: mapping): Failed asserting that two arrays are equal. The subfolder was included.
    After: OK (1 test, 2 assertions)

ECS and Rector are clean. PHPStan passes (pre-commit hook).

criteria.parentId now accepts null, which lists volumes' root folders.
Their IDs are needed to list a volume's top-level folders or create new
ones with asset_folders_create. Folder criteria ignore a null parentId,
so it's mapped to :empty:.

Co-authored-by: Amp <amp@ampcode.com>
Amp-Thread-ID: https://ampcode.com/threads/T-c93e4e62-8dbe-4153-b265-a57d6fb57ac8
Copilot AI balanced review requested due to automatic review settings November 7, 2026 22:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The regression test bypasses MCP schema validation, so it would miss a regression that rejects null parentId values and restores the client invalid-parameters error.

1 open issue
What changed in this PR

This PR lets MCP clients query volume root folders with asset_folders_list, providing the parent IDs needed to list or create top-level folders.

Changes:

  • Accepts null for criteria.parentId and maps it to the explicit root-folder filter.
  • Adds a regression test confirming that root-folder queries exclude subfolders.
  • Documents the behavior in the tool schema and changelog.
File Description
tests/​unit/​mcp/​AssetFoldersCapabilityTest.php Tests root-folder filtering with a null parent ID.
src/​mcp/​capabilities/​AssetFolders.php Allows null parent IDs and translates them into root-folder queries.
CHANGELOG.md Announces support for querying root folders.

Review effort: Balanced

'uid' => ['type' => 'string', 'format' => 'uuid'],
'volumeId' => ['type' => 'integer'],
'parentId' => ['type' => 'integer', 'description' => 'Parent folder ID.'],
'parentId' => ['type' => ['integer', 'null'], 'description' => 'Parent folder ID, or null for volumes’ root folders. List a volume’s top-level folders with its root folder’s ID.'],

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in 52b6968: the test now calls asset_folders_list through tools/call on the authenticated server, so the input schema applies. It fails both when parentId reverts to integer-only (-32602 … Expected 'integer', but received 'null') and when the :empty: mapping is removed (the subfolder is included).

Calling AssetFolders::list() directly bypassed the tool's input schema,
so the test wouldn't catch parentId reverting to integer-only.

Co-authored-by: Amp <amp@ampcode.com>
Amp-Thread-ID: https://ampcode.com/threads/T-c93e4e62-8dbe-4153-b265-a57d6fb57ac8

This branch has not been deployed

No deployments
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