* review-stack 1/4: code (37 files, +3217/-3958)
Review-and-land stack for synap5e/feat/asset-record-content-split, generated by review-stack.py. Once approved,
merges DOWN into the layer below (a fast-forward); only the bottom layer
squash-merges into the real base. See ~/adocs/review-stack.md.
Rule: path not under tests-unit/ or tests/
Question: Is the logic change right?
Source tip: 7007d18582
Merge-base: 783545f689
* review-stack 2/4: tests-removed (24 files, +274/-8220)
Review-and-land stack for synap5e/feat/asset-record-content-split, generated by review-stack.py. Once approved,
merges DOWN into the layer below (a fast-forward); only the bottom layer
squash-merges into the real base. See ~/adocs/review-stack.md.
Rule: test file deleted, or modified with deleted/(added+deleted) >= 0.9
Question: For each dropped assertion: obsolete by a ruling, or covered by a tests-new test?
Source tip: 7007d18582
Merge-base: 783545f689
* review-stack 3/4: tests-changed (13 files, +1043/-1218)
Review-and-land stack for synap5e/feat/asset-record-content-split, generated by review-stack.py. Once approved,
merges DOWN into the layer below (a fast-forward); only the bottom layer
squash-merges into the real base. See ~/adocs/review-stack.md.
Rule: remaining modified test files (incl. conftest.py / helpers)
Question: Did the edits weaken an existing check?
Source tip: 7007d18582
Merge-base: 783545f689
* review-stack 4/4: tests-new (46 files, +8601/-0)
Review-and-land stack for synap5e/feat/asset-record-content-split, generated by review-stack.py. Once approved,
merges DOWN into the layer below (a fast-forward); only the bottom layer
squash-merges into the real base. See ~/adocs/review-stack.md.
Rule: test file added
Question: Is the code layer well covered?
Source tip: 7007d18582
Merge-base: 783545f689
* review-stack 5/6: code (13 files, +351/-104)
Review-and-land stack for synap5e/feat/assets-di, generated by review-stack.py. Once approved,
merges DOWN into the layer below (a fast-forward); only the bottom layer
squash-merges into the real base. See ~/adocs/review-stack.md.
Rule: path not under tests-unit/ or tests/
Question: Is the logic change right?
Source tip: eca2c74bff
Merge-base: 20d59d2a5f
* review-stack 6/6: tests (8 files, +753/-238)
Review-and-land stack for synap5e/feat/assets-di, generated by review-stack.py. Once approved,
merges DOWN into the layer below (a fast-forward); only the bottom layer
squash-merges into the real base. See ~/adocs/review-stack.md.
Rule: every changed file under tests-unit/ or tests/ (added, modified, or deleted)
Question: Is the code layer well covered, and did any edit weaken an existing check?
Source tip: eca2c74bff
Merge-base: 20d59d2a5f
* review-stack 7/8: ported-fixes (42 files, +1361/-180)
Review-and-land stack for synap5e/feat/assets-di-v2, generated by
review-stack.py conventions (hand-built continuation layer; see the PR body).
Once approved, merges DOWN into the layer below (a fast-forward); only the
bottom layer squash-merges into the real base. See ~/adocs/review-stack.md.
Rule: the 11 base-branch fix/docs commits 595cd6e4..94d7185b cherry-picked across the DI refactor (7efdd1d7 excluded, superseded by layer 8)
Question: was each base fix ported faithfully across the DI refactor?
Source tip: 6841881069284803b902b4a9e33bdcda13126771
Merge-base: 7fdfb40f4b
* review-stack 8/8: defensive-parity (4 files, +36/-3)
Review-and-land stack for synap5e/feat/assets-di-v2, generated by
review-stack.py conventions (hand-built continuation layer; see the PR body).
Once approved, merges DOWN into the layer below (a fast-forward); only the
bottom layer squash-merges into the real base. See ~/adocs/review-stack.md.
Rule: match-or-improve master's dependency defenses — NoAssets selection when DB deps unavailable (7efdd1d7's outcome via the DI seam), requirements warning before assets imports, blake3 in the guarded dependency set
Question: does each degradation path now match or improve master's behavior?
Source tip: ebc2cfeebc
Merge-base: 7fdfb40f4b
* fix(assets): only discard content rows this operation actually inserted
CR-9: Enumerated all six create_content call sites. Only scanner seeding and the three ingest registration paths track IDs for failure cleanup.
* fix(assets): reject hash-only uploads with FEATURE_DISABLED when hashing is off
CodeRabbit finding CR-2: reject hash-only multipart uploads before create_from_hash when hashing is disabled.
* fix(assets): seed persists the stat it verified
CR-7: persist the fresh seed-time restat instead of walk-time spec values.
* fix(assets): route database lock failures to the lock guidance
CR-16: route file-lock startup failures through the existing lock guidance and exit path.
* fix(assets): drop the inaccurate temp-cleanup claim from the shutdown warning
References CR-10.
* fix(assets): walk the output root after execution so undeclared outputs register promptly
Custom nodes that write files into the output directory without declaring
them in output_ui only became assets when the next full walk happened - a
frontend GET /object_info or a restart. Headless and API-only sessions never
trigger either, so those files never converged into the asset database.
The post-execution hook now requests a FULL scan of the output root instead of
an enrich-only pass. The seeder's pending-request queue was generalised from
enrich-specific to carrying a scan phase, so the request starts immediately
when the seeder is idle and coalesces (escalating to FULL on a phase mismatch)
when a scan is already running. queue_output_enrichment is renamed to
queue_output_scan across the protocol, the NoAssets no-op and the call site.
References FIX-6.
* chore(assets): remove seeder paths orphaned by the output-scan change
45c2f96e rerouted both former enrich call sites to start()/enqueue_scan(),
leaving two seeder methods that look live but are not. Review round F2
raised this along with four smaller items; the user's disposition was to
fix all six here.
- Delete start_enrich: zero callers repo-wide after 45c2f96e.
- Delete enqueue_enrich: no production callers; its ~18 call sites in
tests/test_asset_seeder.py move to enqueue_scan(phase=ScanPhase.ENRICH)
with their semantics unchanged. The deletion forces the half-done class
renames (TestEnqueueEnrich* -> TestEnqueueScan*, consistent with the
already-renamed TestPendingScanDrain) and restores the module docstring
that was dropped rather than reworded.
- Document at manager.queue_output_scan that ScanPhase.FULL per debounce
window is the deliberate, user-ratified trade, so it is not optimised
back to ENRICH without revisiting the decision.
- Document that SeedAssetSpec.size_bytes/mtime_ns are walk-time
diagnostics only - production persists the seed-time restat since CR-7.
- Export create_content_reporting_insert from the queries facade and fold
scanner.py's direct-module import into the existing facade block.
- Harden test_queue_output_scan_does_not_duplicate_declared_output against
a vacuous pass: it now asserts the seeder finished without errors and
that an undeclared sibling written into the same directory WAS
registered by the same scan, proving the walk actually ran.
No production behaviour changes beyond the two deletions.
References F2-cleanup.
* chore: comment cleanup
Comment-Gate: 18 quarantined
* fix(assets): preserve pause across the seeder's pending-scan drain
pause() runs before every prompt, while pending-scan enqueue and resume only run inside the debounced gc-interval gate. If the active scan finishes just after the next prompt's pause, its finally block resets the seeder to idle and the pending drain starts a replacement with the run gate open, so resume becomes a no-op.
Capture pausedness under the lock before resetting to idle, then start the drained scan already paused. Setting the state and gate before launching the thread avoids the start-then-reclear window and lets resume release the existing scan checkpoints.
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
* test(assets): pin job_id absence for scan-discovered assets
Owner ruling, recorded 2026-09-03 in the stack-9-hardening planning notepad: scan-discovered assets — including undeclared outputs found by the post-execution walk — carry job_id = None, always; only emission-time registration (output_ui declaration) attributes a job; attributing walk finds to the most recent prompt would be a temporal-correlation guess that is wrong exactly when prompts interleave; None is honest provenance. Do NOT add proximity-based attribution heuristics to the scanner. Ratified against Jacob Segal's cross-job-attribution concern (2026-09-08 review meeting) — a wrongly-attributed asset could mean one user's cloud job sees another user's asset.
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
* [review-stack 10/10] assets-tests (#16218)
* test(execution): run the battery with assets enabled and assert asset-system health at teardown
* test(execution): cover list-shaped outputs registering assets
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
---------
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
* [review-stack 11/11] review-fixes (#16261)
* fix(assets): only exit on database file-lock timeout when assets are enabled
* test(assets): pin live_contents_under_prefixes path-filtering semantics
* perf(assets): push live-content prefix filtering into SQL
* test(assets): declare per-entry intent in the path-prefix corpus
* test(assets): normalize POSIX-literal path expectations for Windows
* test(assets): force observable stat changes and close-before-mutate on Windows-sensitive rewrites
* test(assets): force an observable mtime change in the hash-mode split test
---------
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Co-authored-by: guill <jacob.e.segal@gmail.com>
The frontend's partner-nodes run gate (Comfy-Org/ComfyUI_frontend#15248)
reads this key from /features as its kill switch. Registering it makes
--feature-flag partner_run_gate_enabled=false coerce to a real boolean;
unregistered keys pass through as strings, and the string "false" is
truthy to the frontend, so the switch would silently not work.
* feat(assets): add namespaced model type tags
* fix(assets): mark path-derived upload tags automatic
* fix(assets): merge duplicate scan specs
* test(assets): make duplicate path normalization portable
* feat(assets): add loader_path as the authoritative loader locator (#14796)
* fix(assets): filter model_type tags by bucket extension sets
Buckets sharing a base directory (e.g. diffusion_models and a custom
unet_gguf) tagged every file in the directory regardless of whether the
bucket could load it, so .safetensors files were tagged
model_type:unet_gguf and vice versa. Carry each bucket's registered
extension set through get_comfy_models_folders and only emit a
model_type tag when the file extension matches, keeping the empty-set
match-all convention from folder_paths.filter_files_extensions.
Files under a model base matching no bucket now keep only the models
tag instead of every directory-matching model_type tag.
* feat(assets): replace response file_path with persisted loader_path
The old file_path response field was a namespaced storage locator
(models/checkpoints/foo.safetensors): not an absolute path, not unique
identity, and not the value a loader consumes. Nothing needs that shape
on the wire (hash/ID-based locating is the long-term direction), so it
is dropped rather than renamed; the storage-root matching stays internal,
powering display_name.
What loaders DO need is the in-root loader path (category dropped:
models/checkpoints/foo/bar.safetensors -> foo/bar.safetensors). Serve it
as a first-class loader_path field, persisted on asset_references
(migration 0006) and written by every ingest pipeline at insert, so
responses read the column verbatim.
Like the model_type tags, loader_path is a seed-time derivative of the
model folder registry, maintained by the same scan lifecycle (new files seed
fresh values, pruning retires rows whose bucket disappeared). Rows
predating the column serve a null loader_path; databases from before
this stack already need recreating for the base branch's tag changes.
loader_path resolves every registered base including extra_model_paths
entries; display_name only the canonical storage roots. A file can
therefore be loadable with no display name (extra-path models) or the
reverse (unregistered files under the models root), and loader_path is
null exactly when no loader can resolve the file.
* test(assets): lock loader_path matrix (asymmetry, null, persist/read)
Cover the behaviour that has no production change but is easy to regress:
the extra-path asymmetry (loadable but no storage namespace), null
loader_path persistence for orphan files, and the response reading the
stored column with a compute fallback for un-backfilled rows.
* fix(assets): persist subfolder-qualified loader_path for ingested outputs
ingest_existing_file built its seed spec with the file's basename, so
outputs saved into a subfolder persisted loader_path (and the
user_metadata filename that preview URLs split for their subfolder
param) as just the basename: the served locator pointed at a file that
does not exist at that path. Scanner and seeder specs already derive
fname via compute_loader_path; use the same derivation here.
* fix(assets): only extension-matching buckets contribute a loader_path
The model-base match in get_asset_category_and_relative_path ignored
each bucket's extension set, so a file inside a registered base whose
extension the bucket cannot load (e.g. a .txt uploaded into
model_type:checkpoints) advertised a loader_path that no loader list
would ever resolve, while the tag side of the same stack already
excluded it. Apply the extension check used for backend tags (empty set
accepts any extension), keeping loader_path null exactly when no loader
can resolve the file.
* fix(assets): refresh loader_path when re-ingesting an existing reference
upsert_reference only wrote loader_path on the INSERT branch, so
re-ingesting an existing reference (an output overwritten in place, or a
file re-registered after its loader_path derivation changed) kept the
stale or NULL value forever. Write it on the UPDATE branch too, with a
null-safe change guard so a loader_path difference alone is enough to
trigger the update, and identical values stay a no-op.
* fix(assets): repair semantic merge breakage from #14796 and master
Two textually-clean but semantically-broken merges:
- routes.py lost its folder_paths import when #14796's import block
superseded the base's, while the content-type hardening added via the
base's master merge still calls folder_paths.is_dangerous_content_type.
- master's SVG download-hardening test uploads with the pre-namespacing
bare checkpoints tag, which this branch's destination validation
rejects; use model_type:checkpoints.
---------
Co-authored-by: guill <jacob.e.segal@gmail.com>
* Support for async execution functions
This commit adds support for node execution functions defined as async. When
a node's execution function is defined as async, we can continue
executing other nodes while it is processing.
Standard uses of `await` should "just work", but people will still have
to be careful if they spawn actual threads. Because torch doesn't really
have async/await versions of functions, this won't particularly help
with most locally-executing nodes, but it does work for e.g. web
requests to other machines.
In addition to the execute function, the `VALIDATE_INPUTS` and
`check_lazy_status` functions can also be defined as async, though we'll
only resolve one node at a time right now for those.
* Add the execution model tests to CI
* Add a missing file
It looks like this got caught by .gitignore? There's probably a better
place to put it, but I'm not sure what that is.
* Add the websocket library for automated tests
* Add additional tests for async error cases
Also fixes one bug that was found when an async function throws an error
after being scheduled on a task.
* Add a feature flags message to reduce bandwidth
We now only send 1 preview message of the latest type the client can
support.
We'll add a console warning when the client fails to send a feature
flags message at some point in the future.
* Add async tests to CI
* Don't actually add new tests in this PR
Will do it in a separate PR
* Resolve unit test in GPU-less runner
* Just remove the tests that GHA can't handle
* Change line endings to UNIX-style
* Avoid loading model_management.py so early
Because model_management.py has a top-level `logging.info`, we have to
be careful not to import that file before we call `setup_logging`. If we
do, we end up having the default logging handler registered in addition
to our custom one.
Simon Pinfold