mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-09-14 16:08:41 +00:00
fix: make upload deletion and sandbox creation transactional
This commit is contained in:
parent
86fa061772
commit
16979184ac
@ -1461,12 +1461,12 @@ Multi-file upload with automatic document conversion:
|
||||
- Reuses one conversion worker per request when called from an active event loop
|
||||
- Files stored in thread-isolated directories under the resolving user's bucket (`users/{user_id}/threads/{thread_id}/user-data/uploads`). For IM channels the owner is threaded explicitly via the `user_id=` kwarg (see IM Channels → Owner-scoped file storage); HTTP/embedded callers resolve it from `get_effective_user_id()`
|
||||
- Every ingress stages a complete payload and atomically publishes it without replacing an existing entry. Collisions across requests, processes, HTTP, embedded, and IM adapters use `name.ext`, `name_1.ext`, `name_2.ext`; storage that cannot provide atomic no-replace publication fails explicitly.
|
||||
- Exact-name generation leases use a portable NFC-plus-casefold coordination key with Win32 trailing-dot/space folding, so filesystem aliases cannot bypass an active generation; filenames that Windows cannot represent losslessly are rejected before staging, while exact legacy POSIX basenames remain deletable after upgrade. Legacy deletion has a POSIX-only lease mode for literal backslashes and components made entirely from dots/spaces; it does not weaken new-upload validation. The original filename remains the published name. Publication tries each candidate lease without blocking and treats a busy canonical key as a collision, so same-batch and inverse concurrent batches advance to a UTF-8-bounded `_N` candidate instead of deadlocking while retaining earlier generations; pathological long suffixes fall back to truncating the complete basename. Deletion rejects an inode that moved outside the requested name's lease. Primary deletion and rollback atomically move the selected directory entry into an inode-and-intent-tagged transaction directory under the protected conversion namespace before revalidating its identity, so a pathname replacement between check and removal is restored rather than unlinked. Once a user deletion stages its primary, its persisted crash intent is commit-on-recovery: startup completes removal of that exact tombstone and its unambiguous host conversion rather than resurrecting a primary after remote or companion deletion may already have occurred; a conversion belonging to a replacement primary is preserved. An exception returned to the live caller still rolls the staged primary back before it is surfaced. Failed-upload rollback tombstones use the same discard recovery, while legacy restore-intent transactions retain basename/recovery-name restoration. Neither transaction kind is treated as an ordinary disposable upload stage. Final lease release is the commit point: cancellation newly arriving during release is delayed and swallowed so a committed upload is returned as success rather than an indeterminate cancelled result. Embedded-client finalization attempts every lease release and conversion-pool shutdown independently, logging cleanup failures rather than changing an already-committed response or stranding later leases.
|
||||
- Exact-name generation leases use a portable NFC-plus-casefold coordination key with Win32 trailing-dot/space folding, so filesystem aliases cannot bypass an active generation; filenames that Windows cannot represent losslessly are rejected before staging, while exact legacy POSIX basenames remain deletable after upgrade. Legacy deletion has a POSIX-only lease mode for literal backslashes and components made entirely from dots/spaces; it does not weaken new-upload validation. The original filename remains the published name. Publication tries each candidate lease without blocking and treats a busy canonical key as a collision, so same-batch and inverse concurrent batches advance to a UTF-8-bounded `_N` candidate instead of deadlocking while retaining earlier generations; pathological long suffixes fall back to truncating the complete basename. Deletion rejects an inode that moved outside the requested name's lease. Primary deletion and rollback move the selected primary and its exact generated conversion into one inode-and-intent-tagged transaction directory under the protected conversion namespace before side effects, so a same-name replacement can never inherit the prior generation's Markdown. User deletion starts with restore-on-crash intent; after explicit remote deletion succeeds, a persisted commit marker switches recovery to discard before authoritative host tombstones are removed. A live rollback writes a restore marker before publishing either hard link, so recovery can finish restoring both entries even when the transaction began with discard intent. A partial remote failure is compensated from the staged authoritative bytes before host restoration; if compensation fails, host deletion commits instead of publishing a split generation. Failed-upload rollback tombstones otherwise use discard recovery, while legacy primary-only transactions retain their recorded restore/discard behavior. Neither transaction kind is treated as an ordinary disposable upload stage. Final lease release is the commit point: cancellation newly arriving during release is delayed and swallowed so a committed upload is returned as success rather than an indeterminate cancelled result. Embedded-client finalization attempts every lease release and conversion-pool shutdown independently, logging cleanup failures rather than changing an already-committed response or stranding later leases.
|
||||
- Filenames containing NUL, `<`, `>`, or reserved model-context boundary markers are rejected before staging so accepted filenames and exact virtual paths remain lossless in model-visible upload context. Legacy files discovered on disk are still neutralized when listed.
|
||||
- Gateway HTTP uploads use same-directory `.upload-*.part` staging files. Each active stage holds a cross-process liveness lock under `.upload-conversions/.locks/stages/`; startup cleanup skips held stages and sweeps only crash-orphaned files, while `.upload-delete-<restore|discard>-<inode>-<nonce>.part/` transaction directories are resolved according to their persisted crash intent. Legacy intent-less deletion transactions retain restore behavior. Cancellation during staging creation drains the worker and aborts the returned stage before propagating. Staging files are hidden from upload listings, agent upload context, and sandbox listing/search tools.
|
||||
- Generated Markdown is owned by `user-data/.upload-conversions/<actual-primary-filename>.md` and is omitted from primary upload listings. Deletion holds the generation lease while it removes an explicitly synchronized sandbox primary/conversion and then the authoritative host paths; a remote failure is reported and leaves the host primary intact. It never guesses or deletes a legacy/user-owned `uploads/<stem>.md` sibling. Outline extraction opens one descriptor, verifies its `fstat` against the current exclusive regular directory entry, and uses that same descriptor for both outline and preview reads.
|
||||
- Generated Markdown is owned by `user-data/.upload-conversions/<actual-primary-filename>.md` and is omitted from primary upload listings. Deletion holds the generation lease while it stages both authoritative host paths, removes an explicitly synchronized sandbox primary/conversion, and commits the host transaction. A remote failure is reported and restores the host generation only after every possibly removed remote path has been re-published; failed remote compensation commits host deletion. It never guesses or deletes a legacy/user-owned `uploads/<stem>.md` sibling. Outline extraction opens one descriptor, verifies its `fstat` against the current exclusive regular directory entry, and uses that same descriptor for both outline and preview reads.
|
||||
- Gateway HTTP upload/list/delete handlers offload filesystem work through `deerflow.utils.file_io.run_file_io`, a dedicated ContextVar-preserving file IO executor; only operations that may block waiting for a name lease use the separate lease-wait pool. Work needed by an existing lease holder and non-blocking publication stays on the general pool, so waiters cannot starve conversion, rollback, or release. Cold sandbox-provider construction is also offloaded. Gateway, embedded-client, and IM ingresses share provider-aware publication: mounted providers make the exact host paths sandbox-readable; non-mounted providers acquire the sandbox and synchronize the primary plus generated conversion to their exact virtual paths. Each ingress records attempted remote paths before the write can commit and, on later failure or cancellation, calls the provider-neutral `Sandbox.remove_file()` for those paths before host rollback and lease release; the command fallback requires a per-call unpredictable exact success trailer. Embedded multi-file calls retain every publication and receipt until the whole response is built, then roll back the complete batch on failure. WeChat download publication uses the cancellation-safe async lease adapter, so cancellation drains and rolls back a publication worker that completes late.
|
||||
- Mounted upload paths skip both sandbox acquisition and per-file synchronization. For AIO remote/provisioner deployments this requires an explicit, accurate `sandbox.thread_data_mounts: true` plus a Provisioner advertising a compatible mount contract; every upload/delete/artifact decision waits for any in-flight stale capability probe in a worker thread and uses one atomic snapshot. The same frozen version selects the deterministic sandbox ID, mount set, cache/reuse candidate, create request, and response validation; if the peer changes versions during acquisition, the whole attempt restarts with a new ID. After acquisition, consumers use the immutable contract recorded on that sandbox instance rather than the provider's mutable global capability. Unavailable or legacy results use explicit synchronization and are retried without a Gateway restart. A confirmed legacy peer may create thread sandboxes only for the `default` no-auth user, because its primary user-data mount cannot isolate equal thread IDs across authenticated users; authenticated rolling deployments upgrade the Provisioner first. The Provisioner validates the request before its idempotent fast path, labels Pods with hashed user/thread identity and contract version, stores the exact identity plus a mount-signature annotation, and compares the live Pod specification before reuse. Every remote acquisition path, including active-cache and warm-pool reuse, deliberately replays the idempotent create request instead of trusting GET discovery or a liveness response. Contracts newer than v2 are accepted as forward-compatible, but the response must echo the exact frozen version, user, and thread. The negotiated mount-contract version namespaces deterministic AIO sandbox IDs so a container created under another version cannot satisfy the acquisition; active reuse also replaces its client when validation returns a changed service URL. Reconciliation may still enumerate/adopt the old ID for orphan cleanup; it is not selected for the new identity.
|
||||
- Mounted upload paths skip both sandbox acquisition and per-file synchronization. For AIO remote/provisioner deployments this requires an explicit, accurate `sandbox.thread_data_mounts: true` plus a Provisioner advertising a compatible mount contract; every upload/delete/artifact decision waits for any in-flight stale capability probe in a worker thread and uses one atomic snapshot. The same frozen version selects the deterministic sandbox ID, mount set, cache/reuse candidate, create precondition, and response validation; if the peer changes versions during acquisition, the whole attempt restarts with a new ID. The required version is sent in the create JSON, and the Provisioner rejects a mismatch before any Kubernetes read or write, preventing a rolling upgrade from creating an orphan under the prior version's ID. After acquisition, consumers use the immutable contract recorded on that sandbox instance rather than the provider's mutable global capability. Unavailable or legacy results use explicit synchronization and are retried without a Gateway restart. A confirmed legacy peer may create thread sandboxes only for the `default` no-auth user, because its primary user-data mount cannot isolate equal thread IDs across authenticated users; authenticated rolling deployments upgrade the Provisioner first. The Provisioner validates the request before its idempotent fast path, labels Pods with hashed user/thread identity and contract version, stores the exact identity plus a mount-signature annotation, and compares the live Pod specification before reuse. Every remote acquisition path, including active-cache and warm-pool reuse, deliberately replays the idempotent create request instead of trusting GET discovery or a liveness response. Contracts newer than v2 are accepted as forward-compatible, but the response must echo the exact frozen version, user, and thread; legacy responses may omit the version or return exact integer zero, never strings or booleans. The negotiated mount-contract version namespaces deterministic AIO sandbox IDs so a container created under another version cannot satisfy the acquisition; active reuse also replaces its client when validation returns a changed service URL. Reconciliation may still enumerate/adopt the old ID for orphan cleanup; it is not selected for the new identity.
|
||||
- Agent receives uploaded file list via `UploadsMiddleware`
|
||||
|
||||
See [docs/FILE_UPLOAD.md](docs/FILE_UPLOAD.md) for details.
|
||||
|
||||
@ -407,6 +407,7 @@ class RemoteSandboxBackend(SandboxBackend):
|
||||
"sandbox_id": sandbox_id,
|
||||
"thread_id": thread_id,
|
||||
"user_id": effective_user_id,
|
||||
"required_mount_contract_version": required_mount_contract_version,
|
||||
"include_legacy_skills": include_legacy_skills,
|
||||
"provision_lark_cli_runtime": provision_lark_cli_runtime,
|
||||
"provision_lark_cli_broker": provision_lark_cli_broker,
|
||||
@ -425,6 +426,15 @@ class RemoteSandboxBackend(SandboxBackend):
|
||||
headers=self._auth_headers(),
|
||||
timeout=30,
|
||||
)
|
||||
if getattr(resp, "status_code", 200) == 409:
|
||||
conflict_payload = resp.json()
|
||||
detail = conflict_payload.get("detail") if isinstance(conflict_payload, dict) else None
|
||||
if isinstance(detail, dict) and detail.get("code") == "mount_contract_changed" and type(detail.get("actual")) is int:
|
||||
self._invalidate_capability_snapshot()
|
||||
raise MountContractChangedError(
|
||||
required_mount_contract_version,
|
||||
detail["actual"],
|
||||
)
|
||||
resp.raise_for_status()
|
||||
data = resp.json()
|
||||
if not isinstance(data, dict):
|
||||
@ -452,11 +462,20 @@ class RemoteSandboxBackend(SandboxBackend):
|
||||
)
|
||||
if response_contract != expected_contract:
|
||||
raise RuntimeError(f"Provisioner mount contract response does not match the requested sandbox identity: expected={expected_contract!r}, received={response_contract!r}")
|
||||
elif (type(response_version) is int and response_version != required_mount_contract_version) or (type(response_version) is not int and required_mount_contract_version != 0):
|
||||
elif response_version is None:
|
||||
if required_mount_contract_version != 0:
|
||||
self._invalidate_capability_snapshot()
|
||||
raise MountContractChangedError(
|
||||
required_mount_contract_version,
|
||||
0,
|
||||
)
|
||||
elif type(response_version) is not int:
|
||||
raise RuntimeError("Provisioner mount contract response contains a non-integer version")
|
||||
elif response_version != required_mount_contract_version:
|
||||
self._invalidate_capability_snapshot()
|
||||
raise MountContractChangedError(
|
||||
required_mount_contract_version,
|
||||
response_version if type(response_version) is int else 0,
|
||||
response_version,
|
||||
)
|
||||
logger.info(f"Provisioner created sandbox {sandbox_id}: sandbox_url={data['sandbox_url']}")
|
||||
return SandboxInfo(
|
||||
@ -464,7 +483,7 @@ class RemoteSandboxBackend(SandboxBackend):
|
||||
sandbox_url=data["sandbox_url"],
|
||||
user_id=data.get("user_id"),
|
||||
thread_id=data.get("thread_id"),
|
||||
mount_contract_version=data.get("mount_contract_version"),
|
||||
mount_contract_version=response_version,
|
||||
)
|
||||
except requests.RequestException as exc:
|
||||
logger.error(f"Provisioner create failed for {sandbox_id}: {exc}")
|
||||
|
||||
@ -22,6 +22,7 @@ from deerflow.uploads.layout import (
|
||||
UPLOAD_CONVERSIONS_DIRNAME,
|
||||
_truncate_utf8,
|
||||
artifact_url_for_virtual_path,
|
||||
conversion_path_for_upload,
|
||||
ensure_conversion_dir,
|
||||
existing_conversion_path_for_upload,
|
||||
upload_virtual_path,
|
||||
@ -36,9 +37,16 @@ UPLOAD_STAGING_SUFFIX = ".part"
|
||||
UPLOAD_DELETION_TRANSACTION_PREFIX = ".upload-delete-"
|
||||
_UPLOAD_DELETION_RESTORE_INTENT = "restore"
|
||||
_UPLOAD_DELETION_DISCARD_INTENT = "discard"
|
||||
_UPLOAD_DELETION_CONVERSION_TOMBSTONE = ".conversion"
|
||||
_UPLOAD_DELETION_COMMIT_MARKER = ".commit"
|
||||
_UPLOAD_DELETION_RESTORE_MARKER = ".restore"
|
||||
_WINDOWS_FORBIDDEN_FILENAME_CHARS = frozenset('<>:"|?*')
|
||||
|
||||
|
||||
class RemoteDeletionCommitRequiredError(RuntimeError):
|
||||
"""Remote deletion could not be compensated, so host deletion must commit."""
|
||||
|
||||
|
||||
@dataclass(slots=True)
|
||||
class StagedUpload:
|
||||
"""A complete-or-in-progress upload stored under a hidden temporary name."""
|
||||
@ -470,16 +478,14 @@ def rollback_published_upload(publication: PublishedUpload) -> None:
|
||||
publication.path,
|
||||
publication.identity,
|
||||
recover_on_crash=False,
|
||||
conversion_path=owned_conversion,
|
||||
)
|
||||
except (FileNotFoundError, UnsafeUploadPathError):
|
||||
# The pathname was replaced after the optimistic identity check. The
|
||||
# staging helper restores that replacement without deleting it.
|
||||
return
|
||||
try:
|
||||
if owned_conversion is not None:
|
||||
owned_conversion.unlink(missing_ok=True)
|
||||
staged_path.unlink()
|
||||
_finish_deletion_transaction(staged_path)
|
||||
_discard_staged_deletion(staged_path)
|
||||
except BaseException:
|
||||
_restore_staged_deletion(
|
||||
staged_path,
|
||||
@ -684,43 +690,139 @@ def _restore_staged_deletion(
|
||||
original_path: Path,
|
||||
identity: UploadIdentity,
|
||||
) -> None:
|
||||
"""Restore a staged primary without replacing a newly-created entry."""
|
||||
"""Restore a staged primary and conversion without replacing a new generation."""
|
||||
try:
|
||||
staged_stat = os.lstat(staged_path)
|
||||
except FileNotFoundError:
|
||||
return
|
||||
if not stat.S_ISREG(staged_stat.st_mode) or staged_stat.st_nlink != 1 or (staged_stat.st_dev, staged_stat.st_ino) != (identity.device, identity.inode):
|
||||
raise UnsafeUploadPathError("Staged upload deletion changed identity")
|
||||
_clear_staged_deletion_commit(staged_path)
|
||||
_mark_staged_deletion_restore(staged_path)
|
||||
try:
|
||||
original_stat = os.lstat(original_path)
|
||||
except FileNotFoundError:
|
||||
original_stat = None
|
||||
restored_path: Path
|
||||
if original_stat is not None:
|
||||
if stat.S_ISREG(original_stat.st_mode) and (
|
||||
original_stat.st_dev,
|
||||
original_stat.st_ino,
|
||||
) == (identity.device, identity.inode):
|
||||
staged_path.unlink()
|
||||
_finish_deletion_transaction(staged_path)
|
||||
return
|
||||
recovery_path = _preserve_staged_entry_as_recovery(staged_path, original_path)
|
||||
logger.warning(
|
||||
"Upload name was recreated during deletion rollback; preserved the prior generation as %s",
|
||||
recovery_path,
|
||||
)
|
||||
_finish_deletion_transaction(staged_path)
|
||||
restored_path = original_path
|
||||
else:
|
||||
restored_path = _link_staged_entry_as_recovery(staged_path, original_path)
|
||||
logger.warning(
|
||||
"Upload name was recreated during deletion rollback; preserved the prior generation as %s",
|
||||
restored_path,
|
||||
)
|
||||
else:
|
||||
try:
|
||||
os.link(staged_path, original_path, follow_symlinks=False)
|
||||
except FileExistsError:
|
||||
restored_path = _link_staged_entry_as_recovery(staged_path, original_path)
|
||||
logger.warning(
|
||||
"Upload name was recreated during deletion rollback; preserved the prior generation as %s",
|
||||
restored_path,
|
||||
)
|
||||
else:
|
||||
restored_path = original_path
|
||||
|
||||
_restore_staged_conversion(staged_path, restored_path)
|
||||
staged_path.unlink()
|
||||
_clear_staged_deletion_restore(staged_path)
|
||||
_finish_deletion_transaction(staged_path)
|
||||
|
||||
|
||||
def _staged_conversion_path(staged_path: Path) -> Path:
|
||||
return staged_path.parent / _UPLOAD_DELETION_CONVERSION_TOMBSTONE
|
||||
|
||||
|
||||
def _staged_deletion_commit_marker(staged_path: Path) -> Path:
|
||||
return staged_path.parent / _UPLOAD_DELETION_COMMIT_MARKER
|
||||
|
||||
|
||||
def _staged_deletion_restore_marker(staged_path: Path) -> Path:
|
||||
return staged_path.parent / _UPLOAD_DELETION_RESTORE_MARKER
|
||||
|
||||
|
||||
def _create_deletion_phase_marker(marker: Path, *, error_message: str) -> None:
|
||||
"""Create and validate one durable deletion phase marker."""
|
||||
try:
|
||||
descriptor = os.open(marker, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600)
|
||||
except FileExistsError:
|
||||
marker_stat = os.lstat(marker)
|
||||
if not stat.S_ISREG(marker_stat.st_mode) or marker_stat.st_nlink != 1:
|
||||
raise UnsafeUploadPathError(error_message)
|
||||
return
|
||||
try:
|
||||
os.link(staged_path, original_path, follow_symlinks=False)
|
||||
except FileExistsError:
|
||||
recovery_path = _preserve_staged_entry_as_recovery(staged_path, original_path)
|
||||
logger.warning(
|
||||
"Upload name was recreated during deletion rollback; preserved the prior generation as %s",
|
||||
recovery_path,
|
||||
)
|
||||
_finish_deletion_transaction(staged_path)
|
||||
os.fsync(descriptor)
|
||||
finally:
|
||||
os.close(descriptor)
|
||||
|
||||
|
||||
def _mark_staged_deletion_committed(staged_path: Path) -> None:
|
||||
"""Persist that remote side effects completed and host recovery must discard."""
|
||||
_create_deletion_phase_marker(
|
||||
_staged_deletion_commit_marker(staged_path),
|
||||
error_message="Unsafe upload deletion commit marker",
|
||||
)
|
||||
|
||||
|
||||
def _mark_staged_deletion_restore(staged_path: Path) -> None:
|
||||
"""Persist that a live rollback began and recovery must finish restoring."""
|
||||
_create_deletion_phase_marker(
|
||||
_staged_deletion_restore_marker(staged_path),
|
||||
error_message="Unsafe upload deletion restore marker",
|
||||
)
|
||||
|
||||
|
||||
def _clear_staged_deletion_commit(staged_path: Path) -> None:
|
||||
_staged_deletion_commit_marker(staged_path).unlink(missing_ok=True)
|
||||
|
||||
|
||||
def _clear_staged_deletion_restore(staged_path: Path) -> None:
|
||||
_staged_deletion_restore_marker(staged_path).unlink(missing_ok=True)
|
||||
|
||||
|
||||
def _restore_staged_conversion(staged_path: Path, restored_primary_path: Path) -> None:
|
||||
"""Restore the exact conversion moved into a deletion transaction."""
|
||||
staged_conversion = _staged_conversion_path(staged_path)
|
||||
try:
|
||||
conversion_stat = os.lstat(staged_conversion)
|
||||
except FileNotFoundError:
|
||||
return
|
||||
staged_path.unlink()
|
||||
if not stat.S_ISREG(conversion_stat.st_mode) or conversion_stat.st_nlink not in {1, 2}:
|
||||
raise UnsafeUploadPathError("Staged upload conversion changed identity")
|
||||
|
||||
target = conversion_path_for_upload(restored_primary_path)
|
||||
if conversion_stat.st_nlink == 2:
|
||||
try:
|
||||
target_stat = os.lstat(target)
|
||||
except FileNotFoundError as exc:
|
||||
raise UnsafeUploadPathError("Staged upload conversion has an unknown hard-link peer") from exc
|
||||
if not stat.S_ISREG(target_stat.st_mode) or (target_stat.st_dev, target_stat.st_ino) != (conversion_stat.st_dev, conversion_stat.st_ino):
|
||||
raise UnsafeUploadPathError("Staged upload conversion has an unexpected hard-link peer")
|
||||
staged_conversion.unlink()
|
||||
return
|
||||
try:
|
||||
os.link(staged_conversion, target, follow_symlinks=False)
|
||||
except FileExistsError:
|
||||
# A replacement generation may already own this deterministic target.
|
||||
# Never overwrite it with the prior generation's conversion.
|
||||
logger.warning(
|
||||
"Discarding staged conversion because the restored target already exists: %s",
|
||||
target,
|
||||
)
|
||||
staged_conversion.unlink()
|
||||
|
||||
|
||||
def _discard_staged_deletion(staged_path: Path) -> None:
|
||||
"""Commit deletion of the exact primary and conversion tombstones."""
|
||||
_staged_conversion_path(staged_path).unlink(missing_ok=True)
|
||||
staged_path.unlink(missing_ok=True)
|
||||
_clear_staged_deletion_commit(staged_path)
|
||||
_clear_staged_deletion_restore(staged_path)
|
||||
_finish_deletion_transaction(staged_path)
|
||||
|
||||
|
||||
@ -736,6 +838,13 @@ def _recovery_path_for(original_path: Path) -> Path:
|
||||
|
||||
def _preserve_staged_entry_as_recovery(staged_path: Path, original_path: Path) -> Path:
|
||||
"""Publish staged bytes under a visible no-replace recovery name."""
|
||||
recovery_path = _link_staged_entry_as_recovery(staged_path, original_path)
|
||||
staged_path.unlink()
|
||||
return recovery_path
|
||||
|
||||
|
||||
def _link_staged_entry_as_recovery(staged_path: Path, original_path: Path) -> Path:
|
||||
"""Link staged bytes under a visible no-replace recovery name."""
|
||||
while True:
|
||||
recovery_path = _recovery_path_for(original_path)
|
||||
try:
|
||||
@ -743,7 +852,6 @@ def _preserve_staged_entry_as_recovery(staged_path: Path, original_path: Path) -
|
||||
break
|
||||
except FileExistsError:
|
||||
continue
|
||||
staged_path.unlink()
|
||||
return recovery_path
|
||||
|
||||
|
||||
@ -782,14 +890,66 @@ def _recover_stale_deletion_transaction(transaction_dir: Path) -> bool:
|
||||
if not entries:
|
||||
transaction_dir.rmdir()
|
||||
return True
|
||||
if len(entries) != 1:
|
||||
conversion_entries = [entry for entry in entries if entry.name == _UPLOAD_DELETION_CONVERSION_TOMBSTONE]
|
||||
commit_entries = [entry for entry in entries if entry.name == _UPLOAD_DELETION_COMMIT_MARKER]
|
||||
restore_entries = [entry for entry in entries if entry.name == _UPLOAD_DELETION_RESTORE_MARKER]
|
||||
primary_entries = [
|
||||
entry
|
||||
for entry in entries
|
||||
if entry.name
|
||||
not in {
|
||||
_UPLOAD_DELETION_CONVERSION_TOMBSTONE,
|
||||
_UPLOAD_DELETION_COMMIT_MARKER,
|
||||
_UPLOAD_DELETION_RESTORE_MARKER,
|
||||
}
|
||||
]
|
||||
if len(conversion_entries) > 1 or len(commit_entries) > 1 or len(restore_entries) > 1 or len(primary_entries) > 1:
|
||||
logger.warning(
|
||||
"Refusing malformed upload deletion transaction with %s entries: %s",
|
||||
len(entries),
|
||||
transaction_dir,
|
||||
)
|
||||
return False
|
||||
entry = entries[0]
|
||||
if conversion_entries:
|
||||
conversion_stat = conversion_entries[0].stat(follow_symlinks=False)
|
||||
if not stat.S_ISREG(conversion_stat.st_mode) or conversion_stat.st_nlink not in {1, 2}:
|
||||
logger.warning("Refusing malformed upload conversion tombstone: %s", transaction_dir)
|
||||
return False
|
||||
if commit_entries:
|
||||
commit_stat = commit_entries[0].stat(follow_symlinks=False)
|
||||
if not stat.S_ISREG(commit_stat.st_mode) or commit_stat.st_nlink != 1:
|
||||
logger.warning("Refusing malformed upload deletion commit marker: %s", transaction_dir)
|
||||
return False
|
||||
recover_on_crash = False
|
||||
if restore_entries:
|
||||
restore_stat = restore_entries[0].stat(follow_symlinks=False)
|
||||
if not stat.S_ISREG(restore_stat.st_mode) or restore_stat.st_nlink != 1:
|
||||
logger.warning("Refusing malformed upload deletion restore marker: %s", transaction_dir)
|
||||
return False
|
||||
if commit_entries:
|
||||
logger.warning("Refusing upload deletion transaction with conflicting phase markers: %s", transaction_dir)
|
||||
return False
|
||||
recover_on_crash = True
|
||||
if not recover_on_crash and conversion_entries and conversion_stat.st_nlink != 1:
|
||||
logger.warning(
|
||||
"Refusing discard transaction with a linked conversion tombstone: %s",
|
||||
transaction_dir,
|
||||
)
|
||||
return False
|
||||
if not primary_entries:
|
||||
if restore_entries and not conversion_entries:
|
||||
Path(restore_entries[0].path).unlink()
|
||||
transaction_dir.rmdir()
|
||||
return True
|
||||
if not commit_entries:
|
||||
logger.warning("Refusing upload deletion transaction without a primary: %s", transaction_dir)
|
||||
return False
|
||||
if conversion_entries:
|
||||
Path(conversion_entries[0].path).unlink()
|
||||
Path(commit_entries[0].path).unlink()
|
||||
transaction_dir.rmdir()
|
||||
return True
|
||||
entry = primary_entries[0]
|
||||
if not entry.is_file(follow_symlinks=False):
|
||||
logger.warning("Refusing malformed upload deletion transaction: %s", transaction_dir)
|
||||
return False
|
||||
@ -845,28 +1005,22 @@ def _recover_stale_deletion_transaction(transaction_dir: Path) -> bool:
|
||||
staged_path,
|
||||
)
|
||||
return False
|
||||
original_path = uploads_dir / original_name
|
||||
try:
|
||||
current_original_stat = os.lstat(original_path)
|
||||
except FileNotFoundError:
|
||||
current_original_stat = None
|
||||
delete_conversion = current_original_stat is None or (stat.S_ISREG(current_original_stat.st_mode) and (current_original_stat.st_dev, current_original_stat.st_ino) == (identity.device, identity.inode))
|
||||
if delete_conversion:
|
||||
owned_conversion = existing_conversion_path_for_upload(original_path)
|
||||
if owned_conversion is not None:
|
||||
owned_conversion.unlink(missing_ok=True)
|
||||
else:
|
||||
# The upload name was reused after the crash. Its conversion
|
||||
# path is generation-ambiguous, so preserve it rather than
|
||||
# deleting a possible replacement generation's companion.
|
||||
logger.warning(
|
||||
"Preserving replacement upload conversion during discard recovery: %s",
|
||||
original_path,
|
||||
)
|
||||
if not conversion_entries:
|
||||
# Backward compatibility for transactions created before the
|
||||
# exact conversion was moved beside the primary tombstone.
|
||||
original_path = uploads_dir / original_name
|
||||
try:
|
||||
current_original_stat = os.lstat(original_path)
|
||||
except FileNotFoundError:
|
||||
current_original_stat = None
|
||||
delete_conversion = current_original_stat is None or (stat.S_ISREG(current_original_stat.st_mode) and (current_original_stat.st_dev, current_original_stat.st_ino) == (identity.device, identity.inode))
|
||||
if delete_conversion:
|
||||
owned_conversion = existing_conversion_path_for_upload(original_path)
|
||||
if owned_conversion is not None:
|
||||
owned_conversion.unlink(missing_ok=True)
|
||||
if visible_matches:
|
||||
visible_matches[0].unlink()
|
||||
staged_path.unlink()
|
||||
_finish_deletion_transaction(staged_path)
|
||||
_discard_staged_deletion(staged_path)
|
||||
return True
|
||||
if staged_stat.st_nlink == 2:
|
||||
# A previous recovery may have crashed after publishing the visible
|
||||
@ -891,7 +1045,9 @@ def _recover_stale_deletion_transaction(transaction_dir: Path) -> bool:
|
||||
staged_path,
|
||||
)
|
||||
return False
|
||||
_restore_staged_conversion(staged_path, visible_matches[0])
|
||||
staged_path.unlink()
|
||||
_clear_staged_deletion_restore(staged_path)
|
||||
_finish_deletion_transaction(staged_path)
|
||||
return True
|
||||
_restore_staged_deletion(
|
||||
@ -932,8 +1088,9 @@ def _stage_primary_deletion(
|
||||
identity: UploadIdentity,
|
||||
*,
|
||||
recover_on_crash: bool = True,
|
||||
conversion_path: Path | None = None,
|
||||
) -> tuple[Path, UploadStageLease]:
|
||||
"""Atomically move the selected entry into the protected conversion namespace."""
|
||||
"""Move one primary generation and its exact conversion into a transaction."""
|
||||
staging_dir = ensure_conversion_dir(base_dir)
|
||||
intent = _UPLOAD_DELETION_RESTORE_INTENT if recover_on_crash else _UPLOAD_DELETION_DISCARD_INTENT
|
||||
while True:
|
||||
@ -958,6 +1115,13 @@ def _stage_primary_deletion(
|
||||
if not stat.S_ISREG(staged_stat.st_mode) or staged_stat.st_nlink != 1 or (staged_stat.st_dev, staged_stat.st_ino) != (identity.device, identity.inode):
|
||||
_restore_unexpected_staged_entry(staged_path, primary_path)
|
||||
raise UnsafeUploadPathError("Upload is no longer an exclusive directory entry")
|
||||
if conversion_path is not None:
|
||||
conversion_identity = UploadIdentity.from_path(conversion_path)
|
||||
staged_conversion = _staged_conversion_path(staged_path)
|
||||
os.rename(conversion_path, staged_conversion)
|
||||
staged_conversion_stat = os.lstat(staged_conversion)
|
||||
if not stat.S_ISREG(staged_conversion_stat.st_mode) or staged_conversion_stat.st_nlink != 1 or (staged_conversion_stat.st_dev, staged_conversion_stat.st_ino) != (conversion_identity.device, conversion_identity.inode):
|
||||
raise UnsafeUploadPathError("Upload conversion changed during deletion staging")
|
||||
return staged_path, stage_lease
|
||||
except BaseException:
|
||||
if staged_path.exists():
|
||||
@ -991,7 +1155,7 @@ def delete_file_safe(
|
||||
base_dir: Path,
|
||||
filename: str,
|
||||
*,
|
||||
delete_remote_copy: Callable[[str], None] | None = None,
|
||||
delete_remote_copy: Callable[[str, Path, Path | None], None] | None = None,
|
||||
) -> dict:
|
||||
"""Delete a primary upload and only its exact owned conversion.
|
||||
|
||||
@ -1053,16 +1217,53 @@ def delete_file_safe(
|
||||
base_dir,
|
||||
actual_file_path,
|
||||
identity,
|
||||
recover_on_crash=False,
|
||||
recover_on_crash=True,
|
||||
conversion_path=owned_conversion,
|
||||
)
|
||||
remote_delete_committed = False
|
||||
try:
|
||||
if delete_remote_copy is not None:
|
||||
delete_remote_copy(companion_name)
|
||||
if owned_conversion is not None:
|
||||
owned_conversion.unlink(missing_ok=True)
|
||||
staged_path.unlink()
|
||||
_finish_deletion_transaction(staged_path)
|
||||
staged_conversion = _staged_conversion_path(staged_path)
|
||||
delete_remote_copy(
|
||||
companion_name,
|
||||
staged_path,
|
||||
staged_conversion if staged_conversion.exists() else None,
|
||||
)
|
||||
remote_delete_committed = True
|
||||
_mark_staged_deletion_committed(staged_path)
|
||||
else:
|
||||
_mark_staged_deletion_committed(staged_path)
|
||||
_discard_staged_deletion(staged_path)
|
||||
except RemoteDeletionCommitRequiredError:
|
||||
# At least one remote mutation could not be rolled back. Restoring
|
||||
# the host generation would publish a permanently split view, so
|
||||
# keep the persisted discard intent and finish locally when able.
|
||||
try:
|
||||
_mark_staged_deletion_committed(staged_path)
|
||||
_discard_staged_deletion(staged_path)
|
||||
except BaseException:
|
||||
logger.warning(
|
||||
"Failed to finish a deletion after remote compensation failed: %s",
|
||||
actual_file_path,
|
||||
exc_info=True,
|
||||
)
|
||||
raise
|
||||
except BaseException:
|
||||
if remote_delete_committed:
|
||||
# The remote view already committed. Leave the discard
|
||||
# transaction for startup recovery if cleanup still fails,
|
||||
# instead of resurrecting a host-only generation. This also
|
||||
# handles a commit-marker write error while the process is
|
||||
# still alive.
|
||||
try:
|
||||
_discard_staged_deletion(staged_path)
|
||||
except BaseException:
|
||||
logger.warning(
|
||||
"Failed to finish a deletion after the remote copy was removed: %s",
|
||||
actual_file_path,
|
||||
exc_info=True,
|
||||
)
|
||||
raise
|
||||
_restore_staged_deletion(staged_path, actual_file_path, identity)
|
||||
raise
|
||||
finally:
|
||||
|
||||
@ -16,7 +16,11 @@ from deerflow.sandbox.sandbox_provider import (
|
||||
)
|
||||
from deerflow.uploads.async_helpers import run_upload_io_cancellation_safe, wait_for_task_completion
|
||||
from deerflow.uploads.layout import conversion_virtual_path
|
||||
from deerflow.uploads.manager import make_upload_file_sandbox_readable, upload_virtual_path
|
||||
from deerflow.uploads.manager import (
|
||||
RemoteDeletionCommitRequiredError,
|
||||
make_upload_file_sandbox_readable,
|
||||
upload_virtual_path,
|
||||
)
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@ -51,15 +55,47 @@ def _remove_remote_paths(sandbox: Any, virtual_paths: tuple[str, ...]) -> None:
|
||||
raise first_error
|
||||
|
||||
|
||||
def _deletion_hook_for_sandbox(sandbox: Any) -> Callable[[str], None]:
|
||||
def delete_remote_copy(filename: str) -> None:
|
||||
_remove_remote_paths(
|
||||
sandbox,
|
||||
def _deletion_hook_for_sandbox(
|
||||
sandbox: Any,
|
||||
) -> Callable[[str, Path, Path | None], None]:
|
||||
def delete_remote_copy(
|
||||
filename: str,
|
||||
primary_path: Path,
|
||||
conversion_path: Path | None,
|
||||
) -> None:
|
||||
targets = (
|
||||
(upload_virtual_path(filename), primary_path.read_bytes()),
|
||||
(
|
||||
conversion_virtual_path(filename),
|
||||
upload_virtual_path(filename),
|
||||
conversion_path.read_bytes() if conversion_path is not None else None,
|
||||
),
|
||||
)
|
||||
removed: list[tuple[str, bytes | None]] = []
|
||||
for virtual_path, authoritative_bytes in targets:
|
||||
try:
|
||||
sandbox.remove_file(virtual_path)
|
||||
except BaseException as delete_error:
|
||||
# A remote failure is ambiguous: the server may have removed
|
||||
# the file before the client observed the error. Re-publish
|
||||
# the current target and every prior success from the staged
|
||||
# authoritative bytes before allowing the host rollback.
|
||||
compensation_failed = False
|
||||
for rollback_path, rollback_bytes in reversed([*removed, (virtual_path, authoritative_bytes)]):
|
||||
if rollback_bytes is None:
|
||||
continue
|
||||
try:
|
||||
sandbox.update_file(rollback_path, rollback_bytes)
|
||||
except BaseException:
|
||||
compensation_failed = True
|
||||
logger.warning(
|
||||
"Failed to compensate partially deleted sandbox upload: %s",
|
||||
rollback_path,
|
||||
exc_info=True,
|
||||
)
|
||||
if compensation_failed:
|
||||
raise RemoteDeletionCommitRequiredError("Remote upload deletion could not be compensated") from delete_error
|
||||
raise
|
||||
removed.append((virtual_path, authoritative_bytes))
|
||||
|
||||
return delete_remote_copy
|
||||
|
||||
@ -69,7 +105,7 @@ def prepare_upload_deletion(
|
||||
thread_id: str,
|
||||
*,
|
||||
user_id: str | None,
|
||||
) -> Callable[[str], None] | None:
|
||||
) -> Callable[[str, Path, Path | None], None] | None:
|
||||
"""Return a lease-safe remote deletion hook for an explicitly synced sandbox."""
|
||||
if sandbox_provider_uses_thread_data_mounts(sandbox_provider):
|
||||
return None
|
||||
@ -87,7 +123,7 @@ async def prepare_upload_deletion_async(
|
||||
thread_id: str,
|
||||
*,
|
||||
user_id: str | None,
|
||||
) -> Callable[[str], None] | None:
|
||||
) -> Callable[[str, Path, Path | None], None] | None:
|
||||
"""Async counterpart that keeps remote acquisition off the event loop."""
|
||||
if await sandbox_provider_uses_thread_data_mounts_async(sandbox_provider):
|
||||
return None
|
||||
|
||||
@ -1247,6 +1247,7 @@ def test_remote_backend_create_forwards_effective_user_id(monkeypatch):
|
||||
"sandbox_id": "sandbox-42",
|
||||
"thread_id": "thread-42",
|
||||
"user_id": "user-7",
|
||||
"required_mount_contract_version": 2,
|
||||
"include_legacy_skills": True,
|
||||
"provision_lark_cli_runtime": False,
|
||||
"provision_lark_cli_broker": False,
|
||||
@ -1285,6 +1286,7 @@ def test_remote_backend_create_prefers_explicit_user_id(monkeypatch):
|
||||
|
||||
assert posted["json"]["user_id"] == "ou-user"
|
||||
assert posted["json"]["include_legacy_skills"] is False
|
||||
assert posted["json"]["required_mount_contract_version"] == 2
|
||||
|
||||
|
||||
def test_create_sandbox_requests_runtime_when_lark_installed(tmp_path, monkeypatch):
|
||||
|
||||
@ -42,6 +42,33 @@ def test_provisioner_accepts_canonical_thread_ids(provisioner_module, thread_id:
|
||||
assert request.thread_id == thread_id
|
||||
|
||||
|
||||
def test_create_rejects_mount_contract_precondition_before_k8s_io(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
provisioner_module,
|
||||
) -> None:
|
||||
fake_core_v1 = _RecordingCoreV1(event_loop_thread_id=-1)
|
||||
monkeypatch.setattr(provisioner_module, "core_v1", fake_core_v1)
|
||||
required_version = provisioner_module.MOUNT_CONTRACT_VERSION + 1
|
||||
|
||||
with pytest.raises(provisioner_module.HTTPException) as exc_info:
|
||||
provisioner_module.create_sandbox(
|
||||
provisioner_module.CreateSandboxRequest(
|
||||
sandbox_id="sandbox-wrong-contract",
|
||||
thread_id="thread-1",
|
||||
user_id="alice",
|
||||
required_mount_contract_version=required_version,
|
||||
)
|
||||
)
|
||||
|
||||
assert exc_info.value.status_code == 409
|
||||
assert exc_info.value.detail == {
|
||||
"code": "mount_contract_changed",
|
||||
"expected": required_version,
|
||||
"actual": provisioner_module.MOUNT_CONTRACT_VERSION,
|
||||
}
|
||||
assert fake_core_v1.thread_ids == []
|
||||
|
||||
|
||||
class _RecordingCoreV1:
|
||||
def __init__(
|
||||
self,
|
||||
|
||||
@ -457,6 +457,84 @@ def test_create_accepts_and_validates_future_compatible_contract(monkeypatch):
|
||||
assert info.mount_contract_version == 3
|
||||
|
||||
|
||||
def test_create_sends_frozen_mount_contract_as_a_provisioner_precondition(monkeypatch):
|
||||
backend = RemoteSandboxBackend("http://provisioner:8002")
|
||||
backend._mount_contract_version = 2
|
||||
backend._mount_contract_capability_known = True
|
||||
backend._capability_next_probe_at = time.monotonic() + 30
|
||||
monkeypatch.setattr(remote_backend_mod, "user_should_see_legacy_skills", lambda _user_id: False)
|
||||
captured: dict = {}
|
||||
|
||||
def post(_url, *, json, **_kwargs):
|
||||
captured.update(json)
|
||||
return _StubResponse(
|
||||
payload={
|
||||
"sandbox_id": "sandbox-v2",
|
||||
"sandbox_url": "http://v2.local",
|
||||
"user_id": "alice",
|
||||
"thread_id": "thread-1",
|
||||
"mount_contract_version": 2,
|
||||
}
|
||||
)
|
||||
|
||||
monkeypatch.setattr(requests, "post", post)
|
||||
|
||||
backend.create("thread-1", "sandbox-v2", user_id="alice")
|
||||
|
||||
assert captured["required_mount_contract_version"] == 2
|
||||
|
||||
|
||||
def test_create_retries_structured_provisioner_contract_precondition_failure(monkeypatch):
|
||||
backend = RemoteSandboxBackend("http://provisioner:8002")
|
||||
backend._mount_contract_version = 2
|
||||
backend._mount_contract_capability_known = True
|
||||
backend._capability_next_probe_at = time.monotonic() + 30
|
||||
monkeypatch.setattr(remote_backend_mod, "user_should_see_legacy_skills", lambda _user_id: False)
|
||||
monkeypatch.setattr(
|
||||
requests,
|
||||
"post",
|
||||
lambda *_args, **_kwargs: _StubResponse(
|
||||
status_code=409,
|
||||
payload={
|
||||
"detail": {
|
||||
"code": "mount_contract_changed",
|
||||
"expected": 2,
|
||||
"actual": 3,
|
||||
}
|
||||
},
|
||||
),
|
||||
)
|
||||
|
||||
with pytest.raises(remote_backend_mod.MountContractChangedError) as exc_info:
|
||||
backend.create("thread-1", "sandbox-v2", user_id="alice")
|
||||
|
||||
assert (exc_info.value.expected, exc_info.value.actual) == (2, 3)
|
||||
assert backend._capability_next_probe_at == 0.0
|
||||
|
||||
|
||||
@pytest.mark.parametrize("invalid_version", ["2", True])
|
||||
def test_legacy_create_rejects_non_integer_response_contract(monkeypatch, invalid_version):
|
||||
backend = RemoteSandboxBackend("http://provisioner:8002")
|
||||
backend._mount_contract_version = 0
|
||||
backend._mount_contract_capability_known = True
|
||||
backend._capability_next_probe_at = time.monotonic() + 30
|
||||
monkeypatch.setattr(remote_backend_mod, "user_should_see_legacy_skills", lambda _user_id: False)
|
||||
monkeypatch.setattr(
|
||||
requests,
|
||||
"post",
|
||||
lambda *_args, **_kwargs: _StubResponse(
|
||||
payload={
|
||||
"sandbox_id": "sandbox-v0",
|
||||
"sandbox_url": "http://legacy.local",
|
||||
"mount_contract_version": invalid_version,
|
||||
}
|
||||
),
|
||||
)
|
||||
|
||||
with pytest.raises(RuntimeError, match="mount contract response"):
|
||||
backend.create("thread-1", "sandbox-v0", user_id="default")
|
||||
|
||||
|
||||
def test_provisioner_create_returns_sandbox_info(monkeypatch):
|
||||
backend = RemoteSandboxBackend("http://provisioner:8002")
|
||||
monkeypatch.setattr(remote_backend_mod, "user_should_see_legacy_skills", lambda user_id: True)
|
||||
@ -467,6 +545,7 @@ def test_provisioner_create_returns_sandbox_info(monkeypatch):
|
||||
"sandbox_id": "abc123",
|
||||
"thread_id": "thread-1",
|
||||
"user_id": "test-user-autouse",
|
||||
"required_mount_contract_version": 0,
|
||||
"include_legacy_skills": True,
|
||||
"provision_lark_cli_runtime": False,
|
||||
"provision_lark_cli_broker": False,
|
||||
@ -588,6 +667,7 @@ def test_provisioner_create_accepts_anonymous_thread_id(monkeypatch):
|
||||
"sandbox_id": "anon123",
|
||||
"thread_id": None,
|
||||
"user_id": "test-user-autouse",
|
||||
"required_mount_contract_version": 0,
|
||||
"include_legacy_skills": False,
|
||||
"provision_lark_cli_runtime": False,
|
||||
"provision_lark_cli_broker": False,
|
||||
|
||||
@ -502,14 +502,16 @@ class TestUploadPublication:
|
||||
assert publication.path.read_bytes() == b"new"
|
||||
|
||||
def test_rollback_preserves_primary_when_conversion_removal_fails(self, tmp_path):
|
||||
publication = publish_upload_bytes_leased(tmp_path, "report.pdf", b"old")
|
||||
uploads = tmp_path / "users" / "alice" / "threads" / "thread-1" / "user-data" / "uploads"
|
||||
uploads.mkdir(parents=True)
|
||||
publication = publish_upload_bytes_leased(uploads, "report.pdf", b"old")
|
||||
owned_conversion = conversion_path_for_upload(publication.path)
|
||||
owned_conversion.parent.mkdir(exist_ok=True)
|
||||
owned_conversion.write_text("generated", encoding="utf-8")
|
||||
real_unlink = Path.unlink
|
||||
|
||||
def fail_conversion_unlink(path, *args, **kwargs):
|
||||
if path == owned_conversion:
|
||||
if path.name == ".conversion" and path.parent.name.startswith(".upload-delete-"):
|
||||
raise OSError("cannot unlink conversion")
|
||||
return real_unlink(path, *args, **kwargs)
|
||||
|
||||
@ -522,9 +524,15 @@ class TestUploadPublication:
|
||||
|
||||
assert publication.path.read_bytes() == b"old"
|
||||
assert owned_conversion.read_text(encoding="utf-8") == "generated"
|
||||
assert cleanup_stale_upload_staging_files(tmp_path) == 1
|
||||
assert publication.path.read_bytes() == b"old"
|
||||
assert owned_conversion.read_text(encoding="utf-8") == "generated"
|
||||
assert not list(owned_conversion.parent.glob(".upload-delete-*.part"))
|
||||
|
||||
def test_successful_rollback_removes_deletion_transaction_directory(self, tmp_path):
|
||||
publication = publish_upload_bytes_leased(tmp_path, "report.pdf", b"old")
|
||||
uploads = tmp_path / "uploads"
|
||||
uploads.mkdir()
|
||||
publication = publish_upload_bytes_leased(uploads, "report.pdf", b"old")
|
||||
conversion_dir = conversion_path_for_upload(publication.path).parent
|
||||
try:
|
||||
rollback_published_upload(publication)
|
||||
@ -1019,29 +1027,81 @@ class TestCleanupStaleUploadStagingFiles:
|
||||
assert not conversion.exists()
|
||||
assert not staged_path.exists()
|
||||
|
||||
def test_crashed_committed_delete_preserves_replacement_conversion(self, tmp_path):
|
||||
def test_crashed_committed_delete_does_not_rebind_old_conversion_to_replacement(self, tmp_path):
|
||||
import deerflow.uploads.manager as upload_manager_module
|
||||
|
||||
uploads = tmp_path / "users" / "alice" / "threads" / "thread-1" / "user-data" / "uploads"
|
||||
uploads.mkdir(parents=True)
|
||||
primary = uploads / "report.pdf"
|
||||
primary.write_bytes(b"old generation")
|
||||
conversion = conversion_path_for_upload(primary)
|
||||
conversion.parent.mkdir()
|
||||
conversion.write_text("old conversion", encoding="utf-8")
|
||||
old_identity = UploadIdentity.from_path(primary)
|
||||
staged_path, stage_lease = upload_manager_module._stage_primary_deletion(
|
||||
uploads,
|
||||
primary,
|
||||
old_identity,
|
||||
recover_on_crash=False,
|
||||
conversion_path=conversion,
|
||||
)
|
||||
primary.write_bytes(b"replacement generation")
|
||||
conversion = conversion_path_for_upload(primary)
|
||||
conversion.parent.mkdir(exist_ok=True)
|
||||
conversion.write_text("replacement conversion", encoding="utf-8")
|
||||
stage_lease.release()
|
||||
|
||||
assert cleanup_stale_upload_staging_files(tmp_path) == 1
|
||||
assert primary.read_bytes() == b"replacement generation"
|
||||
assert conversion.read_text(encoding="utf-8") == "replacement conversion"
|
||||
assert not conversion.exists()
|
||||
assert not staged_path.exists()
|
||||
|
||||
def test_crash_before_remote_delete_restores_primary_and_conversion(self, tmp_path):
|
||||
import deerflow.uploads.manager as upload_manager_module
|
||||
|
||||
uploads = tmp_path / "users" / "alice" / "threads" / "thread-1" / "user-data" / "uploads"
|
||||
uploads.mkdir(parents=True)
|
||||
primary = uploads / "report.pdf"
|
||||
primary.write_bytes(b"primary")
|
||||
conversion = conversion_path_for_upload(primary)
|
||||
conversion.parent.mkdir()
|
||||
conversion.write_text("conversion", encoding="utf-8")
|
||||
identity = UploadIdentity.from_path(primary)
|
||||
staged_path, stage_lease = upload_manager_module._stage_primary_deletion(
|
||||
uploads,
|
||||
primary,
|
||||
identity,
|
||||
recover_on_crash=True,
|
||||
conversion_path=conversion,
|
||||
)
|
||||
stage_lease.release()
|
||||
|
||||
assert cleanup_stale_upload_staging_files(tmp_path) == 1
|
||||
assert primary.read_bytes() == b"primary"
|
||||
assert conversion.read_text(encoding="utf-8") == "conversion"
|
||||
assert not staged_path.exists()
|
||||
|
||||
def test_crash_after_remote_delete_commits_primary_and_conversion(self, tmp_path):
|
||||
import deerflow.uploads.manager as upload_manager_module
|
||||
|
||||
uploads = tmp_path / "users" / "alice" / "threads" / "thread-1" / "user-data" / "uploads"
|
||||
uploads.mkdir(parents=True)
|
||||
primary = uploads / "report.pdf"
|
||||
primary.write_bytes(b"primary")
|
||||
conversion = conversion_path_for_upload(primary)
|
||||
conversion.parent.mkdir()
|
||||
conversion.write_text("conversion", encoding="utf-8")
|
||||
identity = UploadIdentity.from_path(primary)
|
||||
staged_path, stage_lease = upload_manager_module._stage_primary_deletion(
|
||||
uploads,
|
||||
primary,
|
||||
identity,
|
||||
recover_on_crash=True,
|
||||
conversion_path=conversion,
|
||||
)
|
||||
upload_manager_module._mark_staged_deletion_committed(staged_path)
|
||||
stage_lease.release()
|
||||
|
||||
assert cleanup_stale_upload_staging_files(tmp_path) == 1
|
||||
assert not primary.exists()
|
||||
assert not conversion.exists()
|
||||
assert not staged_path.exists()
|
||||
|
||||
|
||||
@ -1175,7 +1235,7 @@ class TestDeleteFileSafe:
|
||||
conversion.rename(renamed_conversion)
|
||||
return renamed
|
||||
|
||||
def publish_replacement(actual_name):
|
||||
def publish_replacement(actual_name, _primary_path, _conversion_path):
|
||||
remote_names.append(actual_name)
|
||||
publish_upload_bytes(uploads, actual_name, b"new")
|
||||
renamed_conversion.unlink()
|
||||
@ -1225,7 +1285,7 @@ class TestDeleteFileSafe:
|
||||
real_unlink = Path.unlink
|
||||
|
||||
def fail_conversion_unlink(path, *args, **kwargs):
|
||||
if path == owned_conversion:
|
||||
if path.name == ".conversion" and path.parent.name.startswith(".upload-delete-"):
|
||||
raise OSError("cannot unlink conversion")
|
||||
return real_unlink(path, *args, **kwargs)
|
||||
|
||||
@ -1236,13 +1296,42 @@ class TestDeleteFileSafe:
|
||||
assert primary.read_bytes() == b"PDF"
|
||||
assert owned_conversion.read_text(encoding="utf-8") == "generated"
|
||||
|
||||
def test_remote_success_does_not_restore_host_when_commit_marker_fails(self, tmp_path):
|
||||
import deerflow.uploads.manager as upload_manager_module
|
||||
|
||||
uploads = tmp_path / "user-data" / "uploads"
|
||||
uploads.mkdir(parents=True)
|
||||
primary = uploads / "report.pdf"
|
||||
primary.write_bytes(b"PDF")
|
||||
owned_conversion = conversion_path_for_upload(primary)
|
||||
owned_conversion.parent.mkdir()
|
||||
owned_conversion.write_text("generated", encoding="utf-8")
|
||||
remote_names: list[str] = []
|
||||
|
||||
with patch.object(
|
||||
upload_manager_module,
|
||||
"_mark_staged_deletion_committed",
|
||||
side_effect=OSError("cannot persist commit marker"),
|
||||
):
|
||||
with pytest.raises(OSError, match="cannot persist commit marker"):
|
||||
delete_file_safe(
|
||||
uploads,
|
||||
"report.pdf",
|
||||
delete_remote_copy=lambda name, _primary, _conversion: remote_names.append(name),
|
||||
)
|
||||
|
||||
assert remote_names == ["report.pdf"]
|
||||
assert not primary.exists()
|
||||
assert not owned_conversion.exists()
|
||||
assert not list(owned_conversion.parent.glob(".upload-delete-*.part"))
|
||||
|
||||
def test_failed_delete_preserves_old_generation_when_name_is_recreated(self, tmp_path):
|
||||
uploads = tmp_path / "user-data" / "uploads"
|
||||
uploads.mkdir(parents=True)
|
||||
primary = uploads / "report.pdf"
|
||||
primary.write_bytes(b"old generation")
|
||||
|
||||
def recreate_then_fail(_filename):
|
||||
def recreate_then_fail(_filename, _primary_path, _conversion_path):
|
||||
primary.write_bytes(b"new generation")
|
||||
raise OSError("remote delete failed")
|
||||
|
||||
@ -1287,7 +1376,7 @@ class TestDeleteFileSafe:
|
||||
delete_file_safe(
|
||||
uploads,
|
||||
primary.name,
|
||||
delete_remote_copy=remote_names.append,
|
||||
delete_remote_copy=lambda name, _primary, _conversion: remote_names.append(name),
|
||||
)
|
||||
|
||||
assert not renamed.exists()
|
||||
|
||||
@ -1442,6 +1442,79 @@ def test_delete_uploaded_file_preserves_host_when_remote_delete_fails(tmp_path):
|
||||
assert primary.read_bytes() == b"notes"
|
||||
|
||||
|
||||
def test_delete_uploaded_file_compensates_partial_remote_delete_before_restoring_host(tmp_path):
|
||||
thread_uploads_dir = tmp_path / "uploads"
|
||||
thread_uploads_dir.mkdir(parents=True)
|
||||
primary = thread_uploads_dir / "report.pdf"
|
||||
primary.write_bytes(b"pdf")
|
||||
conversion = conversion_path_for_upload(primary)
|
||||
conversion.parent.mkdir()
|
||||
conversion.write_text("generated", encoding="utf-8")
|
||||
provider = MagicMock()
|
||||
provider.uses_thread_data_mounts = False
|
||||
provider.acquire_async = AsyncMock(return_value="remote-1")
|
||||
sandbox = MagicMock()
|
||||
sandbox.remove_file.side_effect = [None, OSError("conversion unavailable")]
|
||||
provider.get.return_value = sandbox
|
||||
|
||||
with (
|
||||
patch.object(uploads, "get_uploads_dir", return_value=thread_uploads_dir),
|
||||
patch.object(uploads, "get_sandbox_provider", return_value=provider),
|
||||
):
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
asyncio.run(
|
||||
call_unwrapped(
|
||||
uploads.delete_uploaded_file,
|
||||
"thread-remote",
|
||||
"report.pdf",
|
||||
request=MagicMock(),
|
||||
)
|
||||
)
|
||||
|
||||
assert exc_info.value.status_code == 500
|
||||
sandbox.update_file.assert_any_call(
|
||||
"/mnt/user-data/uploads/report.pdf",
|
||||
b"pdf",
|
||||
)
|
||||
assert primary.read_bytes() == b"pdf"
|
||||
assert conversion.read_text(encoding="utf-8") == "generated"
|
||||
|
||||
|
||||
def test_delete_uploaded_file_commits_host_delete_when_remote_compensation_fails(tmp_path):
|
||||
thread_uploads_dir = tmp_path / "uploads"
|
||||
thread_uploads_dir.mkdir(parents=True)
|
||||
primary = thread_uploads_dir / "report.pdf"
|
||||
primary.write_bytes(b"pdf")
|
||||
conversion = conversion_path_for_upload(primary)
|
||||
conversion.parent.mkdir()
|
||||
conversion.write_text("generated", encoding="utf-8")
|
||||
provider = MagicMock()
|
||||
provider.uses_thread_data_mounts = False
|
||||
provider.acquire_async = AsyncMock(return_value="remote-1")
|
||||
sandbox = MagicMock()
|
||||
sandbox.remove_file.side_effect = [None, OSError("conversion unavailable")]
|
||||
sandbox.update_file.side_effect = OSError("remote rollback unavailable")
|
||||
provider.get.return_value = sandbox
|
||||
|
||||
with (
|
||||
patch.object(uploads, "get_uploads_dir", return_value=thread_uploads_dir),
|
||||
patch.object(uploads, "get_sandbox_provider", return_value=provider),
|
||||
):
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
asyncio.run(
|
||||
call_unwrapped(
|
||||
uploads.delete_uploaded_file,
|
||||
"thread-remote",
|
||||
"report.pdf",
|
||||
request=MagicMock(),
|
||||
)
|
||||
)
|
||||
|
||||
assert exc_info.value.status_code == 500
|
||||
assert not primary.exists()
|
||||
assert not conversion.exists()
|
||||
|
||||
|
||||
@pytest.mark.parametrize("filename", ["", None])
|
||||
def test_upload_files_reports_empty_multipart_filename_as_skipped(tmp_path, filename):
|
||||
thread_uploads_dir = tmp_path / "uploads"
|
||||
|
||||
@ -502,6 +502,7 @@ class CreateSandboxRequest(BaseModel):
|
||||
sandbox_id: str
|
||||
thread_id: str | None = Field(default=None, pattern=SAFE_THREAD_ID_PATTERN)
|
||||
user_id: str = Field(default=DEFAULT_USER_ID, pattern=SAFE_USER_ID_PATTERN)
|
||||
required_mount_contract_version: int | None = Field(default=None, ge=0)
|
||||
extra_mounts: list[ExtraMount] = Field(default_factory=list)
|
||||
include_legacy_skills: bool = False
|
||||
# When true (and LARK_CLI_INIT_IMAGE is configured), provision the sandbox
|
||||
@ -1323,6 +1324,23 @@ def create_sandbox(req: CreateSandboxRequest):
|
||||
If the sandbox already exists, returns the existing information
|
||||
(idempotent).
|
||||
"""
|
||||
if (
|
||||
req.required_mount_contract_version is not None
|
||||
and req.required_mount_contract_version != MOUNT_CONTRACT_VERSION
|
||||
):
|
||||
# The Gateway derives the deterministic sandbox ID and mount set from
|
||||
# one frozen capability snapshot. Reject a stale snapshot before any
|
||||
# Kubernetes read or write so a rolling upgrade cannot create a Pod
|
||||
# whose ID belongs to another mount contract.
|
||||
raise HTTPException(
|
||||
status_code=409,
|
||||
detail={
|
||||
"code": "mount_contract_changed",
|
||||
"expected": req.required_mount_contract_version,
|
||||
"actual": MOUNT_CONTRACT_VERSION,
|
||||
},
|
||||
)
|
||||
|
||||
sandbox_id = req.sandbox_id
|
||||
thread_id = req.thread_id or sandbox_id
|
||||
user_id = req.user_id
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user