Skip to content

fix(artifacts): Support nested API names#6051

Closed
spectaclehong wants to merge 4 commits into
google:mainfrom
spectaclehong:fix/artifact-slash-api
Closed

fix(artifacts): Support nested API names#6051
spectaclehong wants to merge 4 commits into
google:mainfrom
spectaclehong:fix/artifact-slash-api

Conversation

@spectaclehong

Copy link
Copy Markdown
Contributor

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

Problem:

Artifact services can save and list logical artifact names containing /, such as reports/summary.txt. The REST API also accepts these names through POST /artifacts because the filename is provided in the request body.

However, read/version/metadata/delete routes treated artifact_name as a single path segment. That made nested artifact names visible in the list endpoint but unreachable through the path-based endpoints.

Solution:

Update artifact read/version/metadata/delete routes to use FastAPI's {artifact_name:path} converter so nested logical artifact names can round-trip through the REST API.

Because {artifact_name:path} is catch-all, the version-specific routes are registered before the general artifact load route. No response schema changes are intended: endpoints that returned types.Part still return types.Part, and metadata endpoints still return ArtifactVersion / list[ArtifactVersion].

Also update artifact URI parsing so artifact references can resolve nested filenames such as folder/file.txt.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

Targeted artifact API tests pass locally:

uv run pytest tests/unittests/cli/test_fast_api.py -k artifact -q

Result:

9 passed, 72 deselected

Artifact service and URI tests pass locally:

uv run pytest tests/unittests/artifacts/test_artifact_util.py tests/unittests/artifacts/test_artifact_service.py -q

Result:

77 passed

Python 3.10 tox unit suite passes locally:

uv tool run --from tox --with tox-uv tox -e py310

Result:

7045 passed, 21 skipped, 31 xfailed, 9 xpassed

Pre-commit hooks pass for changed files:

uv run pre-commit run --files src/google/adk/cli/api_server.py src/google/adk/artifacts/artifact_util.py tests/unittests/cli/test_fast_api.py tests/unittests/artifacts/test_artifact_util.py tests/unittests/artifacts/test_artifact_service.py

Manual End-to-End (E2E) Tests:

Validated a local REST API round-trip using the ADK API server route stack with InMemoryArtifactService:

  • save reports/summary.txt
  • list artifacts
  • load with raw slash path
  • load with encoded slash path
  • list versions
  • load version 0
  • load version metadata
  • delete with encoded slash path

All requests returned 200.

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules.

Additional context

No dependent changes are required.

Artifact services can save and list logical artifact names containing path separators, but the API routes treated artifact_name as a single path segment. This made nested names unreachable through load, version, metadata, and delete endpoints.

Use path-aware artifact routes after the version-specific endpoints, and allow artifact reference URIs to parse nested filenames.

Fixes google#6050
@adk-bot adk-bot added the services [Component] This issue is related to runtime services, e.g. sessions, memory, artifacts, etc label Jun 10, 2026
@rohityan rohityan self-assigned this Jun 11, 2026
@rohityan

Copy link
Copy Markdown
Collaborator

Hi @spectaclehong , Thank you for your contribution! We appreciate you taking the time to submit this pull request. Your PR has been received by the team and is currently under review. We will provide feedback as soon as we have an update to share.

@rohityan

Copy link
Copy Markdown
Collaborator

Hi @GWeale , can you please review this.

@rohityan rohityan added the needs review [Status] The PR/issue is awaiting review from the maintainer label Jun 11, 2026
@boyangsvl boyangsvl self-assigned this Jun 12, 2026
copybara-service Bot pushed a commit that referenced this pull request Jun 12, 2026
Merge #6051

**Please ensure you have read the [contribution guide](https://github.com/google/adk-python/blob/main/CONTRIBUTING.md) before creating a pull request.**

### Link to Issue or Description of Change

**1. Link to an existing issue (if applicable):**

- Fixes: #6050

**Problem:**

Artifact services can save and list logical artifact names containing `/`, such as `reports/summary.txt`. The REST API also accepts these names through `POST /artifacts` because the filename is provided in the request body.

However, read/version/metadata/delete routes treated `artifact_name` as a single path segment. That made nested artifact names visible in the list endpoint but unreachable through the path-based endpoints.

**Solution:**

Update artifact read/version/metadata/delete routes to use FastAPI's `{artifact_name:path}` converter so nested logical artifact names can round-trip through the REST API.

Because `{artifact_name:path}` is catch-all, the version-specific routes are registered before the general artifact load route. No response schema changes are intended: endpoints that returned `types.Part` still return `types.Part`, and metadata endpoints still return `ArtifactVersion` / `list[ArtifactVersion]`.

Also update artifact URI parsing so artifact references can resolve nested filenames such as `folder/file.txt`.

### Testing Plan

**Unit Tests:**

- [x] I have added or updated unit tests for my change.
- [x] All unit tests pass locally.

Targeted artifact API tests pass locally:

```shell
uv run pytest tests/unittests/cli/test_fast_api.py -k artifact -q
```

Result:

```text
9 passed, 72 deselected
```

Artifact service and URI tests pass locally:

```shell
uv run pytest tests/unittests/artifacts/test_artifact_util.py tests/unittests/artifacts/test_artifact_service.py -q
```

Result:

```text
77 passed
```

Python 3.10 tox unit suite passes locally:

```shell
uv tool run --from tox --with tox-uv tox -e py310
```

Result:

```text
7045 passed, 21 skipped, 31 xfailed, 9 xpassed
```

Pre-commit hooks pass for changed files:

```shell
uv run pre-commit run --files src/google/adk/cli/api_server.py src/google/adk/artifacts/artifact_util.py tests/unittests/cli/test_fast_api.py tests/unittests/artifacts/test_artifact_util.py tests/unittests/artifacts/test_artifact_service.py
```

**Manual End-to-End (E2E) Tests:**

Validated a local REST API round-trip using the ADK API server route stack with `InMemoryArtifactService`:

- save `reports/summary.txt`
- list artifacts
- load with raw slash path
- load with encoded slash path
- list versions
- load version `0`
- load version metadata
- delete with encoded slash path

All requests returned `200`.

### Checklist

- [x] I have read the [CONTRIBUTING.md](https://github.com/google/adk-python/blob/main/CONTRIBUTING.md) document.
- [x] I have performed a self-review of my own code.
- [x] I have commented my code, particularly in hard-to-understand areas.
- [x] I have added tests that prove my fix is effective or that my feature works.
- [x] New and existing unit tests pass locally with my changes.
- [x] I have manually tested my changes end-to-end.
- [x] Any dependent changes have been merged and published in downstream modules.

### Additional context

No dependent changes are required.

Co-authored-by: Bo Yang <ybo@google.com>
COPYBARA_INTEGRATE_REVIEW=#6051 from spectaclehong:fix/artifact-slash-api 1c08a4f
PiperOrigin-RevId: 931342522
@adk-bot

adk-bot commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

Thank you @spectaclehong for your contribution! 🎉

Your changes have been successfully imported and merged via Copybara in commit b99546b.

Closing this PR as the changes are now in the main branch.

@adk-bot adk-bot added the merged [Status] This PR is merged label Jun 12, 2026
@adk-bot adk-bot closed this Jun 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged [Status] This PR is merged needs review [Status] The PR/issue is awaiting review from the maintainer services [Component] This issue is related to runtime services, e.g. sessions, memory, artifacts, etc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Artifact API cannot load artifacts with path separators in names

4 participants