From 8234370a6ad2abd83bdba35271b902aa0bd19d47 Mon Sep 17 00:00:00 2001 From: qin-chenghan Date: Sat, 1 Aug 2026 09:15:21 +0800 Subject: [PATCH] feat(artifacts): inline editing for text artifacts in the panel (#4596) * feat(artifacts): inline editing for text artifacts in the panel Add a PUT /api/threads/{id}/artifacts/{path} endpoint that atomically replaces an existing UTF-8 text file under /mnt/user-data/outputs after verifying its SHA-256 revision. Active runs conflict (409); binary, symlink, oversized, and non-output paths are rejected. Frontend: edit/save/discard buttons, draft state with conflict detection, CodeEditor onChange/onSave, loader SHA-256 from ETag, i18n, beforeunload guard. Backend: PUT endpoint with thread reservation, atomic temp-file replacement, sandbox sync for non-mounted providers, rollback on failure, ETag on GET. Tests: 8 backend + 1 blocking-IO + 3 frontend test files. * fix(artifacts): scope replacement permissions and release sandboxes --------- Co-authored-by: Willem Jiang --- README.md | 2 +- backend/AGENTS.md | 4 +- backend/app/gateway/routers/artifacts.py | 196 ++++++++++- .../harness/deerflow/runtime/runs/schemas.py | 1 + .../blocking_io/test_artifacts_router.py | 36 +- backend/tests/test_artifacts_router.py | 262 +++++++++++++++ frontend/AGENTS.md | 1 + .../artifacts/artifact-file-detail.tsx | 311 ++++++++++++++++-- .../workspace/artifacts/context.tsx | 31 ++ .../src/components/workspace/code-editor.tsx | 17 +- frontend/src/core/artifacts/api.ts | 54 +++ frontend/src/core/artifacts/editing.ts | 67 ++++ frontend/src/core/artifacts/hooks.ts | 1 + frontend/src/core/artifacts/index.ts | 2 + frontend/src/core/artifacts/loader.ts | 18 +- frontend/src/core/i18n/locales/en-US.ts | 14 + frontend/src/core/i18n/locales/types.ts | 13 + frontend/src/core/i18n/locales/zh-CN.ts | 13 + .../tests/unit/core/artifacts/api.test.ts | 60 ++++ .../tests/unit/core/artifacts/editing.test.ts | 75 +++++ .../tests/unit/core/artifacts/loader.test.ts | 43 +++ 21 files changed, 1189 insertions(+), 32 deletions(-) create mode 100644 frontend/src/core/artifacts/api.ts create mode 100644 frontend/src/core/artifacts/editing.ts create mode 100644 frontend/tests/unit/core/artifacts/api.test.ts create mode 100644 frontend/tests/unit/core/artifacts/editing.test.ts create mode 100644 frontend/tests/unit/core/artifacts/loader.test.ts diff --git a/README.md b/README.md index 2ac58c9eb..273f34c7c 100644 --- a/README.md +++ b/README.md @@ -943,7 +943,7 @@ Image bytes loaded for a vision-model call are transient: DeerFlow removes the h After each run, DeerFlow records a workspace change summary for the run-owned `workspace` and `outputs` directories. The Web UI shows a compact "files changed" badge on the assistant turn; opening it reveals created, modified, and deleted files with text diffs when safe to display. Uploads are excluded because they are user inputs, not agent-generated changes. Large, binary, or sensitive-looking files are shown as metadata only. -Files presented through `present_files` remain part of the thread's artifact state, and the Web UI restores the artifact panel and selected document after a page refresh. The currently selected formal artifact is refreshed once when the run finishes so edits become visible without a manual reload. +Files presented through `present_files` remain part of the thread's artifact state, and the Web UI restores the artifact panel and selected document after a page refresh. The currently selected formal artifact is refreshed once when the run finishes so edits become visible without a manual reload. Existing UTF-8 text artifacts under `/mnt/user-data/outputs` can also be edited and explicitly saved from the panel while the thread is idle; saves use content revisions to prevent overwriting agent changes. With `AioSandboxProvider`, shell execution runs inside isolated containers. With `LocalSandboxProvider`, file tools still map to per-thread directories on the host, but host `bash` is disabled by default because it is not a secure isolation boundary. Re-enable host bash only for fully trusted local workflows. Host bash commands have a wall-clock timeout, and long-lived processes should be started in the background with output redirected to a workspace log. diff --git a/backend/AGENTS.md b/backend/AGENTS.md index 988e56066..61a492ba5 100644 --- a/backend/AGENTS.md +++ b/backend/AGENTS.md @@ -492,7 +492,7 @@ Localhost persistence deliberately reads the direct request `Host` and ignores ` | **Memory** (`/api/memory`) | `GET /` - memory data; `POST /reload` - force reload; `GET /config` - config; `GET /status` - config + data | | **Uploads** (`/api/threads/{id}/uploads`) | `POST /` - upload files (auto-converts PDF/PPT/Excel/Word); `GET /list` - list; `DELETE /{filename}` - delete | | **Threads** (`/api/threads/{id}`) | `DELETE /` - remove DeerFlow-managed local thread data after LangGraph thread deletion; `POST /branches` - create a new main-thread branch from a completed assistant turn checkpoint and, when an addressable pre-user replay checkpoint exists, materialize it into the branch namespace so the inherited response remains regeneratable. Workspace files are not checkpointed, so the branch only best-effort copies the current workspace when branching from the **latest** turn (`workspace_clone_mode="current_thread_best_effort"`); branching from an older/historical turn skips the copy (`workspace_clone_mode="skipped_historical_turn"`) so the branch never inherits files that only exist in a later timeline. Thread-scoped runtime channels (`sandbox`, `thread_data`) are not copied onto the branch: the parent's `sandbox_id` binds path mappings and the release lifecycle to the parent's workspace, so the branch lazily acquires its own sandbox instead. Branch creation also seeds the new thread's run-event feed from the branch checkpoint's visible messages (`history_seed_mode` in the response): the thread feed reads run_events, not checkpoints, so without the seed the inherited history disappears from the UI after the branch's first run (#4380). Seeded rows are grouped into one synthetic run per inherited turn (`branch-seed-{thread_id}-{n}`, a new turn opening at every persisted human message, including an allowlisted hidden `ask_clarification` reply) because `run_id` is a turn identity to the feed's consumers, not a provenance tag: regenerating an inherited answer supersedes that row's whole `run_id` in `GET /messages/page`, so one shared id for the entire seed deleted the complete inherited history on a branch's first regenerate (#4458); `GET /goal`, `PUT /goal`, `DELETE /goal` - read, set, and clear the active thread goal; `POST /compact` - manually summarize older active context into `summary_text` and retain the recent message window, blocked while a run is in flight; unexpected failures are logged server-side and return a generic 500 detail | -| **Artifacts** (`/api/threads/{id}/artifacts`) | `GET /{path}` - serve artifacts; active content types (`text/html`, `application/xhtml+xml`, `image/svg+xml`) are always forced as download attachments to reduce XSS risk; `?download=true` still forces download for other file types | +| **Artifacts** (`/api/threads/{id}/artifacts`) | `GET /{path}` - serve artifacts; active content types (`text/html`, `application/xhtml+xml`, `image/svg+xml`) are always forced as download attachments to reduce XSS risk; `?download=true` still forces download for other file types. `PUT /{path}` atomically replaces an existing UTF-8 text file under `/mnt/user-data/outputs` when its expected SHA-256 still matches; active runs conflict, and non-mounted sandbox providers receive the same update explicitly. | | **Suggestions** (`/api/suggestions`) | `GET /config` - returns global suggestions config boolean; `POST /threads/{id}/suggestions` - generate follow-up questions; rich list/block model content is normalized and inline reasoning (`...`, including unclosed/truncated blocks from reasoning models like MiniMax-M3) is stripped before JSON parsing | | **Input Polish** (`/api/input-polish`) | `POST /` - rewrite a composer draft before it is sent. This is a short authenticated `runs:create` LLM request using `input_polish` config; it does not create a LangGraph run, persist a message, or modify thread state. Shares the non-graph one-shot LLM path (`deerflow.utils.oneshot_llm.run_oneshot_llm`) with the suggestions route so model build + Langfuse metadata + invoke stay in one place; validates the same stripped view of the draft it sends to the model, and preserves literal `` substrings in the rewrite (`strip_think_blocks(truncate_unclosed=False)`) | | **Thread Runs** (`/api/threads/{id}/runs`) | `POST /` - create background run; `POST /stream` - create + SSE stream; `POST /wait` - create + block. Before the first journaled run, an empty run-event message feed is seeded from an existing checkpoint head so legacy checkpoint-only history receives earlier thread-global sequence numbers and remains visible after the new run; a thread with no checkpoint or an already-populated feed skips this compatibility path. `POST /regenerate/prepare` - prepare clean input + checkpoint metadata for regenerating the latest completed or interrupted assistant answer, carrying the latest non-empty thread title in graph input so resuming an older checkpoint cannot roll back a later manual rename (#4457); `POST /edit-regenerate/prepare` - prepare a checkpoint replay from the latest editable human turn with a replacement user message and edit replay metadata; it carries the current thread title the same way, but only when the replay base already has one — an untitled base belongs to a thread the title middleware has not named yet, so pinning the current title there would keep a name generated from the prompt the edit just replaced; `GET /` - list runs; `GET /{rid}` - run details; `POST /{rid}/cancel` - cancel; `GET /{rid}/join` - join SSE; `GET /{rid}/messages` - paginated per-run messages `{data, has_more}`; `GET /{rid}/events` - full event stream; `GET /{rid}/workspace-changes` - workspace/output file change summary and optional diffs; `GET /../messages` - legacy thread message array; `GET /../messages/page` - backward thread-global `seq` history page with middleware/subagent-AI/successful-regenerate/edit-replay filtering and page-run-scoped feedback enrichment; subagent AI callbacks remain available through run events while parent `task` ToolMessages stay visible for card restoration; `GET /../token-usage` - aggregate tokens plus an optional `context_usage` percentage. Context usage approximately counts messages from the latest materialized thread state through `build_thread_checkpoint_state_accessor`, so full and delta checkpoint modes expose the same input. The percentage uses the latest run's model and its configured `context_window`. | @@ -567,7 +567,7 @@ JSONL event stores when `GATEWAY_WORKERS > 1`. - Store-only hydrated runs are readable history. In multi-worker mode with heartbeat enabled, cancel on a store-only run records `runs.cancel_action` / `cancel_requested_at` while the owner's lease is live; the first action wins even if a retry later lands on the owner. `RunStore.request_cancel()` and owner completion through `finalize_if_not_cancelled()` are competing active-row CAS operations, so an accepted cancel cannot be overwritten by a later success. `RunStore.renew_lease()` renews and observes the request atomically in the SQL implementation. The owner then executes the normal process-local interrupt/rollback and terminal stream path without transferring the lease. An expired owner is still taken over and marked `error`. `wait=true` and cancel-then-stream use the shared bridge to observe owner finalization; a non-standard process-local bridge returns accepted 202 instead of subscribing to an unreachable stream. In single-worker mode (heartbeat off), store-only runs still return 409. - A local worker's `RunRecord.lease_expires_at` is the last durably confirmed ownership deadline. `_renew_leases()` bounds each renewal attempt by that deadline: transient store exceptions remain retryable while it is valid, but an exception or blocked call that reaches expiry sets the process-local `ownership_lost` fence, raises `abort_event`, and cancels the run task. Successful renewals collect durable cancellation actions; after all local renewals have been attempted, heartbeat only signals the corresponding process-local tasks, leaving status writes and rollback cleanup to the worker finalization path. Fenced workers do not perform subsequent journal/delivery-receipt, progress/completion/status, checkpoint/thread-metadata, or `on_run_completed` writes; the peer recovery path owns the terminal receipt. `RunStore.update_run_completion()` also refuses to replace a different terminal status, closing the peer-takeover/late-finalization race. `grace_seconds` delays peer reclamation for clock skew but is not extra execution time for an owner that can no longer confirm its lease. Already-committed remote tool side effects remain outside this local cancellation boundary. - Startup/orphan reconciliation must claim stale active rows with `RunStore.claim_for_takeover()`, not a plain `update_status()`. The final claim re-checks `status` and lease expiry atomically, so a heartbeat renewal between the candidate scan and the recovery write keeps the run active. -- Run admission and independent checkpoint writes are first-class thread operations. `runs.operation_kind` distinguishes user-visible `run` rows from internal `checkpoint_write` reservations, while every active kind shares the existing durable active-thread uniqueness constraint. New operation kinds must go through `RunStore.create_thread_operation_atomic()` and `RunManager.reserve_thread_operation()` rather than adding another lock or metadata marker. Live and lease-less reservations are non-interruptible; an expired leased reservation can be reclaimed immediately by interrupt/rollback admission without waiting for orphan reconciliation. Lease-less rows stay fail-closed because the store cannot distinguish a stale row from a live checkpoint writer in another heartbeat-disabled worker; a rare failed delete therefore requires startup reconciliation, and heartbeat-disabled multi-worker deployment remains unsupported. Reservation bodies are attached to their caller task so loss detected by lease renewal cancels the checkpoint writer before it can continue after takeover; the context manager translates that lease-loss cancellation to `ConflictError` after cleanup so Gateway mutation routes return a retryable 409 instead of dropping the HTTP request. The cleanup scope begins immediately after durable admission, including the await that attaches the caller task, so cancellation cannot strand a locally renewed pending reservation. A failed renewal is revalidated under the manager lock before cancellation; if the reservation completed and unregistered while the store update was in flight, its request task must not be cancelled after the checkpoint write. Reservations are excluded from run history/reporting and from run-only helpers such as `list_by_thread()` and `has_inflight()`, release uses the captured owner rather than ambient user context, and local cleanup still runs when the best-effort store delete fails. `RunStore.create_run_atomic()` remains a deprecated compatibility shim for external stores that only admit normal runs; new stores must implement `create_thread_operation_atomic()` to support internal operation kinds. +- Run admission and independent writes are first-class thread operations. `runs.operation_kind` distinguishes user-visible `run` rows from internal `checkpoint_write` and `artifact_write` reservations, while every active kind shares the existing durable active-thread uniqueness constraint. New operation kinds must go through `RunStore.create_thread_operation_atomic()` and `RunManager.reserve_thread_operation()` rather than adding another lock or metadata marker. Live and lease-less reservations are non-interruptible; an expired leased reservation can be reclaimed immediately by interrupt/rollback admission without waiting for orphan reconciliation. Lease-less rows stay fail-closed because the store cannot distinguish a stale row from a live writer in another heartbeat-disabled worker; a rare failed delete therefore requires startup reconciliation, and heartbeat-disabled multi-worker deployment remains unsupported. Reservation bodies are attached to their caller task so loss detected by lease renewal cancels the writer before it can continue after takeover; the context manager translates that lease-loss cancellation to `ConflictError` after cleanup so Gateway mutation routes return a retryable 409 instead of dropping the HTTP request. The cleanup scope begins immediately after durable admission, including the await that attaches the caller task, so cancellation cannot strand a locally renewed pending reservation. A failed renewal is revalidated under the manager lock before cancellation; if the reservation completed and unregistered while the store update was in flight, its request task must not be cancelled after the write. Reservations are excluded from run history/reporting and from run-only helpers such as `list_by_thread()` and `has_inflight()`, release uses the captured owner rather than ambient user context, and local cleanup still runs when the best-effort store delete fails. `RunStore.create_run_atomic()` remains a deprecated compatibility shim for external stores that only admit normal runs; new stores must implement `create_thread_operation_atomic()` to support internal operation kinds. - Gateway checkpoint mutations outside run execution must use `services.reserve_checkpoint_write()`, which composes the process-local thread lock with the durable `checkpoint_write` reservation. Manual compaction, `POST /threads/{id}/state`, and both goal mutation routes (`PUT` / `DELETE /threads/{id}/goal`, including creation of a missing goal checkpoint) use this boundary, so an existing run blocks the write and the reservation blocks new reject/interrupt/rollback runs across workers. - `POST /wait` (both thread-scoped and `/api/runs/wait`) drains the stream bridge via `wait_for_run_completion()` instead of bare `await record.task`, so it honours the run's `on_disconnect` setting and cancels the background run on real client disconnect rather than returning a stale checkpoint (issue #3265). - Memory and Redis `StreamBridge` implementations retain only `stream_bridge.queue_maxsize` data events. A syntactically valid `Last-Event-ID` older than the retained watermark, or a live subscriber that falls behind it, yields `StreamGap` before any partial replay. `sse_consumer` maps that control item to an id-less SSE `gap` payload (`stream_replay_gap`) and intentionally leaves the run active; internal `/wait` consumers resume from its latest retained ID because they only need terminal completion. Redis checks bounds plus the non-blocking read in one transaction, using blocking `XREAD` only as a wake-up before repeating the atomic snapshot. For a no-cursor subscriber that established a wait on an empty stream, the first wake response remains provisional until that next snapshot verifies its tail is still retained; this closes the pre-first-delivery trimming window without changing malformed-cursor live tailing. The correctness tradeoff is one three-command snapshot pipeline per poll plus the blocking wake round trip while idle. Malformed cursor behavior remains backend-specific. Memory treats a syntactically numeric cursor below its watermark conservatively as a gap even when the evicted timestamp can no longer be verified; unknown ids at or above the watermark retain the legacy replay-from-earliest policy. diff --git a/backend/app/gateway/routers/artifacts.py b/backend/app/gateway/routers/artifacts.py index d39c066e3..35019b97d 100644 --- a/backend/app/gateway/routers/artifacts.py +++ b/backend/app/gateway/routers/artifacts.py @@ -1,17 +1,28 @@ import asyncio +import hashlib import logging import mimetypes +import os +import stat +import tempfile import zipfile +from collections.abc import AsyncIterator +from contextlib import asynccontextmanager from pathlib import Path from urllib.parse import quote from fastapi import APIRouter, HTTPException, Request from fastapi.responses import FileResponse, PlainTextResponse, Response +from pydantic import BaseModel, Field from app.gateway.authz import require_permission +from app.gateway.deps import get_run_manager from app.gateway.internal_auth import get_trusted_internal_owner_user_id from app.gateway.path_utils import resolve_thread_virtual_path from deerflow.config.paths import make_safe_user_id +from deerflow.runtime import ConflictError, ThreadOperationKind +from deerflow.runtime.user_context import get_effective_user_id +from deerflow.sandbox.sandbox_provider import get_sandbox_provider logger = logging.getLogger(__name__) @@ -25,6 +36,110 @@ ACTIVE_CONTENT_MIME_TYPES = { MAX_SKILL_ARCHIVE_MEMBER_BYTES = 16 * 1024 * 1024 _SKILL_ARCHIVE_READ_CHUNK_SIZE = 64 * 1024 +MAX_EDITABLE_ARTIFACT_BYTES = 2 * 1024 * 1024 +_EDITABLE_OUTPUTS_PREFIX = "mnt/user-data/outputs/" +_ARTIFACT_EDIT_TEMP_PREFIX = ".artifact-edit-" + + +class ArtifactUpdateRequest(BaseModel): + content: str + expected_sha256: str = Field(pattern=r"^[0-9a-f]{64}$") + + +class ArtifactUpdateResponse(BaseModel): + path: str + sha256: str + size: int + + +@asynccontextmanager +async def reserve_artifact_write(request: Request, thread_id: str, *, user_id: str) -> AsyncIterator[None]: + """Serialize an artifact edit against runs and other thread mutations.""" + run_manager = get_run_manager(request) + async with run_manager.reserve_thread_operation( + thread_id, + kind=ThreadOperationKind.artifact_write, + user_id=user_id, + ): + yield + + +def _normalize_editable_artifact_path(path: str) -> str: + stripped = path.lstrip("/") + if not stripped.startswith(_EDITABLE_OUTPUTS_PREFIX): + raise HTTPException(status_code=400, detail="Only files in /mnt/user-data/outputs can be edited") + if ".skill/" in stripped or stripped.endswith(".skill"): + raise HTTPException(status_code=415, detail="Skill archives cannot be edited in the artifacts panel") + return f"/{stripped}" + + +def _load_editable_artifact(actual_path: Path, path: str, expected_sha256: str) -> tuple[bytes, os.stat_result]: + try: + file_stat = os.lstat(actual_path) + except FileNotFoundError: + raise HTTPException(status_code=404, detail=f"Artifact not found: {path}") from None + if stat.S_ISLNK(file_stat.st_mode): + raise HTTPException(status_code=415, detail="Symlinked artifacts cannot be edited") + if not stat.S_ISREG(file_stat.st_mode): + raise HTTPException(status_code=400, detail=f"Path is not a file: {path}") + if file_stat.st_size > MAX_EDITABLE_ARTIFACT_BYTES: + raise HTTPException(status_code=413, detail="Artifact is too large to edit") + + current = actual_path.read_bytes() + if len(current) > MAX_EDITABLE_ARTIFACT_BYTES: + raise HTTPException(status_code=413, detail="Artifact is too large to edit") + if b"\x00" in current: + raise HTTPException(status_code=415, detail="Binary artifacts cannot be edited") + try: + current.decode("utf-8") + except UnicodeDecodeError: + raise HTTPException(status_code=415, detail="Only UTF-8 text artifacts can be edited") from None + + current_sha256 = hashlib.sha256(current).hexdigest() + if current_sha256 != expected_sha256: + raise HTTPException(status_code=412, detail="Artifact changed since it was opened") + return current, file_stat + + +def _encode_artifact_update(content: str) -> bytes: + encoded = content.encode("utf-8") + if len(encoded) > MAX_EDITABLE_ARTIFACT_BYTES: + raise HTTPException(status_code=413, detail="Artifact is too large to edit") + if b"\x00" in encoded: + raise HTTPException(status_code=415, detail="Binary content cannot be saved as an artifact") + return encoded + + +def _replace_artifact_atomically(actual_path: Path, content: bytes, file_stat: os.stat_result) -> None: + temp_fd, temp_path_str = tempfile.mkstemp(prefix=_ARTIFACT_EDIT_TEMP_PREFIX, dir=actual_path.parent) + temp_path = Path(temp_path_str) + try: + # Preserve ownership where possible and keep replacement permissions + # scoped to the owner/group. The shared outputs directory allows a + # mounted sandbox to reach the file without making it world-writable. + if hasattr(os, "fchown"): + try: + os.fchown(temp_fd, file_stat.st_uid, file_stat.st_gid) + except OSError: + logger.debug("Could not preserve artifact ownership: %s", actual_path, exc_info=True) + os.fchmod(temp_fd, stat.S_IMODE(file_stat.st_mode) | 0o660) + with os.fdopen(temp_fd, "wb") as handle: + temp_fd = -1 + handle.write(content) + handle.flush() + os.fsync(handle.fileno()) + os.replace(temp_path, actual_path) + finally: + if temp_fd >= 0: + os.close(temp_fd) + try: + temp_path.unlink() + except FileNotFoundError: + pass + + +def _sync_artifact_to_sandbox(sandbox, virtual_path: str, content: bytes) -> None: + sandbox.update_file(virtual_path, content) def _build_content_disposition(disposition_type: str, filename: str) -> str: @@ -255,6 +370,85 @@ async def get_artifact(thread_id: str, path: str, request: Request, download: bo ) if kind == "text": - return PlainTextResponse(content=payload, media_type=mime_type) + assert isinstance(payload, str) + content_sha256 = hashlib.sha256(payload.encode("utf-8")).hexdigest() + return PlainTextResponse(content=payload, media_type=mime_type, headers={"ETag": f'"{content_sha256}"'}) raise AssertionError(f"Unhandled artifact response kind: {kind!r}") + + +@router.put( + "/threads/{thread_id}/artifacts/{path:path}", + response_model=ArtifactUpdateResponse, + summary="Update Artifact File", + description="Replace an existing UTF-8 text artifact after verifying that its content has not changed.", +) +@require_permission("threads", "write", owner_check=True, require_existing=True) +async def update_artifact( + thread_id: str, + path: str, + body: ArtifactUpdateRequest, + request: Request, +) -> ArtifactUpdateResponse: + """Update an existing text artifact while the thread has no active run.""" + virtual_path = _normalize_editable_artifact_path(path) + raw_owner_user_id = get_trusted_internal_owner_user_id(request) + effective_user_id = make_safe_user_id(raw_owner_user_id) if raw_owner_user_id else get_effective_user_id() + + sandbox_provider = None + sandbox_id: str | None = None + sandbox = None + try: + async with reserve_artifact_write(request, thread_id, user_id=effective_user_id): + actual_path = await asyncio.to_thread( + resolve_thread_virtual_path, + thread_id, + virtual_path, + user_id=effective_user_id, + ) + current, file_stat = await asyncio.to_thread( + _load_editable_artifact, + actual_path, + virtual_path, + body.expected_sha256, + ) + updated = _encode_artifact_update(body.content) + + sandbox_provider = get_sandbox_provider() + if not bool(getattr(sandbox_provider, "uses_thread_data_mounts", False)): + sandbox_id = await sandbox_provider.acquire_async(thread_id, user_id=effective_user_id) + sandbox = sandbox_provider.get(sandbox_id) + if sandbox is None: + raise RuntimeError("Failed to acquire sandbox for artifact update") + + try: + if sandbox is not None: + await asyncio.to_thread(_sync_artifact_to_sandbox, sandbox, virtual_path, updated) + await asyncio.to_thread(_replace_artifact_atomically, actual_path, updated, file_stat) + except Exception: + if sandbox is not None: + try: + await asyncio.to_thread(_sync_artifact_to_sandbox, sandbox, virtual_path, current) + except Exception: + logger.exception("Failed to roll back remote artifact after artifact update failure: %s", virtual_path) + raise + except ConflictError: + raise HTTPException(status_code=409, detail="Thread has a run in flight. Save after the run finishes.") from None + except HTTPException: + raise + except Exception: + logger.exception("Failed to update artifact %s for thread %s", path, thread_id) + raise HTTPException(status_code=500, detail="Failed to update artifact") from None + finally: + if sandbox_id is not None and sandbox_provider is not None: + try: + await asyncio.to_thread(sandbox_provider.release, sandbox_id) + except Exception: + logger.warning("Failed to release sandbox after artifact update: %s", sandbox_id, exc_info=True) + + content_sha256 = hashlib.sha256(updated).hexdigest() + return ArtifactUpdateResponse( + path=virtual_path, + sha256=content_sha256, + size=len(updated), + ) diff --git a/backend/packages/harness/deerflow/runtime/runs/schemas.py b/backend/packages/harness/deerflow/runtime/runs/schemas.py index 028bcb1ad..2adcb99b7 100644 --- a/backend/packages/harness/deerflow/runtime/runs/schemas.py +++ b/backend/packages/harness/deerflow/runtime/runs/schemas.py @@ -8,6 +8,7 @@ class ThreadOperationKind(StrEnum): run = "run" checkpoint_write = "checkpoint_write" + artifact_write = "artifact_write" class RunStatus(StrEnum): diff --git a/backend/tests/blocking_io/test_artifacts_router.py b/backend/tests/blocking_io/test_artifacts_router.py index 7bb0d6751..393217763 100644 --- a/backend/tests/blocking_io/test_artifacts_router.py +++ b/backend/tests/blocking_io/test_artifacts_router.py @@ -26,19 +26,23 @@ sit at module top so any import-time IO runs at collection, outside the gate. from __future__ import annotations import asyncio +import hashlib import zipfile +from contextlib import asynccontextmanager from pathlib import Path import pytest from starlette.responses import FileResponse +import app.gateway.routers.artifacts as artifacts_router from app.gateway.path_utils import resolve_thread_virtual_path -from app.gateway.routers.artifacts import get_artifact +from app.gateway.routers.artifacts import ArtifactUpdateRequest, get_artifact, update_artifact pytestmark = pytest.mark.asyncio # The undecorated coroutine (``require_permission`` uses ``functools.wraps``). _get_artifact = get_artifact.__wrapped__ +_update_artifact = update_artifact.__wrapped__ async def _seed(tmp_path: Path, monkeypatch, thread_id: str, virtual_path: str) -> Path: @@ -96,3 +100,33 @@ async def test_get_artifact_skill_archive_member_does_not_block_event_loop(tmp_p assert resp.status_code == 200 assert b"# demo skill" in resp.body + + +async def test_update_artifact_does_not_block_event_loop(tmp_path: Path, monkeypatch) -> None: + vpath = "/mnt/user-data/outputs/notes.txt" + target = await _seed(tmp_path, monkeypatch, "t1", vpath) + original = b"hello world" + await asyncio.to_thread(target.write_bytes, original) + + @asynccontextmanager + async def allow_write(*_args, **_kwargs): + yield + + class MountedProvider: + uses_thread_data_mounts = True + + monkeypatch.setattr(artifacts_router, "reserve_artifact_write", allow_write) + monkeypatch.setattr(artifacts_router, "get_sandbox_provider", lambda: MountedProvider()) + + result = await _update_artifact( + "t1", + vpath, + ArtifactUpdateRequest( + content="updated", + expected_sha256=hashlib.sha256(original).hexdigest(), + ), + request=None, + ) + + assert result.sha256 == hashlib.sha256(b"updated").hexdigest() + assert await asyncio.to_thread(target.read_bytes) == b"updated" diff --git a/backend/tests/test_artifacts_router.py b/backend/tests/test_artifacts_router.py index 6f5864c81..3a4b14922 100644 --- a/backend/tests/test_artifacts_router.py +++ b/backend/tests/test_artifacts_router.py @@ -1,5 +1,8 @@ import asyncio +import hashlib +import stat import zipfile +from contextlib import asynccontextmanager from pathlib import Path from types import SimpleNamespace @@ -44,6 +47,265 @@ def test_get_artifact_reads_utf8_text_file_on_windows_locale(tmp_path, monkeypat assert bytes(response.body).decode("utf-8") == text assert response.media_type == "text/plain" + assert response.headers["etag"] == f'"{hashlib.sha256(text.encode("utf-8")).hexdigest()}"' + + +@asynccontextmanager +async def _allow_artifact_write(*_args, **_kwargs): + yield + + +class _MountedSandboxProvider: + uses_thread_data_mounts = True + + +class _RemoteSandbox: + def __init__(self, *, fail_next_update: bool = False) -> None: + self.updates: list[tuple[str, bytes]] = [] + self.fail_next_update = fail_next_update + + def update_file(self, path: str, content: bytes) -> None: + if self.fail_next_update: + self.fail_next_update = False + raise RuntimeError("sandbox sync failed") + self.updates.append((path, content)) + + +class _RemoteSandboxProvider: + uses_thread_data_mounts = False + + def __init__(self, *, fail_next_update: bool = False) -> None: + self.sandbox = _RemoteSandbox(fail_next_update=fail_next_update) + self.released: list[str] = [] + + async def acquire_async(self, _thread_id: str, *, user_id: str | None = None) -> str: + return "sandbox-1" + + def get(self, sandbox_id: str): + assert sandbox_id == "sandbox-1" + return self.sandbox + + def release(self, sandbox_id: str) -> None: + self.released.append(sandbox_id) + + +def _artifact_sha256(content: str) -> str: + return hashlib.sha256(content.encode("utf-8")).hexdigest() + + +def _patch_artifact_update_dependencies(monkeypatch, artifact_path: Path, provider=None) -> None: + monkeypatch.setattr(artifacts_router, "resolve_thread_virtual_path", lambda _thread_id, _path, user_id=None: artifact_path) + monkeypatch.setattr(artifacts_router, "reserve_artifact_write", _allow_artifact_write) + monkeypatch.setattr(artifacts_router, "get_sandbox_provider", lambda: provider or _MountedSandboxProvider()) + + +def test_update_artifact_replaces_utf8_text_atomically(tmp_path, monkeypatch) -> None: + artifact_path = tmp_path / "note.txt" + artifact_path.write_text("before", encoding="utf-8") + artifact_path.chmod(0o600) + _patch_artifact_update_dependencies(monkeypatch, artifact_path) + + response = asyncio.run( + call_unwrapped( + artifacts_router.update_artifact, + "thread-1", + "mnt/user-data/outputs/note.txt", + artifacts_router.ArtifactUpdateRequest(content="after", expected_sha256=_artifact_sha256("before")), + _make_request(), + ) + ) + + assert artifact_path.read_text(encoding="utf-8") == "after" + assert response.path == "/mnt/user-data/outputs/note.txt" + assert response.sha256 == _artifact_sha256("after") + assert response.size == len(b"after") + replacement_mode = stat.S_IMODE(artifact_path.stat().st_mode) + assert replacement_mode == 0o660 + assert not replacement_mode & stat.S_IWOTH + + +def test_update_artifact_rejects_stale_revision_without_changing_file(tmp_path, monkeypatch) -> None: + artifact_path = tmp_path / "note.txt" + artifact_path.write_text("agent version", encoding="utf-8") + _patch_artifact_update_dependencies(monkeypatch, artifact_path) + + with pytest.raises(HTTPException) as exc_info: + asyncio.run( + call_unwrapped( + artifacts_router.update_artifact, + "thread-1", + "mnt/user-data/outputs/note.txt", + artifacts_router.ArtifactUpdateRequest(content="user version", expected_sha256=_artifact_sha256("old version")), + _make_request(), + ) + ) + + assert exc_info.value.status_code == 412 + assert artifact_path.read_text(encoding="utf-8") == "agent version" + + +def test_update_artifact_rejects_non_output_path(tmp_path, monkeypatch) -> None: + artifact_path = tmp_path / "note.txt" + artifact_path.write_text("before", encoding="utf-8") + _patch_artifact_update_dependencies(monkeypatch, artifact_path) + + with pytest.raises(HTTPException) as exc_info: + asyncio.run( + call_unwrapped( + artifacts_router.update_artifact, + "thread-1", + "mnt/user-data/workspace/note.txt", + artifacts_router.ArtifactUpdateRequest(content="after", expected_sha256=_artifact_sha256("before")), + _make_request(), + ) + ) + + assert exc_info.value.status_code == 400 + assert artifact_path.read_text(encoding="utf-8") == "before" + + +def test_update_artifact_rejects_binary_file(tmp_path, monkeypatch) -> None: + artifact_path = tmp_path / "blob.bin" + artifact_path.write_bytes(b"before\x00binary") + _patch_artifact_update_dependencies(monkeypatch, artifact_path) + + with pytest.raises(HTTPException) as exc_info: + asyncio.run( + call_unwrapped( + artifacts_router.update_artifact, + "thread-1", + "mnt/user-data/outputs/blob.bin", + artifacts_router.ArtifactUpdateRequest(content="after", expected_sha256=hashlib.sha256(b"before\x00binary").hexdigest()), + _make_request(), + ) + ) + + assert exc_info.value.status_code == 415 + + +def test_update_artifact_syncs_non_mounted_sandbox(tmp_path, monkeypatch) -> None: + artifact_path = tmp_path / "note.txt" + artifact_path.write_text("before", encoding="utf-8") + provider = _RemoteSandboxProvider() + _patch_artifact_update_dependencies(monkeypatch, artifact_path, provider) + + asyncio.run( + call_unwrapped( + artifacts_router.update_artifact, + "thread-1", + "mnt/user-data/outputs/note.txt", + artifacts_router.ArtifactUpdateRequest(content="after", expected_sha256=_artifact_sha256("before")), + _make_request(), + ) + ) + + assert provider.sandbox.updates == [("/mnt/user-data/outputs/note.txt", b"after")] + assert provider.released == ["sandbox-1"] + assert artifact_path.read_text(encoding="utf-8") == "after" + + +def test_update_artifact_releases_sandbox_when_initial_sync_fails(tmp_path, monkeypatch) -> None: + artifact_path = tmp_path / "note.txt" + artifact_path.write_text("before", encoding="utf-8") + provider = _RemoteSandboxProvider(fail_next_update=True) + _patch_artifact_update_dependencies(monkeypatch, artifact_path, provider) + + with pytest.raises(HTTPException) as exc_info: + asyncio.run( + call_unwrapped( + artifacts_router.update_artifact, + "thread-1", + "mnt/user-data/outputs/note.txt", + artifacts_router.ArtifactUpdateRequest(content="after", expected_sha256=_artifact_sha256("before")), + _make_request(), + ) + ) + + assert exc_info.value.status_code == 500 + assert provider.released == ["sandbox-1"] + assert provider.sandbox.updates == [("/mnt/user-data/outputs/note.txt", b"before")] + assert artifact_path.read_text(encoding="utf-8") == "before" + + +def test_update_artifact_rolls_back_remote_when_local_replace_fails(tmp_path, monkeypatch) -> None: + artifact_path = tmp_path / "note.txt" + artifact_path.write_text("before", encoding="utf-8") + provider = _RemoteSandboxProvider() + _patch_artifact_update_dependencies(monkeypatch, artifact_path, provider) + + def fail_replace(*_args, **_kwargs) -> None: + raise OSError("replace failed") + + monkeypatch.setattr(artifacts_router, "_replace_artifact_atomically", fail_replace) + + with pytest.raises(HTTPException) as exc_info: + asyncio.run( + call_unwrapped( + artifacts_router.update_artifact, + "thread-1", + "mnt/user-data/outputs/note.txt", + artifacts_router.ArtifactUpdateRequest(content="after", expected_sha256=_artifact_sha256("before")), + _make_request(), + ) + ) + + assert exc_info.value.status_code == 500 + assert provider.sandbox.updates == [ + ("/mnt/user-data/outputs/note.txt", b"after"), + ("/mnt/user-data/outputs/note.txt", b"before"), + ] + assert provider.released == ["sandbox-1"] + assert artifact_path.read_text(encoding="utf-8") == "before" + + +def test_update_artifact_rejects_oversized_content(tmp_path, monkeypatch) -> None: + artifact_path = tmp_path / "note.txt" + artifact_path.write_text("before", encoding="utf-8") + _patch_artifact_update_dependencies(monkeypatch, artifact_path) + + with pytest.raises(HTTPException) as exc_info: + asyncio.run( + call_unwrapped( + artifacts_router.update_artifact, + "thread-1", + "mnt/user-data/outputs/note.txt", + artifacts_router.ArtifactUpdateRequest( + content="x" * (artifacts_router.MAX_EDITABLE_ARTIFACT_BYTES + 1), + expected_sha256=_artifact_sha256("before"), + ), + _make_request(), + ) + ) + + assert exc_info.value.status_code == 413 + assert artifact_path.read_text(encoding="utf-8") == "before" + + +def test_update_artifact_reports_active_run_conflict(tmp_path, monkeypatch) -> None: + artifact_path = tmp_path / "note.txt" + artifact_path.write_text("before", encoding="utf-8") + monkeypatch.setattr(artifacts_router, "resolve_thread_virtual_path", lambda _thread_id, _path, user_id=None: artifact_path) + + @asynccontextmanager + async def reject_artifact_write(*_args, **_kwargs): + raise artifacts_router.ConflictError("active run") + yield + + monkeypatch.setattr(artifacts_router, "reserve_artifact_write", reject_artifact_write) + + with pytest.raises(HTTPException) as exc_info: + asyncio.run( + call_unwrapped( + artifacts_router.update_artifact, + "thread-1", + "mnt/user-data/outputs/note.txt", + artifacts_router.ArtifactUpdateRequest(content="after", expected_sha256=_artifact_sha256("before")), + _make_request(), + ) + ) + + assert exc_info.value.status_code == 409 + assert artifact_path.read_text(encoding="utf-8") == "before" @pytest.mark.parametrize(("filename", "content"), ACTIVE_ARTIFACT_CASES) diff --git a/frontend/AGENTS.md b/frontend/AGENTS.md index b38e706af..9badb84f5 100644 --- a/frontend/AGENTS.md +++ b/frontend/AGENTS.md @@ -70,6 +70,7 @@ The frontend is a stateful chat application. Users create **threads** (conversat File-tool artifact auto-open work must run in an effect with timer cleanup; never schedule timers while rendering streamed `write_file` or `str_replace` updates. `ThreadState.artifacts` remains the authoritative artifact list. The artifacts provider persists only thread-scoped panel UI state (`open`, selected path, and a refresh bootstrap cache) in session storage; an initial empty stream value must not overwrite that restored state before history finishes loading. Formal artifact content is refreshed once when the run finishes; transient `write-file:` previews remain message-driven. + The detail view exposes explicit editing only for an already-opened formal UTF-8 text artifact under `/mnt/user-data/outputs`. Drafts stay in provider memory until Save so switching right-side panels cannot discard them, render in Markdown/HTML preview, and are protected from remote refreshes by the loaded SHA-256 revision. Saving is disabled during an active run; a changed revision preserves the draft and surfaces a conflict instead of overwriting agent output. 3. `useThreadHistory` loads persisted conversation pages from `GET /api/threads/{id}/messages/page`, preserving the backend's thread-global event `seq`; rendering overlays checkpoint/live copies at their matching canonical identities (a summarized checkpoint may contain a protected early input plus a recent tail). Context-compaction rescue diffs every retained visible identity rather than slicing at the first anchor, and keeps a run-scoped ledger of committed visible messages so replacement updates and repeated rolling checkpoint windows cannot erase an already displayed step. The resolver suppresses checkpoint/transient prefixes whose canonical position is still behind an unloaded cursor page instead of collapsing that unknown gap before a recent anchor, then adds optimistic messages without timestamp re-sorting. History invalidation preserves already-loaded pages so their established ordering positions are not discarded. Dynamic context re-keys the submitted user message from `X` to `X__user`; UI identity matching normalizes that reserved suffix only for human messages so the submitted frame and checkpoint replacement remain one visible turn. A locally submitted turn also records its pre-submit identity baseline: if `messages-tuple` publishes new AI/tool steps before `values` publishes that turn's human message, render ordering moves only those non-baseline visible steps behind the new human while leaving history, hidden controls, and reconnected runs untouched. Keep that local order anchor through finish, stop, and stream error because the SDK's settled frame can retain transient event order; replace it on the next local submit and clear it on thread switch or replay-gap recovery. 4. Stop actions call the LangGraph SDK stream stop path; `core/threads/hooks.ts` invalidates current-thread, thread-history, token-usage, and sidebar/search caches immediately and schedules one follow-up refetch because SDK stop may finish via abort + fire-and-forget cancel before backend title finalization commits 5. TanStack Query manages server state; localStorage stores user settings. The diff --git a/frontend/src/components/workspace/artifacts/artifact-file-detail.tsx b/frontend/src/components/workspace/artifacts/artifact-file-detail.tsx index 3d257e226..0d170f316 100644 --- a/frontend/src/components/workspace/artifacts/artifact-file-detail.tsx +++ b/frontend/src/components/workspace/artifacts/artifact-file-detail.tsx @@ -1,3 +1,4 @@ +import { useQueryClient } from "@tanstack/react-query"; import { Code2Icon, CopyIcon, @@ -5,6 +6,10 @@ import { EyeIcon, LoaderIcon, PackageIcon, + PencilIcon, + PencilOffIcon, + RotateCcwIcon, + SaveIcon, SquareArrowOutUpRightIcon, XIcon, } from "lucide-react"; @@ -29,6 +34,15 @@ import { } from "@/components/ui/select"; import { ToggleGroup, ToggleGroupItem } from "@/components/ui/toggle-group"; import { CodeEditor } from "@/components/workspace/code-editor"; +import { + ArtifactRequestError, + updateArtifactContent, +} from "@/core/artifacts/api"; +import { + canEditOpenedArtifact, + createArtifactDraft, + reconcileArtifactDraft, +} from "@/core/artifacts/editing"; import { useArtifactContent } from "@/core/artifacts/hooks"; import { appendHtmlPreviewBaseHref, @@ -78,9 +92,18 @@ export function ArtifactFileDetail({ threadId: string; }) { const { t } = useI18n(); + const queryClient = useQueryClient(); const { user } = useAuth(); const isAdmin = user?.system_role === "admin"; - const { artifacts, setOpen, select } = useArtifacts(); + const { + artifacts, + setOpen, + select, + drafts, + setDrafts, + editingPath, + setEditingPath, + } = useArtifacts(); const { thread, isMock } = useThread(); const isWriteFile = useMemo(() => { return filepathFromProps.startsWith("write-file:"); @@ -170,7 +193,7 @@ export function ArtifactFileDetail({ isSupportPreview, toolResult, }); - const { content, url } = useArtifactContent({ + const { content, url, sha256 } = useArtifactContent({ threadId, filepath: filepathFromProps, enabled: isCodeFile && !isWriteFile, @@ -184,6 +207,38 @@ export function ArtifactFileDetail({ filepathFromProps, ); + const [isSaving, setIsSaving] = useState(false); + const activeDraft = drafts[filepath] ?? createArtifactDraft(filepath); + const isDirty = activeDraft.draftContent !== activeDraft.baselineContent; + const hasUnsavedDrafts = Object.values(drafts).some( + (draft) => draft.draftContent !== draft.baselineContent, + ); + const isEditing = editingPath === filepath; + const canEdit = canEditOpenedArtifact({ + filepath, + isCodeFile, + isWriteFile, + isSkillFile, + isMock: Boolean(isMock), + hasRevision: typeof sha256 === "string", + isStaticWebsite: env.NEXT_PUBLIC_STATIC_WEBSITE_ONLY === "true", + }); + const editorContent = isDirty ? activeDraft.draftContent : visibleContent; + + useEffect(() => { + if (content === undefined || sha256 === undefined || isWriteFile) { + return; + } + setDrafts((current) => { + const existing = current[filepath] ?? createArtifactDraft(filepath); + const next = reconcileArtifactDraft(existing, { content, sha256 }); + if (next === existing) { + return current; + } + return { ...current, [filepath]: next }; + }); + }, [content, filepath, isWriteFile, setDrafts, sha256]); + const [viewMode, setViewMode] = useState<"code" | "preview">( artifactViewState.initialViewMode, ); @@ -192,6 +247,108 @@ export function ArtifactFileDetail({ setViewMode(artifactViewState.initialViewMode); }, [artifactViewState.initialViewMode]); + const confirmDiscard = useCallback(() => { + return !isDirty || window.confirm(t.artifactEditing.discardChanges); + }, [isDirty, t.artifactEditing.discardChanges]); + + const discardDraft = useCallback(() => { + const latestContent = content ?? activeDraft.baselineContent; + const latestSha256 = sha256 ?? activeDraft.baselineSha256; + setDrafts((current) => ({ + ...current, + [filepath]: { + ...activeDraft, + baselineContent: latestContent, + baselineSha256: latestSha256, + draftContent: latestContent, + conflict: false, + }, + })); + setEditingPath(null); + }, [activeDraft, content, filepath, setDrafts, setEditingPath, sha256]); + + const handleSave = useCallback(async () => { + if ( + !canEdit || + !isDirty || + isSaving || + thread.isLoading || + activeDraft.conflict || + activeDraft.baselineSha256 === null + ) { + return; + } + + setIsSaving(true); + try { + const result = await updateArtifactContent({ + threadId, + filepath, + content: activeDraft.draftContent, + expectedSha256: activeDraft.baselineSha256, + }); + const savedContent = activeDraft.draftContent; + setDrafts((current) => ({ + ...current, + [filepath]: { + filepath, + baselineContent: savedContent, + baselineSha256: result.sha256, + draftContent: savedContent, + conflict: false, + }, + })); + queryClient.setQueryData( + ["artifact", filepathFromProps, threadId, isMock], + ( + current: + | { content?: string; url?: string; sha256?: string } + | undefined, + ) => ({ + ...current, + content: savedContent, + sha256: result.sha256, + }), + ); + toast.success(t.artifactEditing.saved); + } catch (error) { + if (error instanceof ArtifactRequestError && error.status === 412) { + setDrafts((current) => ({ + ...current, + [filepath]: { ...(current[filepath] ?? activeDraft), conflict: true }, + })); + void queryClient.invalidateQueries({ + queryKey: ["artifact", filepathFromProps, threadId, isMock], + }); + toast.error(t.artifactEditing.conflict); + } else if ( + error instanceof ArtifactRequestError && + error.status === 409 + ) { + toast.error(t.artifactEditing.runInProgress); + } else { + toast.error( + error instanceof Error ? error.message : t.artifactEditing.saveFailed, + ); + } + } finally { + setIsSaving(false); + } + }, [ + activeDraft, + canEdit, + filepath, + filepathFromProps, + isDirty, + isMock, + isSaving, + queryClient, + setDrafts, + t.artifactEditing, + thread.isLoading, + threadId, + ]); + const handleInstallSkill = useCallback(async () => { if (isInstalling) return; @@ -225,7 +382,17 @@ export function ArtifactFileDetail({ {isWriteFile ? (
{getFileName(filepath)}
) : ( - { + if (confirmDiscard()) { + if (isDirty) { + discardDraft(); + } + select(nextFilepath); + } + }} + > @@ -242,7 +409,7 @@ export function ArtifactFileDetail({ )} -
+
{artifactViewState.canPreview && ( )} + {(isSaving || isDirty || activeDraft.conflict) && ( + + {isSaving + ? t.artifactEditing.saving + : activeDraft.conflict + ? t.artifactEditing.conflictShort + : t.artifactEditing.unsaved} + + )}
- {!isWriteFile && filepath.endsWith(".skill") && isAdmin && ( - - - + {canEdit && !isEditing && ( + { + setViewMode("code"); + setEditingPath(filepath); + }} + /> )} - {!isWriteFile && ( + {canEdit && isEditing && ( + <> + void handleSave()} + /> + setEditingPath(null)} + /> + { + if (confirmDiscard()) { + discardDraft(); + } + }} + /> + + )} + {!isEditing && + !isWriteFile && + filepath.endsWith(".skill") && + isAdmin && ( + + + + )} + {!isEditing && !isWriteFile && ( )} - {isCodeFile && ( + {!isEditing && isCodeFile && ( { void (async () => { const didCopy = await writeTextToClipboard( - visibleContent ?? "", + editorContent ?? "", ); if (!didCopy) { toast.error(t.clipboard.failedToCopyToClipboard); @@ -319,7 +559,7 @@ export function ArtifactFileDetail({ tooltip={t.clipboard.copyToClipboard} /> )} - {!isWriteFile && ( + {!isEditing && !isWriteFile && ( setOpen(false)} + onClick={() => { + if ( + !hasUnsavedDrafts || + window.confirm(t.artifactEditing.discardChanges) + ) { + setDrafts({}); + setEditingPath(null); + setOpen(false); + } + }} tooltip={t.common.close} /> @@ -353,7 +602,7 @@ export function ArtifactFileDetail({ viewMode === "preview" && (language === "markdown" || language === "html") && ( { + setDrafts((current) => ({ + ...current, + [filepath]: { + ...(current[filepath] ?? activeDraft), + draftContent: nextContent, + }, + })); + }} + onSave={() => void handleSave()} /> )} {!isCodeFile && canPreviewInBrowser && ( diff --git a/frontend/src/components/workspace/artifacts/context.tsx b/frontend/src/components/workspace/artifacts/context.tsx index 33a236930..694c789b6 100644 --- a/frontend/src/components/workspace/artifacts/context.tsx +++ b/frontend/src/components/workspace/artifacts/context.tsx @@ -1,15 +1,18 @@ import { usePathname } from "next/navigation"; import { createContext, + type Dispatch, useCallback, useContext, useEffect, useRef, useState, type ReactNode, + type SetStateAction, } from "react"; import { useSidebar } from "@/components/ui/sidebar"; +import type { ArtifactDraftState } from "@/core/artifacts/editing"; import { env } from "@/env"; export interface ArtifactsContextType { @@ -24,6 +27,11 @@ export interface ArtifactsContextType { open: boolean; autoOpen: boolean; setOpen: (open: boolean) => void; + + drafts: Record; + setDrafts: Dispatch>>; + editingPath: string | null; + setEditingPath: Dispatch>; } const ArtifactsContext = createContext( @@ -82,6 +90,8 @@ export function ArtifactsProvider({ children }: ArtifactsProviderProps) { env.NEXT_PUBLIC_STATIC_WEBSITE_ONLY === "true", ); const [autoOpen, setAutoOpen] = useState(true); + const [drafts, setDrafts] = useState>({}); + const [editingPath, setEditingPath] = useState(null); const { setOpen: setSidebarOpen } = useSidebar(); const pathname = usePathname(); const hydratedPathRef = useRef(null); @@ -97,9 +107,25 @@ export function ArtifactsProvider({ children }: ArtifactsProviderProps) { setOpen(persisted?.open ?? env.NEXT_PUBLIC_STATIC_WEBSITE_ONLY === "true"); setAutoOpen(true); setAutoSelect(!persisted?.selectedArtifact); + setDrafts({}); + setEditingPath(null); hydratedPathRef.current = pathname; }, [pathname]); + useEffect(() => { + const hasUnsavedDrafts = Object.values(drafts).some( + (draft) => draft.draftContent !== draft.baselineContent, + ); + if (!hasUnsavedDrafts) { + return; + } + const handleBeforeUnload = (event: BeforeUnloadEvent) => { + event.preventDefault(); + }; + window.addEventListener("beforeunload", handleBeforeUnload); + return () => window.removeEventListener("beforeunload", handleBeforeUnload); + }, [drafts]); + useEffect(() => { if (!pathname || hydratedPathRef.current !== pathname) { return; @@ -151,6 +177,11 @@ export function ArtifactsProvider({ children }: ArtifactsProviderProps) { selectedArtifact, select, deselect, + + drafts, + setDrafts, + editingPath, + setEditingPath, }; return ( diff --git a/frontend/src/components/workspace/code-editor.tsx b/frontend/src/components/workspace/code-editor.tsx index 84c558c46..1d4fa5841 100644 --- a/frontend/src/components/workspace/code-editor.tsx +++ b/frontend/src/components/workspace/code-editor.tsx @@ -41,6 +41,8 @@ export function CodeEditor({ readonly, disabled, autoFocus, + onChange, + onSave, settings, }: { className?: string; @@ -49,6 +51,8 @@ export function CodeEditor({ readonly?: boolean; disabled?: boolean; autoFocus?: boolean; + onChange?: (value: string) => void; + onSave?: () => void; settings?: unknown; }) { const { @@ -76,6 +80,15 @@ export function CodeEditor({ "flex cursor-text flex-col overflow-hidden rounded-md", className, )} + onKeyDown={(event) => { + if ( + (event.metaKey || event.ctrlKey) && + event.key.toLowerCase() === "s" + ) { + event.preventDefault(); + onSave?.(); + } + }} > {isLoading ? (