fix(artifacts): skip symlinks when sealing cache artifacts

[AndyDai-nv]

Problem

Artifact packaging raised on the first symlink found under a cache root:

def _reject_symlinked_source_entries(source_root: Path) -> None:
    for path in source_root.rglob("*"):
        if path.is_symlink():
            raise ValueError(f"tarred artifact source contains symlink: {path}")

A user running Nemotron-3-Ultra on B200 (vLLM 0.27.1, disagg) hit this and read it as a harmless warning. It is not: prepare_source() throws, PublisherThread retries every 5s until MX_ARTIFACT_READY_TIMEOUT_SECS (default 1800s) and gives up, so that worker never publishes its FlashInfer artifact at all — the JIT-compiled modules the artifact exists to share go with it, and every decode worker recompiles them itself. Weight transfer is unaffected (140 GB in 8.84s in their logs); this is only the artifact path.

WARNING [publisher.py:278] [Worker 2] Source publish attempt failed (121s elapsed,
timeout=1800s), will retry next tick: tarred artifact source contains symlink:
~/.cache/flashinfer/0.6.16.post3/100a/generated/trtllm_export/fused_moe_trtllm_sm100/
flashinfer/trtllm/batched_gemm/trtllmGen_bmm_export

This is not an edge case on Blackwell. FlashInfer's trtllm-gen kernels ship as
prebuilt cubins plus headers stored content-addressed under a hash directory,
while generated .cu files include them from a fixed layout, so
jit/fused_moe.py bridges the two with a symlink on every JIT module lookup.
Any SM100 worker using trtllm-gen fused MoE hits it.

Fix

Enumerate archive members explicitly and hand tar that list, instead of letting
tar walk the tree:

tar -cf <tar> -C <src> --null --no-recursion -T <member list>

Symlinks are left out of the list. They are derived state that engines rebuild:
FlashInfer's ensure_symlink() removes whatever sits at the path (including a
real directory, via shutil.rmtree) and recreates the link, and it runs before
build_and_load()'s cache-hit check, so it executes in every process that
touches the op. A link carried across pods would be deleted and recreated
anyway.

Skipping a symlink costs the link entry alone. Neither os.walk(followlinks=False)
nor tar descends through a symlinked directory, so no subtree that tar would
otherwise have archived is lost.

Logging is graded and never fatal: a link resolving inside the cache root logs
at debug (its target is archived under its real path); a link that leaves the
root or dangles logs one warning naming the paths.

Why an explicit member list rather than --exclude

tar reads exclude patterns as globs, so an entry whose name contains glob
metacharacters slips past its own exclusion:

$ ln -s /nowhere 'src/gen/tricky[a]link'
$ tar -cf o.tar -C src --exclude='./gen/tricky[a]link' .
$ tar -tf o.tar
./gen/tricky[a]link          # not excluded

That symlink would then reach the target and fail _validate_tar_members
after a full transfer, turning a source-side problem into a target-side one.
--exclude-from fixes the metacharacter case but still cannot express a name
containing a newline (--null only applies to -T). An explicit member list
is an allowlist: nothing that is not listed can enter the archive, whatever it
is named. That keeps the file-or-directory invariant on the source side, where
it can be enforced, and leaves the extraction path untouched — no new
validation code, no new attack surface.

Other approaches considered

• tar -h (dereference). The bytes would be deleted by the target's
  ensure_symlink(), so it is pure waste. It also breaks on dangling links
  (tar exits 1) and on symlink cycles — GNU tar 1.35 segfaults on
  a/loop -> ../a after expanding 5625 levels.
• Storing symlinks as symlinks. Requires new linkname validation on the
  extraction side for zero benefit, since the target rebuilds the link itself.

Non-regular files (fifo, socket, device nodes) are skipped the same way. They
serve the same invariant: previously the source accepted them and the target
rejected them after a full transfer.

Testing

Replaces test_tarred_p2p_artifact_transfer_rejects_symlink with five cases:
internal symlink, external symlink, broken symlink, awkward member names
(config[sm100].inc and a name containing a newline, alongside symlinks with
the same shapes), and empty-directory preservation.

test_artifact_transfer.py, test_vllm_artifacts.py, test_sglang_artifacts.py,
test_artifact_health_url.py: 119 passed.

Also verified end to end against a reproduction of the reported layout (cubin
include tree under site-packages, cache under
0.6.16.post3/100a/generated/trtllm_export/fused_moe_trtllm_sm100/...): publish
succeeds with one warning naming the skipped link, and the target receives
cached_ops/fused_moe_trtllm_sm100/module.so, the empty cached_ops/tmp, and
the link's parent directory.

Pre-existing unrelated failures in test_vmm_* (the modelexpress.vmm._alloc_ext
C extension is not built in this environment) reproduce identically with these
changes stashed.

Behavior change

Symlinks under a cache root are no longer an error for any artifact type. The
worst case is a target cache missing one derived entry, which degrades to a
recompile — these are all regenerable JIT caches. The current worst case is the
entire cache failing to publish, every time.

Summary by CodeRabbit

• New Features

  • Artifact packaging now safely excludes symlinks and unsupported entries while preserving regular files and empty directories.
  • Archive creation supports filenames containing brackets or newlines.

• Bug Fixes

  • Artifact publication no longer fails when source directories contain symlinks.
  • Broken and external symlinks are skipped with appropriate warnings.

• Documentation

  • Documented artifact packaging, extraction safeguards, and symlink handling behavior.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: a45bef47-bd0f-48a1-9501-39bbe7e47033

📥 Commits

Reviewing files that changed from the base of the PR and between f4fa625 and 9ef9d10.

📒 Files selected for processing (4)

• docs/ARCHITECTURE.md
• docs/DEPLOYMENT.md
• modelexpress_client/python/modelexpress/metadata/artifact_transfer.py
• modelexpress_client/python/tests/test_artifact_transfer.py

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.

---

Walkthrough

Cache artifact packaging now enumerates regular files and directories explicitly. Symlinks and unsupported entries are skipped with bounded diagnostics. Tests cover symlink handling, unusual filenames, and empty directories. Architecture and deployment documentation describe the packaging and extraction rules.

Changes

Cache artifact packaging

Layer / File(s)	Summary
Explicit artifact member packaging docs/ARCHITECTURE.md, docs/DEPLOYMENT.md, modelexpress_client/python/modelexpress/metadata/artifact_transfer.py	Artifact creation archives explicitly enumerated regular files and directories. It skips symlinks and unsupported entries, logs bounded diagnostics, and builds tar archives from a temporary member list. Documentation records the packaging and extraction rules.
Transfer behavior validation modelexpress_client/python/tests/test_artifact_transfer.py	Tests verify symlink omission, warnings for external and broken links, preservation of unusual filenames, and preservation of empty directories and regular files.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: ⚪ Minimal · up to 9ef9d

The change skips derived symlink entries during artifact packaging while preserving required files and directories; no actionable merge-blocking risk remains beyond normal checks and review.

Poem

I hop through the cache with a list in my paw,
Skip every link that should not cross the draw.
Files and dirs rest safe in their tar,
Odd names stay whole, both near and far.
Empty rooms remain—what a tidy burrow!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 2 files. (2 skipped: 2 unsupported.)	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: symlinks are skipped when sealing cache artifacts.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

_{Comment @coderabbitai help to get the list of available commands.}

[AndyDai-nv]

/ok to test dbe8e87