Skip to content

fix(assets): recognize temp directory in asset category resolution#13159

Merged
luke-mino-altherr merged 1 commit intomasterfrom
luke-mino-altherr/fix-temp-file-writing
Mar 26, 2026
Merged

fix(assets): recognize temp directory in asset category resolution#13159
luke-mino-altherr merged 1 commit intomasterfrom
luke-mino-altherr/fix-temp-file-writing

Conversation

@luke-mino-altherr
Copy link
Contributor

Problem

When --enable-assets is active, files written to the temp directory (e.g. GLSLShader_output_00004_.png, ComfyUI_temp_tczip_00004_.png) cause a ValueError in get_asset_category_and_relative_path because the temp directory is not a recognized asset root.

ValueError: Path is not within input, output, or configured model bases: I:\ComfyUI\temp\GLSLShader_output_00004_.png

Fix

Add temp as a recognized category in get_asset_category_and_relative_path (via folder_paths.get_temp_directory()), alongside the existing input, output, and models roots.

Changes

  • app/assets/services/path_utils.py – Add temp directory check in get_asset_category_and_relative_path
  • tests-unit/assets_test/services/test_path_utils.py – New test file covering all category roots including a regression test for the temp directory bug

@coderabbitai
Copy link

coderabbitai bot commented Mar 26, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 563e4481-dcd6-44c8-852c-bfa46fcabd62

📥 Commits

Reviewing files that changed from the base of the PR and between 3cb6066 and 4c111bb.

📒 Files selected for processing (2)
  • app/assets/services/path_utils.py
  • tests-unit/assets_test/services/test_path_utils.py
✅ Files skipped from review due to trivial changes (1)
  • tests-unit/assets_test/services/test_path_utils.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/assets/services/path_utils.py

📝 Walkthrough

Walkthrough

The get_asset_category_and_relative_path() function in path_utils.py is extended to recognize a new asset category temp. Paths within the temporary directory are now classified as "temp", with the classification check placed between output and models. The function signature and return Literal type now include "temp", and the ValueError message for unknown roots was updated. A new test module verifies behavior for input, output, temp, and models.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: adding temp directory recognition to asset category resolution.
Description check ✅ Passed The description clearly explains the problem, the fix, and lists all changed files, directly relating to the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

Files written to the temp directory (e.g. GLSLShader_output_00004_.png)
caused a ValueError in get_asset_category_and_relative_path when
--enable-assets was active, because the temp directory was not a
recognized asset root.

Add 'temp' as a category alongside input/output/models so temp outputs
can be registered as assets.

Amp-Thread-ID: https://ampcode.com/threads/T-019d2800-6944-7039-b2c4-53f809bc7f4f
Co-authored-by: Amp <amp@ampcode.com>
@luke-mino-altherr luke-mino-altherr force-pushed the luke-mino-altherr/fix-temp-file-writing branch from 3cb6066 to 4c111bb Compare March 26, 2026 02:53
@luke-mino-altherr luke-mino-altherr merged commit 3eba2dc into master Mar 26, 2026
16 checks passed
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