test(skills): skip the skill-export capture suite where fd-based walking is unavailable (#5372)

* test(skills): skip the skill-export capture suite where fd-based walking is unavailable

export._capture intentionally rejects every export with 422
skill_export_unsupported on platforms without os.O_NOFOLLOW and dir_fd
directory walking (Windows), so the 40 capture-dependent tests in
test_skill_export.py and test_gateway_skill_export.py fail there. Add a
requires_safe_capture mark mirroring the production feature guard and
apply it to those tests; the 21 tests that only exercise the zip
installer, export lock, or router plumbing keep running.

* test(skills): share requires_safe_capture and cover the blocking-io export suite

Move the mirror of export._capture's platform guard into
tests/support/skill_export_platform.py so the per-file copies cannot
drift, and apply it to blocking_io/test_skill_export.py::test_export_worker_and_response_are_off_loop,
which drives the real exporter and hits the same 422
skill_export_unsupported guard on Windows (the blocking-io suite is
excluded from the default make test run).

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
Shxiao 2026-09-12 15:32:05 +09:00 committed by GitHub
parent f4e740bc49
commit a06688ad82
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 49 additions and 0 deletions

View File

@ -3,6 +3,7 @@
import asyncio
import pytest
from support.skill_export_platform import requires_safe_capture
from app.gateway.skill_export import SkillExportResponse, run_export_work
from deerflow.skills.export import build_skill_export, export_manifest
@ -19,6 +20,7 @@ def storage(tmp_path):
@pytest.mark.asyncio
@requires_safe_capture
async def test_export_worker_and_response_are_off_loop(storage):
manifest, lease = await run_export_work(lambda cancel: export_manifest(storage, "demo", cancel))
lease.release()

View File

@ -0,0 +1,17 @@
"""The platform boundary shared by the skill-export test suites.
``deerflow.skills.export._capture`` feature-detects fd-based directory
walking and otherwise rejects every export with ``422
skill_export_unsupported`` (Windows is the main such platform). Every
suite that drives the real exporter must skip exactly where that guard
fires, and the mirror lives here so the per-file copies cannot drift.
"""
import os
import pytest
requires_safe_capture = pytest.mark.skipif(
not hasattr(os, "O_NOFOLLOW") or os.open not in os.supports_dir_fd or os.scandir not in os.supports_fd,
reason="skill export capture needs os.O_NOFOLLOW and dir_fd directory walking, which this platform does not provide; export_manifest/build_skill_export return 422 skill_export_unsupported there",
)

View File

@ -11,6 +11,7 @@ import pytest
from fastapi import FastAPI
from fastapi.testclient import TestClient
from starlette.requests import ClientDisconnect
from support.skill_export_platform import requires_safe_capture
from app.gateway import skill_export as service
from app.gateway.auth.models import User
@ -46,6 +47,7 @@ def app(tmp_path, monkeypatch):
return app
@requires_safe_capture
def test_manifest_download_and_changed_revision(app):
with TestClient(app) as client:
preview = client.get("/api/skills/custom/demo/export-manifest")
@ -132,6 +134,7 @@ async def test_cancel_drains_worker_and_closes_unclaimed_archive():
lease.release()
@requires_safe_capture
def test_same_name_stays_in_current_user_and_missing_does_not_fall_back(app, monkeypatch):
with TestClient(app) as client:
alice = client.get("/api/skills/custom/demo/export-manifest").json()
@ -200,6 +203,7 @@ async def test_client_disconnect_signals_worker_and_preserves_user_context():
owner.reset(token)
@requires_safe_capture
def test_export_upload_roundtrip_uses_existing_scanner_and_rejects_conflict(app, monkeypatch):
"""Actual public skill, production routes/scanner; only remote model decision stubbed."""
import shutil

View File

@ -2,6 +2,7 @@ import os
import zipfile
import pytest
from support.skill_export_platform import requires_safe_capture
from deerflow.skills import export
from deerflow.skills.storage.local_skill_storage import LocalSkillStorage
@ -16,6 +17,7 @@ def package(tmp_path):
return storage, root
@requires_safe_capture
def test_snapshot_revision_and_zip_roundtrip(package, tmp_path):
storage, root = package
(root / "empty").mkdir()
@ -42,6 +44,7 @@ def test_snapshot_revision_and_zip_roundtrip(package, tmp_path):
@pytest.mark.parametrize("kind", ["symlink", "hardlink", "nested", "reserved", "binary", "collision"])
@requires_safe_capture
def test_structural_blockers(package, kind):
storage, root = package
if kind == "symlink":
@ -65,6 +68,7 @@ def test_structural_blockers(package, kind):
assert manifest["blockers"]
@requires_safe_capture
def test_limits_are_not_truncated(package, monkeypatch):
storage, root = package
monkeypatch.setattr(export, "MAX_ENTRIES", 1)
@ -73,6 +77,7 @@ def test_limits_are_not_truncated(package, monkeypatch):
assert error.value.status == 413
@requires_safe_capture
def test_root_link_and_missing_ownership(package):
storage, root = package
root.rename(root.with_name("other"))
@ -107,6 +112,7 @@ def test_local_writer_waits_for_export_lock(package):
assert finished.is_set()
@requires_safe_capture
def test_snapshot_source_race(package, monkeypatch):
storage, root = package
original = export._walk
@ -122,6 +128,7 @@ def test_snapshot_source_race(package, monkeypatch):
assert error.value.status == 409
@requires_safe_capture
def test_zip_uses_only_snapshot(package, monkeypatch):
storage, root = package
manifest = export.export_manifest(storage, "sample")
@ -143,6 +150,7 @@ def test_zip_uses_only_snapshot(package, monkeypatch):
@pytest.mark.parametrize("limit", ["MAX_FILE_BYTES", "MAX_TOTAL_BYTES", "MAX_PATH_BYTES", "MAX_DEPTH"])
@requires_safe_capture
def test_resource_limits(package, monkeypatch, limit):
storage, root = package
(root / "deep").mkdir()
@ -153,6 +161,7 @@ def test_resource_limits(package, monkeypatch, limit):
assert error.value.status == 413
@requires_safe_capture
def test_zip_limit_and_cancellation_close_files(package, monkeypatch):
import threading
@ -180,6 +189,7 @@ def test_zip_limit_and_cancellation_close_files(package, monkeypatch):
assert all(file.closed for file in files)
@requires_safe_capture
def test_diagnostics_requirements_do_not_expose_contents(package):
import json
@ -193,6 +203,7 @@ def test_diagnostics_requirements_do_not_expose_contents(package):
@pytest.mark.parametrize("skill_name", ["code-documentation", "skill-creator"])
@requires_safe_capture
def test_public_skill_real_install_roundtrip(tmp_path, monkeypatch, skill_name):
import shutil
from pathlib import Path
@ -250,6 +261,7 @@ def test_import_permissions_from_independent_zip(tmp_path, mode, expected):
assert (tmp_path / "script").stat().st_mode & 0o7777 == expected
@requires_safe_capture
def test_cross_process_lock_timeout(package, monkeypatch):
import subprocess
import sys
@ -269,6 +281,7 @@ def test_cross_process_lock_timeout(package, monkeypatch):
child.stdout.close()
@requires_safe_capture
def test_user_read_lock_no_projection_mutation_and_owned_only(tmp_path, monkeypatch):
from deerflow.config.paths import Paths
from deerflow.skills.projection import get_skill_projection_paths
@ -341,6 +354,7 @@ def test_temp_creation_and_cleanup_within_mutation(tmp_path, monkeypatch, user_s
assert not active
@requires_safe_capture
def test_ancestor_symlink_is_not_followed(package, tmp_path):
storage, root = package
# A user scope ancestor is also an ownership boundary, even if custom itself is real.
@ -353,6 +367,7 @@ def test_ancestor_symlink_is_not_followed(package, tmp_path):
assert manifest["blockers"][0]["code"] == "skill_export_link"
@requires_safe_capture
def test_nofollow_file_replacement_is_changed(package, monkeypatch):
storage, root = package
original = export.os.open
@ -373,6 +388,7 @@ def test_nofollow_file_replacement_is_changed(package, monkeypatch):
assert error.value.status == 409
@requires_safe_capture
def test_large_body_and_bounded_frontmatter(package):
storage, root = package
header = (root / "SKILL.md").read_bytes()
@ -384,6 +400,7 @@ def test_large_body_and_bounded_frontmatter(package):
assert error.value.status == 413
@requires_safe_capture
def test_invalid_utf8_in_large_body_is_blocked(package):
storage, root = package
(root / "SKILL.md").write_bytes((root / "SKILL.md").read_bytes() + b"x" * (1024 * 1024 + 1) + b"\xff")
@ -391,6 +408,7 @@ def test_invalid_utf8_in_large_body_is_blocked(package):
@pytest.mark.parametrize("name", ["con", "aux", "com1"])
@requires_safe_capture
def test_windows_reserved_root_is_blocked(tmp_path, name):
storage = LocalSkillStorage(host_path=str(tmp_path))
root = storage.get_custom_skill_dir(name)
@ -399,6 +417,7 @@ def test_windows_reserved_root_is_blocked(tmp_path, name):
assert not export.export_manifest(storage, name)["can_export"]
@requires_safe_capture
def test_required_secrets_normalization_and_invalid_warning(package):
storage, root = package
(root / "SKILL.md").write_text('---\nname: sample\ndescription: Example\nrequired-secrets:\n - " API_KEY "\n - name: API_KEY\n - name: " OPTIONAL_KEY "\n optional: true\n - name: []\n - 123\n---\n', encoding="utf-8")
@ -407,6 +426,7 @@ def test_required_secrets_normalization_and_invalid_warning(package):
assert any(warning["code"] == "skill_export_invalid_declaration" for warning in manifest["warnings"])
@requires_safe_capture
def test_mtime_does_not_change_revision(package):
storage, root = package
before = export.export_manifest(storage, "sample")["revision"]
@ -414,6 +434,7 @@ def test_mtime_does_not_change_revision(package):
assert export.export_manifest(storage, "sample")["revision"] == before
@requires_safe_capture
def test_deep_yaml_is_bounded_without_source_diagnostics(package):
storage, root = package
(root / "SKILL.md").write_text("---\nname: sample\ndescription: " + "[" * 5000 + "secret" + "]" * 5000 + "\n---\n", encoding="utf-8")
@ -422,6 +443,7 @@ def test_deep_yaml_is_bounded_without_source_diagnostics(package):
assert "secret" not in str(manifest["blockers"])
@requires_safe_capture
def test_invalid_unicode_nodes_still_consume_entry_budget(package, monkeypatch):
from contextlib import contextmanager
from types import SimpleNamespace
@ -449,6 +471,7 @@ def test_invalid_unicode_nodes_still_consume_entry_budget(package, monkeypatch):
@pytest.mark.parametrize("metadata", ["metadata: {base: &base {x: 1}, copy: *base}", "description: &text Example\nmetadata: {copy: *text}"])
@requires_safe_capture
def test_yaml_alias_is_rejected_before_constructor(package, monkeypatch, metadata):
storage, root = package
(root / "SKILL.md").write_text("---\nname: sample\ndescription: Example\n" + metadata + "\n---\n", encoding="utf-8")
@ -463,6 +486,7 @@ def test_yaml_alias_is_rejected_before_constructor(package, monkeypatch, metadat
assert "aliases" in manifest["blockers"][0]["message"]
@requires_safe_capture
def test_yaml_merge_alias_bomb_is_blocked_without_expansion(package):
storage, root = package
levels = [" a0: &a0 {x: 1}"]
@ -475,6 +499,7 @@ def test_yaml_merge_alias_bomb_is_blocked_without_expansion(package):
@pytest.mark.parametrize("kind", ["depth", "events"])
@requires_safe_capture
def test_yaml_structural_budget_precedes_constructor(package, monkeypatch, kind):
storage, root = package
extra = "metadata: " + "[" * 40 + "x" + "]" * 40 if kind == "depth" else "metadata: [" + ",".join("x" for _ in range(20)) + "]"
@ -491,6 +516,7 @@ def test_yaml_structural_budget_precedes_constructor(package, monkeypatch, kind)
assert manifest["blockers"][0]["code"] == "skill_export_yaml_complexity"
@requires_safe_capture
def test_yaml_event_preflight_observes_cancellation(package, monkeypatch):
import threading