From ad31a2dcd50a776cc9d973c6dbffdc572dc2f1cc Mon Sep 17 00:00:00 2001 From: Yuqing Yang Date: Mon, 31 Aug 2026 03:12:17 +0000 Subject: [PATCH 01/12] docs: propose cross-space selection moves Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/README.md | 1 + .../move-selected-nodes-between-spaces.md | 346 ++++++++++++++++++ 2 files changed, 347 insertions(+) create mode 100644 docs/proposals/move-selected-nodes-between-spaces.md diff --git a/docs/README.md b/docs/README.md index 3d79280fe..ae0b92f82 100644 --- a/docs/README.md +++ b/docs/README.md @@ -94,6 +94,7 @@ docs/ | [managed-agent-teams.md](./proposals/managed-agent-teams.md) | In-Progress | Huabu-managed discovery, configuration, preparation, and runtime. | | [milkdown-custom-toolbar-plan.md](./proposals/milkdown-custom-toolbar-plan.md) | In-Progress | Huabu-owned Milkdown toolbar and semantic editor commands. | | [model-role-routing.md](./proposals/model-role-routing.md) | Proposed | Model selection by runtime role. | +| [move-selected-nodes-between-spaces.md](./proposals/move-selected-nodes-between-spaces.md) | Proposed | #142 selected-node and Frame-subtree moves between Spaces with bounded compensation. | | [multi-backend-storage.md](./proposals/multi-backend-storage.md) | Partly shipped | Phases 1–3: Blob, structured repositories, catalogue, and bounded reads. | | [note-auto-height-stable-geometry.md](./proposals/note-auto-height-stable-geometry.md) | Proposed | Revision-aware offscreen Note measurement and stable auto-height geometry. | | [space-preview-and-world-redesign.md](./proposals/space-preview-and-world-redesign.md) | In-Progress | View-only Space previews, a preview-based World, and deferred zoom-through navigation. | diff --git a/docs/proposals/move-selected-nodes-between-spaces.md b/docs/proposals/move-selected-nodes-between-spaces.md new file mode 100644 index 000000000..6fe833e10 --- /dev/null +++ b/docs/proposals/move-selected-nodes-between-spaces.md @@ -0,0 +1,346 @@ +# Move Selected Nodes and Frames Between Spaces + +Status: Proposed + +Last updated: 2026-08-31 + +Tracking issue: [#142](https://github.com/microsoft/Huabu/issues/142) + +> **Scope.** This proposal adds the user-facing business operation for moving selected Canvas nodes and Frame subtrees between ordinary Spaces in the active Workspace. It deliberately does not introduce a general multi-Space transaction API, filesystem WAL, crash recovery, Blob reference counting, garbage collection, or multi-backend transaction protocol. + +> **Reliability boundary.** The operation provides user-visible all-or-compensated behavior while the Server process continues running and returns a determinate result. Process termination, power loss, and an unknown remote-backend outcome remain outside #142, matching the current `SpaceHandle.write()` contract. + +## 1. Problem + +Huabu can copy a selection through the clipboard and paste it into another Space, but that path is UI-owned reconstruction rather than a move operation. It expands selected Frames, remaps hierarchy and internal edges, and clones cross-Space artifacts, then leaves source deletion to a separate user action. + +That behavior is insufficient for moving work because it has no destination picker, no create-destination flow, no authoritative Server-side selection expansion, no coordinated source deletion, and no durable summary of omitted boundary edges or collision handling. Artifact cloning is intentionally best-effort during paste, so a failed clone may leave a pasted node with a missing-artifact placeholder. + +The storage layer now provides the primitives needed to implement the business operation without redesigning storage: ordinary Space lifecycle, complete Space records, `SpaceNodes.readMany()`, Blob reads and writes, version-checked `SpaceHandle.write()`, and in-process rollback for each rejected Space write. It does not provide a transaction spanning two Spaces or structured and Blob stores. + +## 2. Decision + +Add one Server-owned `SpaceTransferService` and one HTTP execution endpoint. The web gathers the current selection, lets the user choose or create a destination, shows a confirmation derived from the loaded Canvas, drains pending writes, and submits only selected root IDs plus the destination choice. + +The Server re-reads authoritative source and destination state, expands Frame subtrees, allocates fresh destination IDs, clones required artifacts, executes the destination insertion and source deletion while holding both Canvas locks, and delays both Canvas Sync publications until the operation succeeds. + +If a determinate failure occurs after the destination write, the service compensates by applying the destination inverse deltas before releasing the locks. A destination created by the operation is deleted as a whole on failure. This is application-level coordination over the current single-Space guarantees, not a new portable storage transaction contract. + +## 3. Goals + +- Move one or more selected ordinary Canvas nodes into an existing ordinary Space. +- Create a new ordinary destination Space in the same flow. +- Treat selected Frames as subtree roots and preserve every descendant exactly once. +- Preserve parent-child hierarchy, parent-local child geometry, root-to-root relative geometry, node style, Frame layout data, and internal edge style. +- Omit edges that cross the transfer boundary and report them explicitly. +- Clone only artifacts referenced by transferred nodes and rewrite those references for the destination. +- Avoid overwriting destination nodes, edges, sidecars, and artifacts by allocating fresh IDs and de-duplicating labels. +- Keep source nodes unchanged when validation or destination preparation fails. +- Compensate a completed destination insertion when the following source deletion returns a determinate failure. +- Publish no intermediate Canvas Sync state. +- Return an actionable result containing the destination, moved roots and descendants, preserved edges, omitted boundary edges, label changes, and reset runtime state. + +## 4. Non-goals + +- A reusable transaction spanning arbitrary Spaces or storage aggregates. +- ACID guarantees across `StructuredStore` and `BlobStore`. +- Recovery after process termination, power loss, or an unknown backend outcome. +- A filesystem WAL, two-phase commit protocol, transactional outbox, or idempotency ledger. +- Per-key Blob deletion, artifact reference counting, orphan collection, or general Blob GC. +- Moving nodes across Workspaces. +- Moving the World Canvas or managed World projection/reference nodes. +- Moving Tasks, Runs, Agent histories, pending change-review records, Canvas event history, permissions, or unrelated nearby content. +- Replacing or changing Huabu clipboard copy/paste. + +## 5. User experience + +Both the single-selection and multi-selection floating toolbars expose **Move to Space** for movable ordinary nodes. The action is unavailable for `spacePreview`, `canvasRef`, `frameRef`, and `nodeRef` because their identity is owned by Space Preview or legacy World reconciliation rather than ordinary node transfer. + +The modal reuses `Modal`, `Select`, `TextInput`, and `Button` from `apps/web/src/components/Common`. It contains: + +- an existing-Space picker that excludes the current Space and World; +- a **Create new Space** option with an optional title; +- the number and labels of selected roots; +- the number of descendants included through selected Frames; +- the number of internal edges that will be preserved; +- the number of boundary edges that will be omitted; +- a notice that Question conversations and runtime state do not move; +- **Cancel** and **Move** actions. + +The confirmation summary is derived from the loaded Canvas only to explain the requested action. It is not an authorization or persistence plan. The Server repeats every structural check against authoritative state before mutating anything. + +Before submitting, the web drains pending node-content writes and the structure-save queue. The Move action remains disabled while that drain or the request is in progress. + +On success, the source selection disappears through its normal Canvas Sync update. A persistent success toast reports the transferred node and edge counts and offers **Open destination**. On failure, a persistent localized error explains whether the selection became stale, the destination disappeared, validation failed, or the outcome is unknown. + +## 6. Selection and subtree semantics + +The request carries the IDs selected by the user. The Server validates that every requested ID is a live movable node in the source Space. + +The service removes any requested node that already has a requested Frame ancestor. The remaining IDs are the transferred roots. It then recursively includes every descendant of each transferred Frame, regardless of whether that descendant was independently selected. + +This produces one transfer set: + +```text +requested selection + -> remove selected descendants of selected Frames + -> recursively expand selected Frame roots + -> validate one acyclic source subtree forest +``` + +Nodes whose parent is included retain their parent-local `position` and remap `parentId` to the new destination parent ID. A transferred root whose source parent is not included becomes a root node in the destination. + +## 7. Destination placement + +The service resolves each transferred root's absolute source position and computes the bounding box of all transferred roots and their subtrees. It preserves the relative offsets between roots and applies one translation to the complete root set. + +For an empty destination, the transferred bounds begin at a fixed root-space origin such as `{ x: 0, y: 0 }`. For a non-empty destination, the bounds are placed to the right of the destination's current absolute bounds with a fixed gap. This deterministic rule is intentionally simpler than a general collision-free packing algorithm. + +Only root positions receive the translation. Descendant positions remain parent-local, preserving nested Frame geometry and structured-layout assignments. + +## 8. Identity and collision handling + +Every transferred node receives a fresh `node-` ID and every preserved internal edge receives a fresh `edge-` ID. The response includes the old-to-new node mapping for diagnostics and destination navigation. + +Fresh IDs avoid overwriting destination topology or sidecars and prevent a previous copy of the same source node from colliding with the move. Sketch stroke IDs are also regenerated because strokes form a node-local editing identity domain and may later be merged with another sketch. + +Node labels are processed in source tree order using the existing `deduplicateLabel()` behavior against destination labels and labels already allocated during this transfer. The result reports every changed label. Disk sidecar paths continue to derive from the resolved destination labels; callers never construct those paths. + +## 9. Edge behavior + +An edge is internal when both endpoints belong to the transfer set. Internal edges are recreated with remapped endpoints and their complete persisted `edgeStyle`. + +An edge is a boundary edge when exactly one endpoint belongs to the transfer set. Boundary edges are not created in the destination. Deleting the transferred source nodes removes their incident boundary edges from the source through the existing `DELETE_NODES` semantics. + +The result reports omitted boundary edge IDs and endpoint labels where available. Huabu does not create cross-Space edge references or broken placeholder edges. + +## 10. Artifact behavior + +The service discovers artifact references through the shared `ARTIFACT_DATA_FIELDS`, `markdownArtifactFields()`, `collectMarkdownArtifactRefs()`, and `parseArtifactRef()` helpers. Bare keys and legacy Canvas-scoped artifact URLs are cloneable; `data:`, `blob:`, and external `http(s)` values remain unchanged. + +Clone work is deduplicated by normalized source artifact key across the entire transfer. Each destination clone receives a fresh artifact key and every matching node-data or Markdown reference is rewritten to that key. + +Unlike clipboard paste, a required artifact that is absent or cannot be read or written rejects the move before source deletion. The operation never intentionally creates a visible destination node with a missing required artifact. + +The current Blob port has no per-key delete. A failed transfer into an existing destination may therefore leave newly written, unreferenced artifact bytes. They are not reachable from destination topology and are not a partial user-visible copy. Per-key cleanup and orphan GC are deferred to the storage follow-up rather than added to #142. + +Source artifacts are not deleted after success because other source nodes may still reference the same key and the current storage model has no reference counts. + +## 11. Question and runtime-owned state + +A Question node's authored prompt, label, geometry, and visual style move as ordinary node data. Canvas-local runtime fields are removed in the destination: `threadId`, `status`, `runAt`, `errorMessage`, `responseSummary`, and `viewed`. + +The moved Question therefore arrives as a fresh idle Question with no conversation history. Agent history, ACP session mappings, Tasks, Runs, and pending change-review records remain in the source Space and are never traversed by the transfer service. + +This policy avoids a destination node pointing at a thread namespace owned by another Space and keeps #142 independent of Agenetes history migration. + +## 12. HTTP contract + +Add the shared zod contract under `packages/shared/src/types/api/space-transfer.ts` and validate it at the route boundary according to [API Design](../architecture/api-design.md). + +```ts +type MoveSelectionDestination = + | { kind: 'existing'; canvasId: string } + | { kind: 'new'; title?: string }; + +interface MoveSelectionRequest { + selectedNodeIds: string[]; + destination: MoveSelectionDestination; + expectedSourceVersion: number; +} +``` + +The route is: + +```text +POST /api/canvas/:sourceCanvasId/move-selection +``` + +The response contains: + +```ts +interface MoveSelectionResponse { + transferId: string; + destination: { canvasId: string; title: string | null; created: boolean }; + sourceVersion: number; + destinationVersion: number; + roots: Array<{ + sourceNodeId: string; + destinationNodeId: string; + label: string; + }>; + movedNodeCount: number; + movedFrameCount: number; + preservedEdgeCount: number; + omittedBoundaryEdges: Array<{ + edgeId: string; + source: string; + target: string; + }>; + renamedNodes: Array<{ sourceNodeId: string; from: string; to: string }>; + resetQuestionCount: number; +} +``` + +The route uses typed error codes for stale source version, missing source node, invalid node type, missing or same destination, World refusal, missing artifact, destination conflict, compensation failure, and unknown outcome. User-facing text is localized in the web application. + +A separate planning endpoint is intentionally omitted. It would duplicate most reads and validation for a confirmation that the Server must repeat during execution. The loaded Canvas provides the preview; the execution endpoint remains authoritative. + +## 13. Execution sequence + +The service follows this order: + +```text +drain client writes + -> acquire Workspace operation lease + -> resolve/create destination + -> acquire source and destination Canvas locks in sorted-id order + -> read and validate both current Space records + -> expand selection and build transfer model + -> read required node records and artifacts + -> allocate IDs, labels, placement, and rewritten references + -> write fresh destination artifacts + -> execute destination CREATE_NODES + CONNECT_NODES without publication + -> execute source DELETE_NODES without publication + -> publish destination and source updates + -> return result +``` + +The existing `withCanvasMutex()` becomes a small multi-key coordinator that acquires unique Canvas IDs in lexical order. Existing single-Canvas callers continue through the same one-key path. + +The Canvas executor exposes an internal already-locked execution function plus its current public lock-taking wrapper. Transfer calls the already-locked form so both locks remain held across destination insertion, source deletion, and any compensation. Normal executor callers remain unchanged. + +The destination and source operations reuse `CREATE_NODES`, `CONNECT_NODES`, and `DELETE_NODES`; transfer-specific selection expansion, ID mapping, artifact rewriting, and result reporting stay in `SpaceTransferService` rather than becoming a `CanvasCommand`. + +## 14. Determinate failure and compensation + +Validation, source reads, artifact reads, and destination artifact writes occur before source deletion. Failure in those stages leaves the source unchanged and creates no visible destination topology. + +If destination command execution rejects, its existing `SpaceHandle.write()` rollback restores that Space's structured prestate. The source has not yet changed. + +If source deletion rejects after destination insertion committed, the service applies the destination execution's inverse deltas while both Canvas locks remain held. It publishes neither the insertion nor the compensation. If the destination was created by this request, the service deletes that new Space instead of applying inverse deltas. + +If compensation succeeds, the endpoint returns the original source-deletion failure and both Spaces remain user-visible equivalents of their pre-request states, apart from possible unreachable destination Blob bytes. + +If compensation itself fails or the backend outcome becomes unknown, the endpoint returns a distinct persistent-error code and instructs the client to reload both Spaces before retrying. The service does not claim success and the client must not automatically retry. + +## 15. Durability and publication + +The service allocates one `transferId` and includes it as the `runId` on both executor batches. Existing per-Space delta logs therefore provide durable correlation without adding a transfer ledger. + +Canvas Sync updates are constructed from the two executor results and published only after both writes complete. The destination update is published before the source update so a user with both Spaces open never observes source removal before the destination exists. + +The operation does not require exactly-once delivery. After an ambiguous transport failure, the client reloads the source and destination and uses the returned error guidance rather than automatically submitting the request again. + +## 16. Permissions and destination validation + +Huabu currently has one authenticated owner and no per-Space roles. An existing destination is writable when it is an ordinary live Space in the active Workspace and its configured structured and Blob stores admit the required writes. + +World, the source Space itself, a missing Space, and a Space in deletion admission are rejected before topology mutation. A future role or capability model can replace this predicate without changing selection-transfer semantics. + +## 17. Implementation plan + +1. Add shared request, response, and typed-error contracts plus route builders and the web API helper. +2. Extract reusable selection expansion and transfer-model construction from the current clipboard behavior without changing clipboard semantics. +3. Add sorted multi-Canvas lock acquisition and an already-locked Canvas executor entry while preserving all current callers. +4. Implement `SpaceTransferService` using current Space, node, Blob, executor, and lifecycle APIs. +5. Add the move-selection route and delayed paired Canvas Sync publication. +6. Add the modal and toolbar actions with localized English and Chinese strings. +7. Add focused service, route, resolver, and UI regression tests. +8. Fold shipped behavior into `docs/architecture/canvas-storage.md`, `docs/architecture/canvas-command-architecture.md`, and `docs/architecture/web-architecture.md`. + +## 18. Test plan + +### Selection and hierarchy + +- Move one standalone node. +- Move several standalone roots while preserving their relative geometry. +- Move nested Frames and preserve every parent-local position and Frame layout field. +- Select a Frame and one or more descendants and transfer each node once. +- Move a child without its source parent and place it as a destination root using its source absolute position. +- Reject a stale, missing, cyclic, or managed reference selection without mutation. + +### Edges + +- Recreate internal edges with fresh IDs, remapped endpoints, labels, direction, line shape, dash, stroke, and width. +- Omit incoming and outgoing boundary edges and report each omission. +- Remove source incident edges through the existing recursive delete semantics. + +### Identity and collisions + +- Allocate fresh node, edge, Sketch stroke, and artifact IDs. +- De-duplicate labels against destination nodes and earlier nodes in the same transfer. +- Preserve source labels and IDs unchanged until source deletion commits. +- Move a selection previously copied to the destination without overwriting the copy. + +### Artifacts + +- Clone dedicated `src` and `coverUrl` references. +- Rewrite Markdown-embedded image references. +- Clone one source key once when referenced repeatedly. +- Leave external, inline, and Blob URLs unchanged. +- Reject a missing or failed required artifact before source deletion. +- Confirm a failed existing-destination transfer leaves no topology referencing staged artifact keys. + +### Runtime data boundaries + +- Reset Question conversation and run fields while preserving authored content and style. +- Leave Tasks, Runs, chat history, ACP session mappings, change-review records, and Canvas events in the source. + +### Failure behavior + +- Destination write failure leaves source and destination topology unchanged. +- Source deletion failure applies destination inverse deltas and publishes no intermediate update. +- New-destination failure removes the newly created Space. +- Compensation failure returns the distinct unknown-state error and never auto-retries. +- Concurrent writes serialize under sorted dual locks. +- A stale `expectedSourceVersion` rejects before mutation. + +### UI + +- Single and multi-selection toolbars open the same modal. +- Existing Space and create-new flows submit the correct destination variant. +- The confirmation shows normalized roots, descendants, preserved edges, omitted edges, and Question reset notice. +- Pending writes drain before submission. +- Success shows counts and an Open destination action. +- Failure keeps the current source view and shows localized persistent feedback. + +## 19. Validation + +Run focused checks first: + +```bash +pnpm --filter @huabu/shared test -- src/types/api/space-transfer.test.ts +pnpm --filter @huabu/server test -- src/modules/canvas/space-transfer.service.test.ts src/modules/canvas/space-transfer.route.test.ts +pnpm --filter @huabu/web test -- src/components/Panels/Canvas/MoveSelectionModal.test.tsx +pnpm --filter @huabu/shared typecheck +pnpm --filter @huabu/server typecheck +pnpm --filter @huabu/web typecheck +``` + +Before pull-request handoff, run the repository-required checks: + +```bash +pnpm typecheck +pnpm format +pnpm lint:fix +``` + +## 20. Deferred storage follow-up + +A separate storage design should decide whether and how Huabu provides a backend-neutral transaction spanning two Spaces and Blob scopes. That work owns crash recovery, WAL or native SQL transaction mapping, idempotency, transactional publication, per-key Blob deletion, staging, reference counts, retention, and orphan GC. + +That follow-up may later replace the compensation implementation behind `SpaceTransferService`. It must not expand #142 or delay the user-facing business flow defined here. + +## Code entry points + +| File/dir | Responsibility | +| -------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------- | +| [`packages/shared/src/types/api/`](../../packages/shared/src/types/api/) | Shared move-selection request, response, and error contracts. | +| [`apps/server/src/modules/canvas/canvas-executor.ts`](../../apps/server/src/modules/canvas/canvas-executor.ts) | Existing single-Space command execution, persistence, inverse deltas, and Canvas Sync payload construction. | +| [`apps/server/src/modules/canvas/write-coordinator.ts`](../../apps/server/src/modules/canvas/write-coordinator.ts) | Existing per-Canvas mutex to extend with sorted multi-key acquisition. | +| [`apps/server/src/modules/storage/ports/structured.ts`](../../apps/server/src/modules/storage/ports/structured.ts) | Existing Space records, node reads, versioned writes, and reported-failure rollback boundary. | +| [`apps/server/src/modules/storage/ports/blob.ts`](../../apps/server/src/modules/storage/ports/blob.ts) | Existing scoped artifact reads and writes; per-key cleanup remains deferred. | +| [`apps/web/src/store/canvasStore.ts`](../../apps/web/src/store/canvasStore.ts) | Current selection, clipboard subtree expansion, save queues, and Canvas Sync application. | +| [`apps/web/src/components/Panels/Canvas/FloatingToolbars/`](../../apps/web/src/components/Panels/Canvas/FloatingToolbars/) | Single- and multi-selection action entry points. | +| [`apps/web/src/components/Common/`](../../apps/web/src/components/Common/) | Existing modal, picker, input, and button primitives reused by the flow. | From 2ffb5d87b395c50f37bebb4be6b610bccb022768 Mon Sep 17 00:00:00 2001 From: Yuqing Yang Date: Wed, 2 Sep 2026 01:53:36 +0000 Subject: [PATCH 02/12] docs: refine cross-space move proposal Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../move-selected-nodes-between-spaces.md | 156 ++++++++++++------ 1 file changed, 108 insertions(+), 48 deletions(-) diff --git a/docs/proposals/move-selected-nodes-between-spaces.md b/docs/proposals/move-selected-nodes-between-spaces.md index 6fe833e10..d0c834cf2 100644 --- a/docs/proposals/move-selected-nodes-between-spaces.md +++ b/docs/proposals/move-selected-nodes-between-spaces.md @@ -2,43 +2,45 @@ Status: Proposed -Last updated: 2026-08-31 +Last updated: 2026-09-01 Tracking issue: [#142](https://github.com/microsoft/Huabu/issues/142) -> **Scope.** This proposal adds the user-facing business operation for moving selected Canvas nodes and Frame subtrees between ordinary Spaces in the active Workspace. It deliberately does not introduce a general multi-Space transaction API, filesystem WAL, crash recovery, Blob reference counting, garbage collection, or multi-backend transaction protocol. +> **Scope.** This proposal adds the smallest complete user-facing operation for moving selected Canvas nodes and Frame subtrees between existing ordinary Spaces in the active Workspace. It includes moving an eligible Agent Node's existing conversation identity instead of resetting or copying it. It deliberately does not introduce a general multi-Space transaction API, filesystem WAL, crash recovery, Blob reference counting, garbage collection, or multi-backend transaction protocol. > **Reliability boundary.** The operation provides user-visible all-or-compensated behavior while the Server process continues running and returns a determinate result. Process termination, power loss, and an unknown remote-backend outcome remain outside #142, matching the current `SpaceHandle.write()` contract. ## 1. Problem -Huabu can copy a selection through the clipboard and paste it into another Space, but that path is UI-owned reconstruction rather than a move operation. It expands selected Frames, remaps hierarchy and internal edges, and clones cross-Space artifacts, then leaves source deletion to a separate user action. +Huabu can copy a selection through the clipboard and paste it into another Space, but that path is UI-owned reconstruction rather than a move operation. It expands selected Frames, remaps hierarchy and internal edges, and clones cross-Space artifacts, then leaves source deletion to a separate user action. Its conversation-fork request is also currently unavailable because a generic copy does not provide the complete target Agent workload. -That behavior is insufficient for moving work because it has no destination picker, no create-destination flow, no authoritative Server-side selection expansion, no coordinated source deletion, and no durable summary of omitted boundary edges or collision handling. Artifact cloning is intentionally best-effort during paste, so a failed clone may leave a pasted node with a missing-artifact placeholder. +That behavior is insufficient for moving work because it has no explicit Move semantic, destination picker, authoritative Server-side selection expansion, coordinated source deletion, or reliable artifact handling. Artifact cloning is intentionally best-effort during paste, so a failed clone may leave a pasted node with a missing-artifact placeholder. + +Resetting an Agent Node on move would also violate the user's expectation that moving work preserves it. A move has enough authoritative context to retain the existing `threadId`, workload record, driver state, and conversation logs while changing their owning Space. This is narrower than conversation copy: no second independently continuing Agent is created and no target workload identity must be invented. The storage layer now provides the primitives needed to implement the business operation without redesigning storage: ordinary Space lifecycle, complete Space records, `SpaceNodes.readMany()`, Blob reads and writes, version-checked `SpaceHandle.write()`, and in-process rollback for each rejected Space write. It does not provide a transaction spanning two Spaces or structured and Blob stores. ## 2. Decision -Add one Server-owned `SpaceTransferService` and one HTTP execution endpoint. The web gathers the current selection, lets the user choose or create a destination, shows a confirmation derived from the loaded Canvas, drains pending writes, and submits only selected root IDs plus the destination choice. +Add one Server-owned `SpaceMoveService` and one HTTP execution endpoint. The web gathers the current selection, lets the user choose an existing destination, shows a compact confirmation, drains pending writes, and submits selected root IDs plus the destination Space ID. -The Server re-reads authoritative source and destination state, expands Frame subtrees, allocates fresh destination IDs, clones required artifacts, executes the destination insertion and source deletion while holding both Canvas locks, and delays both Canvas Sync publications until the operation succeeds. +The Server re-reads authoritative source and destination state, expands Frame subtrees, allocates fresh destination node and edge IDs, clones required artifacts, rehomes eligible Agent conversations, executes the destination insertion and source deletion while holding both Canvas locks, and delays both Canvas Sync publications until the operation succeeds. -If a determinate failure occurs after the destination write, the service compensates by applying the destination inverse deltas before releasing the locks. A destination created by the operation is deleted as a whole on failure. This is application-level coordination over the current single-Space guarantees, not a new portable storage transaction contract. +If a determinate failure occurs after the destination write, the service compensates the Agent rehome and destination insertion before releasing the locks. This is application-level coordination over the current single-Space and Agenetes store guarantees, not a new portable storage transaction contract. ## 3. Goals - Move one or more selected ordinary Canvas nodes into an existing ordinary Space. -- Create a new ordinary destination Space in the same flow. - Treat selected Frames as subtree roots and preserve every descendant exactly once. - Preserve parent-child hierarchy, parent-local child geometry, root-to-root relative geometry, node style, Frame layout data, and internal edge style. - Omit edges that cross the transfer boundary and report them explicitly. - Clone only artifacts referenced by transferred nodes and rewrite those references for the destination. +- Preserve an eligible Agent Node's `threadId`, complete workload record, driver state, folded turns, and event log while changing the owning Space. - Avoid overwriting destination nodes, edges, sidecars, and artifacts by allocating fresh IDs and de-duplicating labels. - Keep source nodes unchanged when validation or destination preparation fails. - Compensate a completed destination insertion when the following source deletion returns a determinate failure. - Publish no intermediate Canvas Sync state. -- Return an actionable result containing the destination, moved roots and descendants, preserved edges, omitted boundary edges, label changes, and reset runtime state. +- Return an actionable result containing the destination, moved roots and descendants, preserved edges, omitted boundary edges, label changes, and moved Agent conversations. ## 4. Non-goals @@ -48,30 +50,33 @@ If a determinate failure occurs after the destination write, the service compens - A filesystem WAL, two-phase commit protocol, transactional outbox, or idempotency ledger. - Per-key Blob deletion, artifact reference counting, orphan collection, or general Blob GC. - Moving nodes across Workspaces. +- Creating a destination Space as part of the move flow. - Moving the World Canvas or managed World projection/reference nodes. -- Moving Tasks, Runs, Agent histories, pending change-review records, Canvas event history, permissions, or unrelated nearby content. +- Moving Tasks, Runs, pending change-review records, Canvas event history, permissions, or unrelated nearby content. +- Copying or forking Agent conversations. +- Moving a running Agent, a Task/Run root Agent, or an Agent with pending change-review records. - Replacing or changing Huabu clipboard copy/paste. ## 5. User experience Both the single-selection and multi-selection floating toolbars expose **Move to Space** for movable ordinary nodes. The action is unavailable for `spacePreview`, `canvasRef`, `frameRef`, and `nodeRef` because their identity is owned by Space Preview or legacy World reconciliation rather than ordinary node transfer. -The modal reuses `Modal`, `Select`, `TextInput`, and `Button` from `apps/web/src/components/Common`. It contains: +The modal reuses `Modal`, `Select`, and `Button` from `apps/web/src/components/Common`. It contains: - an existing-Space picker that excludes the current Space and World; -- a **Create new Space** option with an optional title; - the number and labels of selected roots; - the number of descendants included through selected Frames; - the number of internal edges that will be preserved; - the number of boundary edges that will be omitted; -- a notice that Question conversations and runtime state do not move; +- the number of eligible Agent conversations that will move; +- a blocking explanation when the selection contains a running Agent, a Task/Run root Agent, or an Agent with pending change-review records; - **Cancel** and **Move** actions. The confirmation summary is derived from the loaded Canvas only to explain the requested action. It is not an authorization or persistence plan. The Server repeats every structural check against authoritative state before mutating anything. Before submitting, the web drains pending node-content writes and the structure-save queue. The Move action remains disabled while that drain or the request is in progress. -On success, the source selection disappears through its normal Canvas Sync update. A persistent success toast reports the transferred node and edge counts and offers **Open destination**. On failure, a persistent localized error explains whether the selection became stale, the destination disappeared, validation failed, or the outcome is unknown. +On success, the source selection disappears through its normal Canvas Sync update. A success toast reports the moved node and conversation counts and offers **Open destination**. On failure, a persistent localized error explains whether the selection became stale, the destination disappeared, an Agent was ineligible, validation failed, compensation failed, or the outcome is unknown. ## 6. Selection and subtree semantics @@ -100,7 +105,7 @@ Only root positions receive the translation. Descendant positions remain parent- ## 8. Identity and collision handling -Every transferred node receives a fresh `node-` ID and every preserved internal edge receives a fresh `edge-` ID. The response includes the old-to-new node mapping for diagnostics and destination navigation. +Every transferred node receives a fresh `node-` ID and every preserved internal edge receives a fresh `edge-` ID. An Agent Node keeps its globally unique `threadId`; node identity and conversation identity are separate domains. The response includes the old-to-new node mapping for diagnostics and destination navigation. Fresh IDs avoid overwriting destination topology or sidecars and prevent a previous copy of the same source node from colliding with the move. Sketch stroke IDs are also regenerated because strokes form a node-local editing identity domain and may later be merged with another sketch. @@ -126,26 +131,45 @@ The current Blob port has no per-key delete. A failed transfer into an existing Source artifacts are not deleted after success because other source nodes may still reference the same key and the current storage model has no reference counts. -## 11. Question and runtime-owned state +## 11. Agent conversation ownership -A Question node's authored prompt, label, geometry, and visual style move as ordinary node data. Canvas-local runtime fields are removed in the destination: `threadId`, `status`, `runAt`, `errorMessage`, `responseSummary`, and `viewed`. +A Question node without a `threadId` moves as ordinary authored node data. A Question or fixed Agent Node with a `threadId` keeps that thread identity and moves its complete conversation ownership to the destination Space. -The moved Question therefore arrives as a fresh idle Question with no conversation history. Agent history, ACP session mappings, Tasks, Runs, and pending change-review records remain in the source Space and are never traversed by the transfer service. +The move service rejects the complete request before mutation when any transferred Agent: -This policy avoids a destination node pointing at a thread namespace owned by another Space and keeps #142 independent of Agenetes history migration. +- has an active turn or `status === 'running'`; +- is referenced by a Task/Run as `rootNodeId` or `rootThreadId`; +- has pending change-review records; +- has a missing, malformed, or conflicting workload/history record. -## 12. HTTP contract +Pending change-review records do not move because their commands and inverse changes belong to the source Canvas. Tasks and Runs do not move because their ledger and lifecycle form a larger execution aggregate than a Canvas selection. + +Huabu supplies Agenetes with the complete target workload spec. It preserves the source driver kind, workload type, `threadId`, and driver state while replacing the namespace and host-owned Canvas context with the destination. Agenetes does not merge or interpret host fields. -Add the shared zod contract under `packages/shared/src/types/api/space-transfer.ts` and validate it at the route boundary according to [API Design](../architecture/api-design.md). +Add a destructive counterpart to the existing `Agenetes.fork(source, targetSpec)` boundary: ```ts -type MoveSelectionDestination = - | { kind: 'existing'; canvasId: string } - | { kind: 'new'; title?: string }; +interface Agenetes { + rehome(source: ThreadIdentity, targetSpec: WorkloadSpec): void; +} +``` + +`rehome()` requires the source to have no live handle and the target namespace to contain no record or log for the thread. It snapshots and validates the source record plus both logs, writes the target logs, writes the target record last as the destination visibility point, then removes the source record before removing the source logs. It preserves driver state and removes no source data until the complete target is durable. + +The underlying `ThreadStore`, `EventLogStore`, and `TurnStore` receive narrow replace/delete capabilities implemented by both memory and file stores. If any target write or source removal fails, `rehome()` restores the complete source snapshot and removes every target record/log it wrote before returning the original error. A failed rollback becomes an explicit unknown-outcome error. Huabu never reads, renames, or deletes Agenetes-private files directly. This change belongs to the `external/agenetes/` subtree and must be committed separately so it can be pushed upstream. + +Before calling `rehome()`, Huabu closes the dormant runtime handle and verifies the Agent turn lease is free. After rehome, the next invocation recovers from the destination namespace with the preserved driver state and history. + +The existing generic conversation-copy endpoint remains unavailable. Move can build a complete target spec from the node being moved; clipboard copy still cannot safely infer an independent target Agent identity. + +## 12. HTTP contract + +Add the shared zod contract under `packages/shared/src/types/api/space-move.ts` and validate it at the route boundary according to [API Design](../architecture/api-design.md). +```ts interface MoveSelectionRequest { selectedNodeIds: string[]; - destination: MoveSelectionDestination; + destinationCanvasId: string; expectedSourceVersion: number; } ``` @@ -161,7 +185,7 @@ The response contains: ```ts interface MoveSelectionResponse { transferId: string; - destination: { canvasId: string; title: string | null; created: boolean }; + destination: { canvasId: string; title: string | null }; sourceVersion: number; destinationVersion: number; roots: Array<{ @@ -178,11 +202,11 @@ interface MoveSelectionResponse { target: string; }>; renamedNodes: Array<{ sourceNodeId: string; from: string; to: string }>; - resetQuestionCount: number; + movedConversationCount: number; } ``` -The route uses typed error codes for stale source version, missing source node, invalid node type, missing or same destination, World refusal, missing artifact, destination conflict, compensation failure, and unknown outcome. User-facing text is localized in the web application. +The route uses typed error codes for stale source version, missing source node, invalid node type, missing or same destination, World refusal, running Agent, Task/Run-owned Agent, pending Agent changes, invalid Agent history, missing artifact, destination conflict, compensation failure, and unknown outcome. User-facing text is localized in the web application. A separate planning endpoint is intentionally omitted. It would duplicate most reads and validation for a confirmation that the Server must repeat during execution. The loaded Canvas provides the preview; the execution endpoint remains authoritative. @@ -193,14 +217,15 @@ The service follows this order: ```text drain client writes -> acquire Workspace operation lease - -> resolve/create destination -> acquire source and destination Canvas locks in sorted-id order -> read and validate both current Space records -> expand selection and build transfer model - -> read required node records and artifacts + -> validate Agent eligibility and acquire thread leases + -> read required node records, Agent records, and artifacts -> allocate IDs, labels, placement, and rewritten references -> write fresh destination artifacts -> execute destination CREATE_NODES + CONNECT_NODES without publication + -> rehome eligible Agent conversations to the destination namespace -> execute source DELETE_NODES without publication -> publish destination and source updates -> return result @@ -208,9 +233,11 @@ drain client writes The existing `withCanvasMutex()` becomes a small multi-key coordinator that acquires unique Canvas IDs in lexical order. Existing single-Canvas callers continue through the same one-key path. -The Canvas executor exposes an internal already-locked execution function plus its current public lock-taking wrapper. Transfer calls the already-locked form so both locks remain held across destination insertion, source deletion, and any compensation. Normal executor callers remain unchanged. +The Canvas executor exposes internal already-locked variants for both command execution and inverse-delta application, plus the current public lock-taking wrappers. Move calls only the already-locked variants so both locks remain held across destination insertion, source deletion, and any compensation; calling the public `applyDeltasOnServer()` while holding the destination lock would otherwise wait on itself. Normal executor callers remain unchanged. + +The destination and source operations reuse `CREATE_NODES`, `CONNECT_NODES`, and `DELETE_NODES`; move-specific selection expansion, ID mapping, artifact rewriting, Agent eligibility, and result reporting stay in `SpaceMoveService` rather than becoming a `CanvasCommand`. -The destination and source operations reuse `CREATE_NODES`, `CONNECT_NODES`, and `DELETE_NODES`; transfer-specific selection expansion, ID mapping, artifact rewriting, and result reporting stay in `SpaceTransferService` rather than becoming a `CanvasCommand`. +Every acquired Agent turn lease is released in a `finally` path after success, compensation, or unknown-outcome reporting. Canvas locks remain held until Agent leases are released and the final publication or error outcome is fixed. ## 14. Determinate failure and compensation @@ -218,9 +245,11 @@ Validation, source reads, artifact reads, and destination artifact writes occur If destination command execution rejects, its existing `SpaceHandle.write()` rollback restores that Space's structured prestate. The source has not yet changed. -If source deletion rejects after destination insertion committed, the service applies the destination execution's inverse deltas while both Canvas locks remain held. It publishes neither the insertion nor the compensation. If the destination was created by this request, the service deletes that new Space instead of applying inverse deltas. +If an Agent rehome rejects, the service applies the destination execution's inverse deltas while both Canvas locks remain held. The source nodes and Agent ownership remain unchanged. + +If source deletion rejects after Agent rehome, the service rehomes moved conversations back to their source specs, then applies the destination execution's inverse deltas. It publishes neither the insertion nor the compensation. -If compensation succeeds, the endpoint returns the original source-deletion failure and both Spaces remain user-visible equivalents of their pre-request states, apart from possible unreachable destination Blob bytes. +If compensation succeeds, the endpoint returns the original failure and both Spaces remain user-visible equivalents of their pre-request states, apart from possible unreachable destination Blob bytes. If compensation itself fails or the backend outcome becomes unknown, the endpoint returns a distinct persistent-error code and instructs the client to reload both Spaces before retrying. The service does not claim success and the client must not automatically retry. @@ -240,14 +269,35 @@ World, the source Space itself, a missing Space, and a Space in deletion admissi ## 17. Implementation plan +### Slice A: Agenetes rehome primitive + +1. Add replace/delete capabilities to `ThreadStore`, `EventLogStore`, and `TurnStore`, with in-memory and file-backed contract coverage. +2. Add `Agenetes.rehome(source, targetSpec)`, preserving driver state and logs, rejecting live or conflicting targets, and compensating determinate store failures. +3. Cover internal and external workload records, complete history, empty history, destination collision, malformed source, and rollback. +4. Commit the `external/agenetes/` subtree change independently. + +### Slice B: Server move operation + 1. Add shared request, response, and typed-error contracts plus route builders and the web API helper. -2. Extract reusable selection expansion and transfer-model construction from the current clipboard behavior without changing clipboard semantics. +2. Extract pure selection expansion and transfer-model construction from current clipboard behavior without changing clipboard semantics. 3. Add sorted multi-Canvas lock acquisition and an already-locked Canvas executor entry while preserving all current callers. -4. Implement `SpaceTransferService` using current Space, node, Blob, executor, and lifecycle APIs. -5. Add the move-selection route and delayed paired Canvas Sync publication. -6. Add the modal and toolbar actions with localized English and Chinese strings. -7. Add focused service, route, resolver, and UI regression tests. -8. Fold shipped behavior into `docs/architecture/canvas-storage.md`, `docs/architecture/canvas-command-architecture.md`, and `docs/architecture/web-architecture.md`. +4. Implement `SpaceMoveService` using current Space, node, Blob, Task ledger, Agent service, executor, and lifecycle APIs. +5. Add Agent eligibility checks and build complete destination workload specs for `Agenetes.rehome()`. +6. Add the move-selection route and delayed paired Canvas Sync publication. + +### Slice C: Product UI + +1. Add the existing-Space picker modal and single/multi-selection toolbar actions with localized English and Chinese strings. +2. Drain pending source writes before submission, disable the action during execution, and surface eligibility errors. +3. Add success feedback with moved counts and an **Open destination** action. +4. Add focused service, route, resolver, and UI regression tests. +5. Fold shipped behavior into `docs/architecture/canvas-storage.md`, `docs/architecture/agent-architecture.md`, `docs/architecture/canvas-command-architecture.md`, and `docs/architecture/web-architecture.md`. + +### Execution order and effort + +Slice A is the only `external/agenetes/` change and lands as its own upstreamable commit. Slice B depends on Slice A. Slice C can begin after the shared API contract in Slice B is stable, but the feature remains hidden until the server path and end-to-end Agent continuation tests pass. + +The expected implementation size is approximately 700–1,000 production lines and 500–800 test lines. Slice A is expected to take 1–2 engineering days, Slice B 3–4 days, and Slice C 1–2 days, excluding review latency. The estimate intentionally excludes generic transaction infrastructure, new-Space creation, conversation copy, and Task/Run migration. ## 18. Test plan @@ -284,14 +334,20 @@ World, the source Space itself, a missing Space, and a Space in deletion admissi ### Runtime data boundaries -- Reset Question conversation and run fields while preserving authored content and style. -- Leave Tasks, Runs, chat history, ACP session mappings, change-review records, and Canvas events in the source. +- Move an idle/done/error Agent while preserving `threadId`, workload spec, driver state, folded turns, event log, authored content, and style. +- Rewrite the target namespace and host Canvas context without changing Agent binding or driver kind. +- Continue the moved conversation from the destination and verify subsequent Canvas tool writes target the destination. +- Reject a running Agent without mutation. +- Reject an Agent referenced by Task/Run root identity without mutation. +- Reject an Agent with pending change-review records without mutation. +- Leave Tasks, Runs, change-review records, and Canvas events in the source. ### Failure behavior - Destination write failure leaves source and destination topology unchanged. - Source deletion failure applies destination inverse deltas and publishes no intermediate update. -- New-destination failure removes the newly created Space. +- Agent rehome failure removes destination topology and leaves source ownership unchanged. +- Source deletion failure rehomes Agent conversations back before removing destination topology. - Compensation failure returns the distinct unknown-state error and never auto-retries. - Concurrent writes serialize under sorted dual locks. - A stale `expectedSourceVersion` rejects before mutation. @@ -299,8 +355,9 @@ World, the source Space itself, a missing Space, and a Space in deletion admissi ### UI - Single and multi-selection toolbars open the same modal. -- Existing Space and create-new flows submit the correct destination variant. -- The confirmation shows normalized roots, descendants, preserved edges, omitted edges, and Question reset notice. +- The existing-Space flow submits the selected destination ID. +- The confirmation shows normalized roots, descendants, preserved edges, omitted edges, and moved Agent conversation count. +- Ineligible Agent selections disable confirmation and explain the blocking reason. - Pending writes drain before submission. - Success shows counts and an Open destination action. - Failure keeps the current source view and shows localized persistent feedback. @@ -310,8 +367,9 @@ World, the source Space itself, a missing Space, and a Space in deletion admissi Run focused checks first: ```bash -pnpm --filter @huabu/shared test -- src/types/api/space-transfer.test.ts -pnpm --filter @huabu/server test -- src/modules/canvas/space-transfer.service.test.ts src/modules/canvas/space-transfer.route.test.ts +pnpm --filter @huabu/shared test -- src/types/api/space-move.test.ts +pnpm --filter @agenetes/agenetes test -- src/instance.rehome.test.ts src/thread-store.test.ts src/event-log.test.ts src/turn-store.test.ts +pnpm --filter @huabu/server test -- src/modules/canvas/space-move.service.test.ts src/modules/canvas/space-move.route.test.ts pnpm --filter @huabu/web test -- src/components/Panels/Canvas/MoveSelectionModal.test.tsx pnpm --filter @huabu/shared typecheck pnpm --filter @huabu/server typecheck @@ -330,7 +388,7 @@ pnpm lint:fix A separate storage design should decide whether and how Huabu provides a backend-neutral transaction spanning two Spaces and Blob scopes. That work owns crash recovery, WAL or native SQL transaction mapping, idempotency, transactional publication, per-key Blob deletion, staging, reference counts, retention, and orphan GC. -That follow-up may later replace the compensation implementation behind `SpaceTransferService`. It must not expand #142 or delay the user-facing business flow defined here. +That follow-up may later replace the compensation implementation behind `SpaceMoveService`. It must not expand #142 or delay the user-facing business flow defined here. ## Code entry points @@ -341,6 +399,8 @@ That follow-up may later replace the compensation implementation behind `SpaceTr | [`apps/server/src/modules/canvas/write-coordinator.ts`](../../apps/server/src/modules/canvas/write-coordinator.ts) | Existing per-Canvas mutex to extend with sorted multi-key acquisition. | | [`apps/server/src/modules/storage/ports/structured.ts`](../../apps/server/src/modules/storage/ports/structured.ts) | Existing Space records, node reads, versioned writes, and reported-failure rollback boundary. | | [`apps/server/src/modules/storage/ports/blob.ts`](../../apps/server/src/modules/storage/ports/blob.ts) | Existing scoped artifact reads and writes; per-key cleanup remains deferred. | +| [`apps/server/src/modules/agent/agent-thread.service.ts`](../../apps/server/src/modules/agent/agent-thread.service.ts) | Agent eligibility, complete destination workload construction, and turn-lease coordination. | +| [`external/agenetes/packages/agenetes/src/`](../../external/agenetes/packages/agenetes/src/) | Driver-agnostic destructive thread rehome and durable store move primitives. | | [`apps/web/src/store/canvasStore.ts`](../../apps/web/src/store/canvasStore.ts) | Current selection, clipboard subtree expansion, save queues, and Canvas Sync application. | | [`apps/web/src/components/Panels/Canvas/FloatingToolbars/`](../../apps/web/src/components/Panels/Canvas/FloatingToolbars/) | Single- and multi-selection action entry points. | | [`apps/web/src/components/Common/`](../../apps/web/src/components/Common/) | Existing modal, picker, input, and button primitives reused by the flow. | From 41e15edf13e6c5c8ce70f79aac1d4e42c5853a6d Mon Sep 17 00:00:00 2001 From: Yuqing Yang Date: Wed, 2 Sep 2026 08:37:11 +0000 Subject: [PATCH 03/12] feat(agenetes): add durable thread rehome Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../packages/agenetes/src/event-log.test.ts | 69 ++- .../packages/agenetes/src/event-log.ts | 90 +++- .../agenetes/src/instance.rehome.test.ts | 496 ++++++++++++++++++ .../packages/agenetes/src/instance.ts | 188 +++++++ external/agenetes/packages/agenetes/src/io.ts | 30 ++ .../packages/agenetes/src/turn-store.test.ts | 68 ++- .../packages/agenetes/src/turn-store.ts | 61 ++- .../agenetes/packages/runtime/src/errors.ts | 4 +- 8 files changed, 999 insertions(+), 7 deletions(-) create mode 100644 external/agenetes/packages/agenetes/src/instance.rehome.test.ts diff --git a/external/agenetes/packages/agenetes/src/event-log.test.ts b/external/agenetes/packages/agenetes/src/event-log.test.ts index 47e3e32a9..9df529f2e 100644 --- a/external/agenetes/packages/agenetes/src/event-log.test.ts +++ b/external/agenetes/packages/agenetes/src/event-log.test.ts @@ -8,7 +8,7 @@ // and the EventLog live pub/sub (append fans out to subscribers; read is // the caller's backfill). -import { appendFileSync, mkdtempSync, rmSync } from 'node:fs'; +import { appendFileSync, existsSync, mkdtempSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import path from 'node:path'; @@ -125,6 +125,43 @@ describe.each<[string, () => EventLogStore]>([ expect(store.maxSeq(n, 'nope')).toBe(0); expect(store.read(n, 'nope')).toEqual([]); }); + + it('replace() overwrites the whole log and reseeds maxSeq (the rehome() move primitive)', () => { + const store = make(); + const source = ns('canvas-1'); + const target = ns('canvas-2'); + store.append(source, 'thread-1', text('a')); + store.append(source, 'thread-1', text('b')); + const snapshot = store.readRecords(source, 'thread-1'); + + store.replace(target, 'thread-1', snapshot); + expect(store.readRecords(target, 'thread-1')).toEqual(snapshot); + expect(store.maxSeq(target, 'thread-1')).toBe(2); + // The next append after a replace continues from the replaced content, + // not from a stale cached counter. + const next = store.append(target, 'thread-1', text('c')); + expect(next.seq).toBe(3); + + // A second replace() overwrites in full rather than appending. + store.replace(target, 'thread-1', [snapshot[0]!]); + expect(store.readRecords(target, 'thread-1')).toEqual([snapshot[0]]); + expect(store.maxSeq(target, 'thread-1')).toBe(1); + }); + + it('delete() removes a log entirely and idempotently', () => { + const store = make(); + const n = ns('canvas-1'); + store.append(n, 'thread-1', text('a')); + store.delete(n, 'thread-1'); + expect(store.readRecords(n, 'thread-1')).toEqual([]); + expect(store.maxSeq(n, 'thread-1')).toBe(0); + // Deleting an already-missing log is a no-op, not an error. + expect(() => store.delete(n, 'thread-1')).not.toThrow(); + // A fresh append after delete restarts sequencing from 1, not from a + // stale cached counter for the deleted path. + const first = store.append(n, 'thread-1', text('fresh')); + expect(first.seq).toBe(1); + }); }); describe('FileEventLogStore — on-disk specifics', () => { @@ -154,6 +191,36 @@ describe('FileEventLogStore — on-disk specifics', () => { const entries = fresh.read(n, 'thread-1'); expect(entries.map((e) => e.seq)).toEqual([1]); }); + + it('replace() writes the target file and a fresh store instance reads it back', () => { + const source = ns('canvas-1'); + const target = ns('canvas-2'); + const writer = new FileEventLogStore(); + writer.append(source, 'thread-1', text('a')); + writer.append(source, 'thread-1', text('b')); + const snapshot = writer.readRecords(source, 'thread-1'); + + writer.replace(target, 'thread-1', snapshot); + expect(existsSync(eventFilePath(target, 'thread-1'))).toBe(true); + + const restarted = new FileEventLogStore(); + expect(restarted.readRecords(target, 'thread-1')).toEqual(snapshot); + expect(restarted.append(target, 'thread-1', text('c')).seq).toBe(3); + }); + + it('delete() removes the on-disk file so a fresh store observes an empty log', () => { + const n = ns('canvas-1'); + const writer = new FileEventLogStore(); + writer.append(n, 'thread-1', text('a')); + expect(existsSync(eventFilePath(n, 'thread-1'))).toBe(true); + + writer.delete(n, 'thread-1'); + expect(existsSync(eventFilePath(n, 'thread-1'))).toBe(false); + + const restarted = new FileEventLogStore(); + expect(restarted.readRecords(n, 'thread-1')).toEqual([]); + expect(restarted.append(n, 'thread-1', text('fresh')).seq).toBe(1); + }); }); describe('EventLog — durable append + live pub/sub', () => { diff --git a/external/agenetes/packages/agenetes/src/event-log.ts b/external/agenetes/packages/agenetes/src/event-log.ts index 4d0756922..b47a6a424 100644 --- a/external/agenetes/packages/agenetes/src/event-log.ts +++ b/external/agenetes/packages/agenetes/src/event-log.ts @@ -24,7 +24,13 @@ // handles the live fan-out. All of this is L2-internal — the sequence // numbers, the pub/sub, and the file layout never leak to L1 (I9.8). -import { appendJsonLine, readJsonLines, sanitizeId } from './io.js'; +import { + appendJsonLine, + readJsonLines, + removeFileIfExists, + sanitizeId, + writeJsonLines, +} from './io.js'; import type { AgentSubmission, @@ -108,6 +114,24 @@ export interface EventLogStore { ): EventLogRecord[]; /** The highest `seq` persisted for a thread, or `0` when the log is empty. */ maxSeq(namespace: Namespace, threadId: string): number; + /** + * Overwrite a thread's ENTIRE Tier-1 log with `records` (already-sequenced, + * in original order), replacing whatever the target held before. A narrow + * capability reserved for the `rehome()` durable-move primitive — it + * writes the destination log wholesale from a source snapshot, never for + * incremental/streaming writes (those stay on `append`/`appendTurnStart`). + */ + replace( + namespace: Namespace, + threadId: string, + records: readonly EventLogRecord[], + ): void; + /** + * Remove a thread's Tier-1 log entirely (idempotent). Reserved for the + * `rehome()` primitive: dropping the source log after its target twin is + * durable, or compensating a target log written during a failed rehome. + */ + delete(namespace: Namespace, threadId: string): void; } /** Defensive shape-check for a persisted entry read back from disk. */ @@ -149,12 +173,17 @@ function isRecord(value: unknown): value is EventLogRecord { export class InMemoryEventLogStore implements EventLogStore { readonly #byNamespace = new Map>(); - #log(namespace: Namespace, threadId: string): EventLogRecord[] { + #scope(namespace: Namespace): Map { let scope = this.#byNamespace.get(namespace.name); if (!scope) { scope = new Map(); this.#byNamespace.set(namespace.name, scope); } + return scope; + } + + #log(namespace: Namespace, threadId: string): EventLogRecord[] { + const scope = this.#scope(namespace); let log = scope.get(threadId); if (!log) { log = []; @@ -209,6 +238,18 @@ export class InMemoryEventLogStore implements EventLogStore { const log = this.#byNamespace.get(namespace.name)?.get(threadId); return log && log.length > 0 ? log[log.length - 1]!.seq : 0; } + + replace( + namespace: Namespace, + threadId: string, + records: readonly EventLogRecord[], + ): void { + this.#scope(namespace).set(threadId, [...records]); + } + + delete(namespace: Namespace, threadId: string): void { + this.#byNamespace.get(namespace.name)?.delete(threadId); + } } /** @@ -304,6 +345,29 @@ export class FileEventLogStore implements EventLogStore { maxSeq(namespace: Namespace, threadId: string): number { return this.#lastSeq(this.#path(namespace, threadId)); } + + replace( + namespace: Namespace, + threadId: string, + records: readonly EventLogRecord[], + ): void { + const filePath = this.#path(namespace, threadId); + writeJsonLines(filePath, records); + // Reseed the cached last-seq from the just-written content — writing + // bypasses `append`'s incremental counter, so a stale cache would hand + // out a colliding `seq` on the very next append. + const max = records.reduce((acc, entry) => Math.max(acc, entry.seq), 0); + this.#seqByPath.set(filePath, max); + } + + delete(namespace: Namespace, threadId: string): void { + const filePath = this.#path(namespace, threadId); + removeFileIfExists(filePath); + // Drop the cached counter so a future write to this path (e.g. a new + // thread reusing the same id after this log was rehomed away) reseeds + // from disk instead of resuming the stale in-process count. + this.#seqByPath.delete(filePath); + } } /** A live-tail subscriber: invoked for every entry appended after it subscribes. */ @@ -379,6 +443,28 @@ export class EventLog { return this.#store.maxSeq(namespace, threadId); } + /** + * Overwrite a thread's whole Tier-1 log durably; see + * {@link EventLogStore.replace}. Reserved for `rehome()`'s destination + * write — it carries no live fan-out because a rehome target never has + * subscribers yet (its thread does not exist before the move). + */ + replace( + namespace: Namespace, + threadId: string, + records: readonly EventLogRecord[], + ): void { + this.#store.replace(namespace, threadId, records); + } + + /** + * Remove a thread's Tier-1 log entirely; see {@link EventLogStore.delete}. + * Reserved for `rehome()`'s source cleanup / target compensation. + */ + delete(namespace: Namespace, threadId: string): void { + this.#store.delete(namespace, threadId); + } + /** * Subscribe to entries appended AFTER this call for `threadId`. Returns an * idempotent unsubscribe. Backfill (entries already persisted) is the diff --git a/external/agenetes/packages/agenetes/src/instance.rehome.test.ts b/external/agenetes/packages/agenetes/src/instance.rehome.test.ts new file mode 100644 index 000000000..7c6180208 --- /dev/null +++ b/external/agenetes/packages/agenetes/src/instance.rehome.test.ts @@ -0,0 +1,496 @@ +// M5.7 acceptance — `Agenetes.rehome()`, the destructive counterpart to +// `fork()` (docs/proposals/move-selected-nodes-between-spaces.md §11). +// +// Exercises the durable thread-relocation primitive end-to-end with a stub +// driver (no ACP / no host): a successful move preserving threadId, driver +// kind, workload type, driver state, Tier-1 events, and Tier-2 turns while +// rewriting the namespace/spec; the source-live-handle and target-collision +// preconditions; and determinate-failure rollback plus the explicit +// unknown-outcome path when the rollback itself cannot fully restore the +// source. + +import { AgenetesError, defineDriver } from '@agenetes/runtime'; +import { describe, expect, it } from 'vitest'; + +import { InMemoryEventLogStore, type EventLogStore } from './event-log.js'; +import { InMemoryThreadStore, type ThreadStore } from './thread-store.js'; +import { + InMemoryTurnStore, + type PersistedTurn, + type TurnStore, +} from './turn-store.js'; + +import { mountAgenetes } from './index.js'; + +import type { + AgentSpec, + AgentStateSnapshot, + Namespace, +} from '@agenetes/protocol'; +import type { + AgentCreateContext, + AgentHandle, + TypedWorkloadSpec, +} from '@agenetes/runtime'; + +interface StubDriverSpec extends AgentSpec { + readonly note?: string; +} +type StubSpec = TypedWorkloadSpec; +interface StubDriverState { + readonly sessionId?: string; +} + +/** A stub handle recording its close() so live-handle checks are observable. */ +class StubHandle { + closed = false; + constructor( + readonly spec: StubSpec, + readonly createContext: AgentCreateContext, + ) {} + close(): void { + this.closed = true; + } +} + +const stubSpecSchema = { + safeParse(input: unknown) { + return input !== null && typeof input === 'object' + ? { success: true as const, data: input as StubDriverSpec } + : { success: false as const, error: new Error('expected object') }; + }, +}; + +const stubStateSchema = { + safeParse(input: unknown) { + return input !== null && typeof input === 'object' + ? { success: true as const, data: input as StubDriverState } + : { success: false as const, error: new Error('expected object') }; + }, +}; + +function stubDriver() { + return defineDriver({ + schemaVersion: 1, + workloadTypes: ['Job', 'Deployment'], + specSchema: stubSpecSchema, + stateSchema: stubStateSchema, + initialState: () => ({}), + create: (spec, context) => + new StubHandle(spec, context) as unknown as AgentHandle, + }); +} + +const ns = (name: string, root?: string): Namespace => ({ + name, + storage: root ? { root } : undefined, +}); + +/** Seed a durable source thread (record + Tier-1 events + Tier-2 turns). */ +function seedSource( + threadStore: ThreadStore, + eventLogStore: EventLogStore, + turnStore: TurnStore, + sourceNamespace: Namespace, + threadId: string, +): { record: ReturnType; sourceNamespace: Namespace } { + const spec: StubSpec = { + threadId, + kind: 'external', + workloadType: 'Deployment', + namespace: sourceNamespace, + spec: { note: 'source context' }, + }; + const state: AgentStateSnapshot = { + driverState: { sessionId: 'source_session' }, + }; + threadStore.upsert(sourceNamespace, threadId, { + driverSchemaVersion: 1, + spec, + state, + }); + eventLogStore.appendTurnStart(sourceNamespace, threadId, { + type: 'user_text', + content: 'hello', + }); + eventLogStore.append(sourceNamespace, threadId, { + type: 'text_delta', + data: { content: 'hi there' }, + }); + eventLogStore.append(sourceNamespace, threadId, { type: 'end', data: {} }); + turnStore.append(sourceNamespace, threadId, { + turn: { + request: { type: 'user_text', content: 'hello' }, + transcript: [{ type: 'text', data: { content: 'hi there' } }], + }, + seqStart: 1, + seqEnd: 3, + }); + return { + record: threadStore.get(sourceNamespace, threadId), + sourceNamespace, + }; +} + +const targetSpecFor = ( + sourceSpec: StubSpec, + targetNamespace: Namespace, +): StubSpec => ({ + ...sourceSpec, + namespace: targetNamespace, + spec: { note: 'destination context' }, +}); + +describe('Agenetes.rehome() — the destructive counterpart to fork()', () => { + it('relocates the thread record, Tier-1 events, and Tier-2 turns to the target namespace', () => { + const threadStore = new InMemoryThreadStore(); + const eventLogStore = new InMemoryEventLogStore(); + const turnStore = new InMemoryTurnStore(); + const sourceNamespace = ns('canvas_1', '/data/c1'); + const targetNamespace = ns('canvas_2', '/data/c2'); + const threadId = 'thread_1'; + seedSource( + threadStore, + eventLogStore, + turnStore, + sourceNamespace, + threadId, + ); + const sourceRecord = threadStore.get(sourceNamespace, threadId)!; + const sourceEvents = eventLogStore.readRecords(sourceNamespace, threadId); + const sourceTurns = turnStore.list(sourceNamespace, threadId); + + const inst = mountAgenetes({ + drivers: { external: stubDriver() }, + threadStore, + eventLogStore, + turnStore, + }); + + const targetSpec = targetSpecFor( + sourceRecord.spec as StubSpec, + targetNamespace, + ); + inst.rehome({ namespace: sourceNamespace, threadId }, targetSpec); + + // Target: the visible durable owner with the rewritten namespace/spec, + // preserved threadId, driver kind, workload type, and driver state. + expect(inst.record(targetNamespace, threadId)).toEqual({ + driverSchemaVersion: 1, + spec: targetSpec, + state: sourceRecord.state, + }); + expect(eventLogStore.readRecords(targetNamespace, threadId)).toEqual( + sourceEvents, + ); + expect(turnStore.list(targetNamespace, threadId)).toEqual(sourceTurns); + expect(inst.history(targetNamespace, threadId).turns).toEqual( + sourceTurns.map((p) => p.turn), + ); + + // Source: fully removed — record, Tier-1 log, and Tier-2 log. + expect(inst.record(sourceNamespace, threadId)).toBeUndefined(); + expect(eventLogStore.readRecords(sourceNamespace, threadId)).toEqual([]); + expect(turnStore.list(sourceNamespace, threadId)).toEqual([]); + + // rehome() never spawns/enters the live-handle table. + expect(inst.get(threadId)).toBeUndefined(); + }); + + it('rejects a source thread with a live handle, leaving source and target untouched', () => { + const threadStore = new InMemoryThreadStore(); + const eventLogStore = new InMemoryEventLogStore(); + const turnStore = new InMemoryTurnStore(); + const sourceNamespace = ns('canvas_1'); + const targetNamespace = ns('canvas_2'); + const threadId = 'thread_1'; + const inst = mountAgenetes({ + drivers: { external: stubDriver() }, + threadStore, + eventLogStore, + turnStore, + }); + const sourceSpec: StubSpec = { + threadId, + kind: 'external', + workloadType: 'Deployment', + namespace: sourceNamespace, + spec: {}, + }; + // create() spawns a live Deployment handle and upserts the record. + inst.create(sourceSpec); + + const targetSpec = targetSpecFor(sourceSpec, targetNamespace); + expect(() => + inst.rehome({ namespace: sourceNamespace, threadId }, targetSpec), + ).toThrow(/live handle/); + expect(inst.record(targetNamespace, threadId)).toBeUndefined(); + expect(inst.record(sourceNamespace, threadId)).toBeDefined(); + expect(inst.get(threadId)).toBeDefined(); + }); + + it('rejects a missing source thread', () => { + const inst = mountAgenetes({ drivers: { external: stubDriver() } }); + const namespace = ns('canvas_1'); + const targetSpec: StubSpec = { + threadId: 'missing', + kind: 'external', + workloadType: 'Deployment', + namespace: ns('canvas_2'), + spec: {}, + }; + expect(() => + inst.rehome({ namespace, threadId: 'missing' }, targetSpec), + ).toThrow(/missing source thread/); + }); + + it.each([ + ['a conflicting thread record', 'record'], + ['conflicting Tier-2 turns', 'turns'], + ['conflicting Tier-1 events', 'events'], + ] as const)( + 'rejects a target namespace with %s, leaving source untouched', + (_label, conflictKind) => { + const threadStore = new InMemoryThreadStore(); + const eventLogStore = new InMemoryEventLogStore(); + const turnStore = new InMemoryTurnStore(); + const sourceNamespace = ns('canvas_1'); + const targetNamespace = ns('canvas_2'); + const threadId = 'thread_1'; + seedSource( + threadStore, + eventLogStore, + turnStore, + sourceNamespace, + threadId, + ); + const sourceRecord = threadStore.get(sourceNamespace, threadId)!; + + if (conflictKind === 'record') { + threadStore.upsert(targetNamespace, threadId, sourceRecord); + } else if (conflictKind === 'turns') { + turnStore.append(targetNamespace, threadId, { + turn: { + request: { type: 'user_text', content: 'stale' }, + transcript: [], + }, + seqStart: 1, + seqEnd: 1, + }); + } else { + eventLogStore.append(targetNamespace, threadId, { + type: 'end', + data: {}, + }); + } + + const inst = mountAgenetes({ + drivers: { external: stubDriver() }, + threadStore, + eventLogStore, + turnStore, + }); + const targetSpec = targetSpecFor( + sourceRecord.spec as StubSpec, + targetNamespace, + ); + expect(() => + inst.rehome({ namespace: sourceNamespace, threadId }, targetSpec), + ).toThrow(/already exists/); + // The source is completely untouched by a rejected precondition. + expect(inst.record(sourceNamespace, threadId)).toEqual(sourceRecord); + expect(eventLogStore.readRecords(sourceNamespace, threadId).length).toBe( + 3, + ); + expect(turnStore.list(sourceNamespace, threadId).length).toBe(1); + }, + ); + + it('rejects a target threadId, driver kind, or workload type that differs from the source', () => { + const threadStore = new InMemoryThreadStore(); + const eventLogStore = new InMemoryEventLogStore(); + const turnStore = new InMemoryTurnStore(); + const sourceNamespace = ns('canvas_1'); + const targetNamespace = ns('canvas_2'); + const threadId = 'thread_1'; + seedSource( + threadStore, + eventLogStore, + turnStore, + sourceNamespace, + threadId, + ); + const sourceRecord = threadStore.get(sourceNamespace, threadId)!; + const inst = mountAgenetes({ + drivers: { external: stubDriver(), internal: stubDriver() }, + threadStore, + eventLogStore, + turnStore, + }); + const base = targetSpecFor(sourceRecord.spec as StubSpec, targetNamespace); + + expect(() => + inst.rehome( + { namespace: sourceNamespace, threadId }, + { ...base, threadId: 'renamed' }, + ), + ).toThrow(/threadId must equal source/); + expect(() => + inst.rehome( + { namespace: sourceNamespace, threadId }, + { ...base, kind: 'internal' }, + ), + ).toThrow(/driver kind must match source/); + expect(() => + inst.rehome( + { namespace: sourceNamespace, threadId }, + { ...base, workloadType: 'Job' }, + ), + ).toThrow(/workload type must match source/); + expect(() => + inst.rehome( + { namespace: sourceNamespace, threadId }, + { ...base, namespace: sourceNamespace }, + ), + ).toThrow(/namespace must differ from source/); + }); + + it('restores the source and removes every target write on a determinate failure', () => { + const threadStore = new InMemoryThreadStore(); + const eventLogStore = new InMemoryEventLogStore(); + const turnStore = new InMemoryTurnStore(); + const sourceNamespace = ns('canvas_1'); + const targetNamespace = ns('canvas_2'); + const threadId = 'thread_1'; + seedSource( + threadStore, + eventLogStore, + turnStore, + sourceNamespace, + threadId, + ); + const sourceRecord = threadStore.get(sourceNamespace, threadId)!; + const sourceEvents = eventLogStore.readRecords(sourceNamespace, threadId); + const sourceTurns = turnStore.list(sourceNamespace, threadId); + + // The target thread-record write (step 3 — the destination visibility + // point) rejects AFTER both target logs (steps 1-2) already succeeded, + // so rollback must undo those two log writes and leave the source + // completely untouched. + let upsertCalls = 0; + const failingThreadStore: ThreadStore = { + upsert(namespace, id, record) { + if (namespace.name === targetNamespace.name) { + upsertCalls += 1; + throw new Error('simulated target record write failure'); + } + threadStore.upsert(namespace, id, record); + }, + get: (namespace, id) => threadStore.get(namespace, id), + list: (namespace) => threadStore.list(namespace), + delete: (namespace, id) => threadStore.delete(namespace, id), + }; + + const inst = mountAgenetes({ + drivers: { external: stubDriver() }, + threadStore: failingThreadStore, + eventLogStore, + turnStore, + }); + const targetSpec = targetSpecFor( + sourceRecord.spec as StubSpec, + targetNamespace, + ); + + expect(() => + inst.rehome({ namespace: sourceNamespace, threadId }, targetSpec), + ).toThrow(/simulated target record write failure/); + expect(upsertCalls).toBe(1); + + // Target logs written during the attempt are fully rolled back. + expect(eventLogStore.readRecords(targetNamespace, threadId)).toEqual([]); + expect(turnStore.list(targetNamespace, threadId)).toEqual([]); + expect(threadStore.get(targetNamespace, threadId)).toBeUndefined(); + + // Source is byte-for-byte unchanged. + expect(threadStore.get(sourceNamespace, threadId)).toEqual(sourceRecord); + expect(eventLogStore.readRecords(sourceNamespace, threadId)).toEqual( + sourceEvents, + ); + expect(turnStore.list(sourceNamespace, threadId)).toEqual(sourceTurns); + }); + + it('reports a distinct unknown-outcome error when the rollback itself fails', () => { + const threadStore = new InMemoryThreadStore(); + const eventLogStore = new InMemoryEventLogStore(); + const realTurnStore = new InMemoryTurnStore(); + const sourceNamespace = ns('canvas_1'); + const targetNamespace = ns('canvas_2'); + const threadId = 'thread_1'; + seedSource( + threadStore, + eventLogStore, + realTurnStore, + sourceNamespace, + threadId, + ); + const sourceRecord = threadStore.get(sourceNamespace, threadId)!; + const sourceEvents = eventLogStore.readRecords(sourceNamespace, threadId); + const sourceTurns = realTurnStore.list(sourceNamespace, threadId); + + // `delete()` always rejects: this fails the source Tier-2 removal + // (the last forward step, after everything else already succeeded) AND + // fails the compensation that would otherwise undo the target Tier-2 + // write (step 2's rollback), so the rollback itself cannot fully + // restore the pre-call state and rehome() must report the distinct + // unknown-outcome error rather than silently claiming success. + const flakyTurnStore: TurnStore = { + append: (namespace, id, persisted) => + realTurnStore.append(namespace, id, persisted), + list: (namespace, id) => realTurnStore.list(namespace, id), + count: (namespace, id) => realTurnStore.count(namespace, id), + fence: (namespace, id) => realTurnStore.fence(namespace, id), + replace: (namespace, id, persisted: readonly PersistedTurn[]) => + realTurnStore.replace(namespace, id, persisted), + delete() { + throw new Error('simulated turn log delete failure'); + }, + }; + + const inst = mountAgenetes({ + drivers: { external: stubDriver() }, + threadStore, + eventLogStore, + turnStore: flakyTurnStore, + }); + const targetSpec = targetSpecFor( + sourceRecord.spec as StubSpec, + targetNamespace, + ); + + let caught: unknown; + try { + inst.rehome({ namespace: sourceNamespace, threadId }, targetSpec); + } catch (error) { + caught = error; + } + expect(caught).toBeInstanceOf(AgenetesError); + expect((caught as InstanceType).code).toBe( + 'rehome_unknown_outcome', + ); + expect((caught as Error).message).toMatch(/unknown/); + + // The source record and Tier-1 log — the steps whose compensations DID + // succeed — are restored; the Tier-2 log is left in a genuinely + // unresolved state: the source copy was never actually removed (its + // own forward `delete()` is what failed), but the leftover TARGET + // Tier-2 write from step 2 could not be cleaned up because its + // compensation is the very call that keeps throwing. + expect(threadStore.get(sourceNamespace, threadId)).toEqual(sourceRecord); + expect(eventLogStore.readRecords(sourceNamespace, threadId)).toEqual( + sourceEvents, + ); + expect(realTurnStore.list(sourceNamespace, threadId)).toEqual(sourceTurns); + expect(realTurnStore.list(targetNamespace, threadId)).toEqual(sourceTurns); + }); +}); diff --git a/external/agenetes/packages/agenetes/src/instance.ts b/external/agenetes/packages/agenetes/src/instance.ts index 8eeb8974c..7bf80e4c0 100644 --- a/external/agenetes/packages/agenetes/src/instance.ts +++ b/external/agenetes/packages/agenetes/src/instance.ts @@ -82,6 +82,40 @@ export interface Agenetes { * starts with an empty target state. */ fork(source: ThreadIdentity, targetSpec: WorkloadSpec): AgentHandle; + /** + * The destructive counterpart to {@link Agenetes.fork}: relocate a + * durable thread's complete conversation ownership — its thread record, + * Tier-1 event log, and Tier-2 turn log — from `source` to the namespace + * and host-owned spec context in `targetSpec`, preserving `threadId`, + * driver kind, workload type, and driver state unchanged (I9.4 / I9.8). + * Unlike `fork`, this MUTATES the source: on success the source record + * and both source logs no longer exist and the target is the sole + * durable owner of the thread's history. + * + * Preconditions (both checked before any write): `source` has no live + * handle (the host must have closed/never spawned it), and the target + * `(namespace, threadId)` holds no thread record and no Tier-1/Tier-2 log + * — a rehome never overwrites an existing target. + * + * Durable ordering: the target Tier-1 log, then the target Tier-2 log, + * then the target thread record are written FIRST — the target record + * write is the destination visibility point, the first moment `record` / + * `records` observe the thread under `targetSpec.namespace`. Only once + * the target is completely durable are the source record and then the + * source logs removed, so a reader never observes the thread missing + * from both sides at once. + * + * On a determinate failure at any step, `rehome` restores the source to + * its pre-call snapshot and removes every target record/log it wrote, + * then re-throws the original error — the source is left unchanged + * from the caller's perspective. If that restoration itself fails, the + * unresolved outcome is reported as a distinct + * `rehome_unknown_outcome` {@link AgenetesError}, which wraps the + * original failure and the rollback failure; a caller must treat this as + * "unknown, do not assume the source is intact" rather than a normal + * determinate failure. + */ + rehome(source: ThreadIdentity, targetSpec: WorkloadSpec): void; /** * Pure lookup of the live handle for `threadId` — **never spawns** * (I9.3). A missing handle is a precondition failure (e.g. a control @@ -603,6 +637,160 @@ export function createAgenetesInstance( { driverState: target.driver.initialState() }, ); }, + rehome(source: ThreadIdentity, rawTargetSpec: WorkloadSpec): void { + if (runtime.get(source.threadId) !== undefined) { + throw new AgenetesError( + 'rehome_conflict', + `cannot rehome thread '${source.namespace.name}/${source.threadId}' with a live handle`, + ); + } + const sourceRecord = threadStore.get(source.namespace, source.threadId); + if (!sourceRecord) { + throw new AgenetesError( + 'invalid_workload', + `cannot rehome missing source thread '${source.namespace.name}/${source.threadId}'`, + ); + } + const validatedSource = validateRecord(sourceRecord); + const target = validateSpec(rawTargetSpec); + const targetSpec = target.spec; + if (targetSpec.threadId !== source.threadId) { + throw new AgenetesError( + 'invalid_workload', + 'rehome target threadId must equal source threadId', + ); + } + if (targetSpec.namespace.name === source.namespace.name) { + throw new AgenetesError( + 'invalid_workload', + 'rehome target namespace must differ from source', + ); + } + if (targetSpec.kind !== validatedSource.spec.kind) { + throw new AgenetesError( + 'invalid_workload', + `rehome target driver kind must match source '${validatedSource.spec.kind}'`, + ); + } + if (targetSpec.workloadType !== validatedSource.spec.workloadType) { + throw new AgenetesError( + 'invalid_workload', + `rehome target workload type must match source '${validatedSource.spec.workloadType}'`, + ); + } + const targetHasRecord = + threadStore.get(targetSpec.namespace, targetSpec.threadId) !== + undefined; + const targetHasTurns = + turnStore.list(targetSpec.namespace, targetSpec.threadId).length > 0; + const targetHasEvents = + eventLog.readRecords(targetSpec.namespace, targetSpec.threadId).length > + 0; + if (targetHasRecord || targetHasTurns || targetHasEvents) { + throw new AgenetesError( + 'rehome_conflict', + `rehome target thread already exists '${targetSpec.namespace.name}/${targetSpec.threadId}'`, + ); + } + + // Snapshot the complete source BEFORE any write, so a determinate + // failure at any later step can restore it byte-for-byte regardless + // of which step failed. + const sourceEvents = eventLog.readRecords( + source.namespace, + source.threadId, + ); + const sourceTurns = turnStore.list(source.namespace, source.threadId); + const targetRecord: ThreadRecord = { + driverSchemaVersion: validatedSource.driverSchemaVersion, + spec: targetSpec, + state: validatedSource.state, + }; + + // Each step's compensation is pushed ONLY once the step itself + // durably succeeds, so a mid-sequence failure unwinds exactly the + // completed prefix — never more, never less. + const undo: Array<() => void> = []; + const step = (write: () => void, compensate: () => void): void => { + write(); + undo.push(compensate); + }; + + try { + // Target Tier-1 log, then target Tier-2 log, then the target + // thread record LAST — the record write is the destination + // visibility point (I9.4): the first moment a reader can observe + // the thread under `targetSpec.namespace`. + step( + () => + eventLog.replace( + targetSpec.namespace, + targetSpec.threadId, + sourceEvents, + ), + () => eventLog.delete(targetSpec.namespace, targetSpec.threadId), + ); + step( + () => + turnStore.replace( + targetSpec.namespace, + targetSpec.threadId, + sourceTurns, + ), + () => turnStore.delete(targetSpec.namespace, targetSpec.threadId), + ); + step( + () => + threadStore.upsert( + targetSpec.namespace, + targetSpec.threadId, + targetRecord, + ), + () => threadStore.delete(targetSpec.namespace, targetSpec.threadId), + ); + // Only once the target is completely durable: remove the source + // record (its own visibility point) before its now-orphaned logs. + step( + () => threadStore.delete(source.namespace, source.threadId), + () => + threadStore.upsert(source.namespace, source.threadId, sourceRecord), + ); + step( + () => eventLog.delete(source.namespace, source.threadId), + () => + eventLog.replace(source.namespace, source.threadId, sourceEvents), + ); + step( + () => turnStore.delete(source.namespace, source.threadId), + () => + turnStore.replace(source.namespace, source.threadId, sourceTurns), + ); + } catch (error) { + // Unwind the completed prefix in reverse (LIFO) order, restoring + // the source snapshot and removing every target record/log this + // call wrote. Each store primitive either durably succeeds or + // throws with no partial effect, so a compensation failure here + // means the true state is genuinely unknown, not just "source + // unchanged" — that becomes its own distinct error rather than a + // silently swallowed best-effort cleanup. + const rollbackErrors: unknown[] = []; + for (const compensate of undo.reverse()) { + try { + compensate(); + } catch (rollbackError) { + rollbackErrors.push(rollbackError); + } + } + if (rollbackErrors.length > 0) { + throw new AgenetesError( + 'rehome_unknown_outcome', + `rehome for '${source.namespace.name}/${source.threadId}' failed and rollback could not fully restore the source; the outcome is unknown and requires manual recovery`, + { cause: error, rollbackErrors }, + ); + } + throw error; + } + }, get(threadId: string): AgentHandle | undefined { return runtime.get(threadId); }, diff --git a/external/agenetes/packages/agenetes/src/io.ts b/external/agenetes/packages/agenetes/src/io.ts index e70489d8a..0dc0cfc38 100644 --- a/external/agenetes/packages/agenetes/src/io.ts +++ b/external/agenetes/packages/agenetes/src/io.ts @@ -10,6 +10,7 @@ import { mkdirSync, readFileSync, renameSync, + unlinkSync, writeFileSync, } from 'node:fs'; import path from 'node:path'; @@ -100,6 +101,35 @@ export function appendJsonLine(filePath: string, data: unknown): void { appendFileSync(filePath, `${JSON.stringify(data)}\n`, 'utf-8'); } +/** + * Atomically OVERWRITE a JSONL file with exactly `items`, one per line + * (write to a `.tmp` sibling, then rename), replacing any prior content. + * Unlike {@link appendJsonLine} this is O(whole file) and is reserved for + * whole-log replacement (the `rehome()` durable-move primitive), never for + * per-event streaming appends. An empty `items` still creates the file so a + * subsequent read observes a present-but-empty log rather than a missing one. + */ +export function writeJsonLines( + filePath: string, + items: readonly unknown[], +): void { + mkdirSync(path.dirname(filePath), { recursive: true }); + const tmp = `${filePath}.tmp`; + const content = items.map((item) => `${JSON.stringify(item)}\n`).join(''); + writeFileSync(tmp, content, 'utf-8'); + renameOverWithRetry(tmp, filePath); +} + +/** + * Remove a file if present; a no-op when it is already missing (mirrors the + * idempotent `delete()` semantics of {@link FileThreadStore}). Used to + * physically drop a thread's Tier-1/Tier-2 log file once it has been + * relocated (or to compensate a failed `rehome()`). + */ +export function removeFileIfExists(filePath: string): void { + if (existsSync(filePath)) unlinkSync(filePath); +} + /** * Read a JSONL file into an array, one parsed value per non-empty line. * Returns `[]` when the file is missing or unreadable, and silently skips diff --git a/external/agenetes/packages/agenetes/src/turn-store.test.ts b/external/agenetes/packages/agenetes/src/turn-store.test.ts index 3e11497e4..ec9121b05 100644 --- a/external/agenetes/packages/agenetes/src/turn-store.test.ts +++ b/external/agenetes/packages/agenetes/src/turn-store.test.ts @@ -6,7 +6,7 @@ // resumes from), on-disk round-trip + tolerance (a corrupt tail line never // bricks a read), and per-`(namespace, threadId)` isolation. -import { appendFileSync, mkdtempSync, rmSync } from 'node:fs'; +import { appendFileSync, existsSync, mkdtempSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import path from 'node:path'; @@ -110,6 +110,42 @@ describe.each<[string, () => TurnStore]>([ expect(store.list(a, 'missing')).toEqual([]); expect(store.fence(a, 'missing')).toBe(0); }); + + it('replace() overwrites the whole log and reseeds count()/fence() (the rehome() move primitive)', () => { + const store = make(); + const source = ns('canvas-1'); + const target = ns('canvas-2'); + store.append(source, 'thread-1', persisted('a', 1, 3)); + store.append(source, 'thread-1', persisted('b', 4, 7)); + const snapshot = store.list(source, 'thread-1'); + + store.replace(target, 'thread-1', snapshot); + expect(store.list(target, 'thread-1')).toEqual(snapshot); + expect(store.count(target, 'thread-1')).toBe(2); + expect(store.fence(target, 'thread-1')).toBe(7); + + // A second replace() overwrites in full rather than appending. + store.replace(target, 'thread-1', [snapshot[0]!]); + expect(store.list(target, 'thread-1')).toEqual([snapshot[0]]); + expect(store.count(target, 'thread-1')).toBe(1); + expect(store.fence(target, 'thread-1')).toBe(3); + }); + + it('delete() removes a log entirely and idempotently', () => { + const store = make(); + const n = ns('canvas-1'); + store.append(n, 'thread-1', persisted('a', 1, 3)); + store.delete(n, 'thread-1'); + expect(store.list(n, 'thread-1')).toEqual([]); + expect(store.count(n, 'thread-1')).toBe(0); + expect(store.fence(n, 'thread-1')).toBe(0); + // Deleting an already-missing log is a no-op, not an error. + expect(() => store.delete(n, 'thread-1')).not.toThrow(); + // A fresh append after delete starts a clean log, not one that merges + // with stale cached metadata for the deleted path. + store.append(n, 'thread-1', persisted('fresh', 1, 1)); + expect(store.count(n, 'thread-1')).toBe(1); + }); }); describe('FileTurnStore — on-disk specifics', () => { @@ -138,4 +174,34 @@ describe('FileTurnStore — on-disk specifics', () => { expect(list.map((p) => p.seqEnd)).toEqual([3, 7]); expect(store.fence(n, 'thread-1')).toBe(7); }); + + it('replace() writes the target file and a fresh store instance reads it back', () => { + const source = ns('canvas-1'); + const target = ns('canvas-2'); + const writer = new FileTurnStore(); + writer.append(source, 'thread-1', persisted('a', 1, 3)); + writer.append(source, 'thread-1', persisted('b', 4, 7)); + const snapshot = writer.list(source, 'thread-1'); + + writer.replace(target, 'thread-1', snapshot); + expect(existsSync(turnFilePath(target, 'thread-1'))).toBe(true); + + const restarted = new FileTurnStore(); + expect(restarted.list(target, 'thread-1')).toEqual(snapshot); + expect(restarted.fence(target, 'thread-1')).toBe(7); + }); + + it('delete() removes the on-disk file so a fresh store observes an empty log', () => { + const n = ns('canvas-1'); + const writer = new FileTurnStore(); + writer.append(n, 'thread-1', persisted('a', 1, 3)); + expect(existsSync(turnFilePath(n, 'thread-1'))).toBe(true); + + writer.delete(n, 'thread-1'); + expect(existsSync(turnFilePath(n, 'thread-1'))).toBe(false); + + const restarted = new FileTurnStore(); + expect(restarted.list(n, 'thread-1')).toEqual([]); + expect(restarted.count(n, 'thread-1')).toBe(0); + }); }); diff --git a/external/agenetes/packages/agenetes/src/turn-store.ts b/external/agenetes/packages/agenetes/src/turn-store.ts index f32149f5e..8c384a4a0 100644 --- a/external/agenetes/packages/agenetes/src/turn-store.ts +++ b/external/agenetes/packages/agenetes/src/turn-store.ts @@ -20,7 +20,13 @@ // on-disk backing, one JSONL file per thread under the namespace's // `chat_v2/` sub-dir, the folded twin of the Tier-1 `.events.jsonl`). -import { appendJsonLine, readJsonLines, sanitizeId } from './io.js'; +import { + appendJsonLine, + readJsonLines, + removeFileIfExists, + sanitizeId, + writeJsonLines, +} from './io.js'; import type { AgentTurn, Namespace } from '@agenetes/protocol'; @@ -64,6 +70,24 @@ export interface TurnStore { * the very first event). */ fence(namespace: Namespace, threadId: string): number; + /** + * Overwrite a thread's ENTIRE Tier-2 log with `persisted` (already in fold + * order), replacing whatever the target held before. A narrow capability + * reserved for the `rehome()` durable-move primitive — it writes the + * destination turn log wholesale from a source snapshot, never for + * incremental folds (those stay on `append`). + */ + replace( + namespace: Namespace, + threadId: string, + persisted: readonly PersistedTurn[], + ): void; + /** + * Remove a thread's Tier-2 log entirely (idempotent). Reserved for the + * `rehome()` primitive: dropping the source log after its target twin is + * durable, or compensating a target log written during a failed rehome. + */ + delete(namespace: Namespace, threadId: string): void; } /** Defensive shape-check for a persisted record read back from disk. */ @@ -88,12 +112,17 @@ function isPersistedTurn(value: unknown): value is PersistedTurn { export class InMemoryTurnStore implements TurnStore { readonly #byNamespace = new Map>(); - #log(namespace: Namespace, threadId: string): PersistedTurn[] { + #scope(namespace: Namespace): Map { let scope = this.#byNamespace.get(namespace.name); if (!scope) { scope = new Map(); this.#byNamespace.set(namespace.name, scope); } + return scope; + } + + #log(namespace: Namespace, threadId: string): PersistedTurn[] { + const scope = this.#scope(namespace); let log = scope.get(threadId); if (!log) { log = []; @@ -123,6 +152,18 @@ export class InMemoryTurnStore implements TurnStore { const log = this.#byNamespace.get(namespace.name)?.get(threadId); return log && log.length > 0 ? log[log.length - 1]!.seqEnd : 0; } + + replace( + namespace: Namespace, + threadId: string, + persisted: readonly PersistedTurn[], + ): void { + this.#scope(namespace).set(threadId, [...persisted]); + } + + delete(namespace: Namespace, threadId: string): void { + this.#byNamespace.get(namespace.name)?.delete(threadId); + } } /** @@ -187,6 +228,22 @@ export class FileTurnStore implements TurnStore { return this.#metadata(this.#path(namespace, threadId)).fence; } + replace( + namespace: Namespace, + threadId: string, + persisted: readonly PersistedTurn[], + ): void { + const filePath = this.#path(namespace, threadId); + writeJsonLines(filePath, persisted); + this.#metadataByPath.set(filePath, this.#metadataFor(persisted)); + } + + delete(namespace: Namespace, threadId: string): void { + const filePath = this.#path(namespace, threadId); + removeFileIfExists(filePath); + this.#metadataByPath.delete(filePath); + } + #read(filePath: string): PersistedTurn[] { return readJsonLines(filePath).filter(isPersistedTurn); } diff --git a/external/agenetes/packages/runtime/src/errors.ts b/external/agenetes/packages/runtime/src/errors.ts index 6c5a3340b..670f73855 100644 --- a/external/agenetes/packages/runtime/src/errors.ts +++ b/external/agenetes/packages/runtime/src/errors.ts @@ -5,7 +5,9 @@ export type AgenetesErrorCode = | 'invalid_driver_spec' | 'invalid_driver_state' | 'invalid_driver_definition' - | 'invalid_persisted_record'; + | 'invalid_persisted_record' + | 'rehome_conflict' + | 'rehome_unknown_outcome'; /** Structured synchronous failure surfaced by the Agenetes control plane. */ export class AgenetesError extends Error { From ccdf787641be1b64fa5c68905401fc51a23dc110 Mon Sep 17 00:00:00 2001 From: Yuqing Yang Date: Wed, 2 Sep 2026 08:37:17 +0000 Subject: [PATCH 04/12] feat: move selections between Spaces Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/modules/canvas/canvas-executor.ts | 910 +++++++++--------- .../server/src/modules/canvas/canvas.route.ts | 29 + .../modules/canvas/space-move-plan.test.ts | 123 +++ .../src/modules/canvas/space-move-plan.ts | 287 ++++++ .../modules/canvas/space-move.service.test.ts | 167 ++++ .../src/modules/canvas/space-move.service.ts | 456 +++++++++ .../src/modules/canvas/write-coordinator.ts | 21 + apps/web/src/api/_routes.ts | 2 + apps/web/src/api/canvas.ts | 13 + .../src/components/Panels/Canvas/Canvas.tsx | 2 + .../FloatingToolbars/MultiSelectToolbar.tsx | 22 +- .../FloatingToolbars/NodeFloatingToolbar.tsx | 19 +- .../Panels/Canvas/MoveSelectionModal.tsx | 167 ++++ apps/web/src/i18n/resources/en/common.json | 15 + apps/web/src/i18n/resources/zh-CN/common.json | 15 + apps/web/src/store/canvasStore.ts | 5 + docs/architecture/agent-architecture.md | 1 + .../canvas-command-architecture.md | 2 + docs/architecture/canvas-storage.md | 2 + docs/architecture/web-architecture.md | 4 + .../move-selected-nodes-between-spaces.md | 2 +- packages/shared/src/types/api/index.ts | 1 + .../shared/src/types/api/space-move.test.ts | 69 ++ packages/shared/src/types/api/space-move.ts | 79 ++ 24 files changed, 1977 insertions(+), 436 deletions(-) create mode 100644 apps/server/src/modules/canvas/space-move-plan.test.ts create mode 100644 apps/server/src/modules/canvas/space-move-plan.ts create mode 100644 apps/server/src/modules/canvas/space-move.service.test.ts create mode 100644 apps/server/src/modules/canvas/space-move.service.ts create mode 100644 apps/web/src/components/Panels/Canvas/MoveSelectionModal.tsx create mode 100644 packages/shared/src/types/api/space-move.test.ts create mode 100644 packages/shared/src/types/api/space-move.ts diff --git a/apps/server/src/modules/canvas/canvas-executor.ts b/apps/server/src/modules/canvas/canvas-executor.ts index 97b5eadc7..02248a18b 100644 --- a/apps/server/src/modules/canvas/canvas-executor.ts +++ b/apps/server/src/modules/canvas/canvas-executor.ts @@ -137,7 +137,7 @@ function stripNodesForCanvas(nodes: readonly CanvasNode[]): CanvasNode[] { }); } -function hydrateNodes( +export function hydrateCanvasNodes( records: ReadonlyMap, nodes: readonly CanvasNode[], ): CanvasNode[] { @@ -587,6 +587,8 @@ export interface ExecuteOnServerInput { * (ACP agents) opts in so the built-in agent path pays no cost. */ computeChanges?: boolean; + /** Internal coordination hook; ordinary callers always publish. */ + publish?: boolean; } export interface ExecuteOnServerOutput { @@ -672,7 +674,7 @@ export class CanvasNotFoundError extends Error { export async function executeOnServer( input: ExecuteOnServerInput, ): Promise { - const { canvasId, originator, runId } = input; + const { canvasId, originator } = input; let commands = preAssignIds(input.commands); // Normalize agent-authored `data.src` values into artifact keys BEFORE the @@ -692,90 +694,74 @@ export async function executeOnServer( commands = await normalizeImageNodeSizes(canvasId, commands); } - return await withCanvasMutex(canvasId, async () => { - const handle = space(canvasId); - const canvas = await handle.read(); - if (!canvas) throw new CanvasNotFoundError(canvasId); + return await withCanvasMutex(canvasId, () => + executeOnServerAlreadyLocked({ ...input, commands }), + ); +} + +/** + * Execute against a Canvas whose write mutex is already held by the caller. + * + * Cross-Canvas application services use this entry to keep both Canvas locks + * across a coordinated operation. Ordinary callers must use + * {@link executeOnServer}. + */ +export async function executeOnServerAlreadyLocked( + input: ExecuteOnServerInput, +): Promise { + const { canvasId, originator, runId } = input; + let commands = [...input.commands]; + + const handle = space(canvasId); + const canvas = await handle.read(); + if (!canvas) throw new CanvasNotFoundError(canvasId); - // Executor prestate is whole-Space work: every md-backed node in the - // topology needs its stored content before the engine sees it. - const records = await handle.nodes.list(); + // Executor prestate is whole-Space work: every md-backed node in the + // topology needs its stored content before the engine sees it. + const records = await handle.nodes.list(); - const fromVersion = canvas.version; + const fromVersion = canvas.version; - // Hydrate per-node content from .md sidecars before the engine sees - // the prestate — handlers like MERGE_NODE_DATA need the current - // `data.content` to merge against, but topology never carries it. - const prestateNodes = hydrateNodes( - records, - canvas.state.nodes as CanvasNode[], - ); - const prestateEdges = (canvas.state.edges ?? []) as CanvasEdge[]; + // Hydrate per-node content from .md sidecars before the engine sees + // the prestate — handlers like MERGE_NODE_DATA need the current + // `data.content` to merge against, but topology never carries it. + const prestateNodes = hydrateCanvasNodes( + records, + canvas.state.nodes as CanvasNode[], + ); + const prestateEdges = (canvas.state.edges ?? []) as CanvasEdge[]; - assertWorldPortalMutationsAllowed( + assertWorldPortalMutationsAllowed( + canvasId, + commands, + prestateNodes, + originator.source, + ); + + if (originator.source === 'agent') { + // Order matters: fix explicit image resizes first (edits items in + // place), then let the merge pass append geometry for src-swaps that + // have no explicit resize. The two target disjoint node sets. + commands = await normalizeSetGeometryImageSizes( canvasId, commands, prestateNodes, - originator.source, ); - - if (originator.source === 'agent') { - // Order matters: fix explicit image resizes first (edits items in - // place), then let the merge pass append geometry for src-swaps that - // have no explicit resize. The two target disjoint node sets. - commands = await normalizeSetGeometryImageSizes( - canvasId, - commands, - prestateNodes, - ); - commands = await normalizeMergeImageGeometry( - canvasId, - commands, - prestateNodes, - ); - } - - // Compare-and-swap pre-flight (agent writes only). A stale or - // never-read content rewrite mutates NOTHING — the whole batch is a - // no-op and the agent reconciles from the echoed `currentContent`. - // ui / system writes are trusted and skip the guard. - if (originator.source === 'agent') { - const conflicts = collectMergeConflicts(commands, prestateNodes); - if (conflicts.length > 0) { - const conflictIds = new Set(conflicts.map((c) => c.nodeId)); - return { - canvasId, - fromVersion, - toVersion: fromVersion, - deltas: [], - results: commands.map((command) => ({ - command, - applied: false, - ...(command.type === 'MERGE_NODE_DATA' && - command.patches.some((p) => conflictIds.has(p.nodeId)) - ? { reason: 'conflict' as const } - : {}), - })), - commands, - pendingEffects: { - mutatedNodes: [], - deletedNodeIds: [], - contentEditedNodeIds: [], - deferredFitFrameIds: [], - }, - conflicts, - }; - } - } - - const viewConflicts = collectInteractiveViewConflicts( + commands = await normalizeMergeImageGeometry( + canvasId, commands, prestateNodes, ); - if (viewConflicts.length > 0) { - const conflictIds = new Set( - viewConflicts.map((conflict) => conflict.nodeId), - ); + } + + // Compare-and-swap pre-flight (agent writes only). A stale or + // never-read content rewrite mutates NOTHING — the whole batch is a + // no-op and the agent reconciles from the echoed `currentContent`. + // ui / system writes are trusted and skip the guard. + if (originator.source === 'agent') { + const conflicts = collectMergeConflicts(commands, prestateNodes); + if (conflicts.length > 0) { + const conflictIds = new Set(conflicts.map((c) => c.nodeId)); return { canvasId, fromVersion, @@ -785,7 +771,7 @@ export async function executeOnServer( command, applied: false, ...(command.type === 'MERGE_NODE_DATA' && - command.patches.some((patch) => conflictIds.has(patch.nodeId)) + command.patches.some((p) => conflictIds.has(p.nodeId)) ? { reason: 'conflict' as const } : {}), })), @@ -796,259 +782,290 @@ export async function executeOnServer( contentEditedNodeIds: [], deferredFitFrameIds: [], }, - viewConflicts, + conflicts, }; } + } - const { writeResult, commandResults, pendingEffects } = - executeCanvasCommands( - { source: originator.source, commands }, - { - nodes: prestateNodes, - edges: prestateEdges, - canvasId, - }, - { forceFitFrames: originator.source === 'agent' }, - ); - - // Pure host-agnostic cleanups (edge handle reroute) — same path the - // web's `executeCommands` runs before its set(). - const sharedOut = applySharedPostEffectsFromWriteResult(writeResult); - const finalNodes = writeResult.nodes; - const finalEdges = sharedOut.edges; - assertWorldPortalResultAllowed(canvasId, prestateNodes, finalNodes); - - const deltas = diffCanvasState( - { nodes: prestateNodes, edges: prestateEdges }, - { nodes: finalNodes, edges: finalEdges }, + const viewConflicts = collectInteractiveViewConflicts( + commands, + prestateNodes, + ); + if (viewConflicts.length > 0) { + const conflictIds = new Set( + viewConflicts.map((conflict) => conflict.nodeId), ); + return { + canvasId, + fromVersion, + toVersion: fromVersion, + deltas: [], + results: commands.map((command) => ({ + command, + applied: false, + ...(command.type === 'MERGE_NODE_DATA' && + command.patches.some((patch) => conflictIds.has(patch.nodeId)) + ? { reason: 'conflict' as const } + : {}), + })), + commands, + pendingEffects: { + mutatedNodes: [], + deletedNodeIds: [], + contentEditedNodeIds: [], + deferredFitFrameIds: [], + }, + viewConflicts, + }; + } + + const { writeResult, commandResults, pendingEffects } = executeCanvasCommands( + { source: originator.source, commands }, + { + nodes: prestateNodes, + edges: prestateEdges, + canvasId, + }, + { forceFitFrames: originator.source === 'agent' }, + ); - // Built once: id → final node, used to echo image dimensions back so - // agents can lay out follow-up nodes with exact geometry. - const finalById = new Map(); - for (const node of finalNodes) finalById.set(node.id as string, node); + // Pure host-agnostic cleanups (edge handle reroute) — same path the + // web's `executeCommands` runs before its set(). + const sharedOut = applySharedPostEffectsFromWriteResult(writeResult); + const finalNodes = writeResult.nodes; + const finalEdges = sharedOut.edges; + assertWorldPortalResultAllowed(canvasId, prestateNodes, finalNodes); - const results = commandResults.map((r) => { - const result: ExecuteOnServerOutput['results'][0] = { - command: r.command, - applied: r.applied, - ...(r.reason ? { reason: r.reason } : {}), - }; + const deltas = diffCanvasState( + { nodes: prestateNodes, edges: prestateEdges }, + { nodes: finalNodes, edges: finalEdges }, + ); - // Echo created node ids (+labels) so the agent can wire them up in a - // follow-up CONNECT_NODES / SET_NODE_PARENT call with the real, - // server-assigned ids instead of inventing ids that collide across - // runs. Image nodes also carry server-derived dimensions/src. - if (r.applied && r.command.type === 'CREATE_NODES') { - const nodes = r.command.nodes - .map((n) => { - const node = finalById.get(n.id as string); - if (!node) return null; - const style = (node.style ?? {}) as Record; - const label = node.data?.label; - return { - nodeId: node.id as string, - ...(typeof label === 'string' ? { label } : {}), - width: typeof style.width === 'number' ? style.width : 0, - height: typeof style.height === 'number' ? style.height : 0, - ...(node.type === 'image' && typeof node.data?.src === 'string' - ? { src: node.data.src } - : {}), - }; - }) - .filter((n): n is NonNullable => n !== null); - - if (nodes.length > 0) result.nodes = nodes; - } else if (r.applied && r.command.type === 'CONNECT_NODES') { - const edges = r.command.edges.flatMap((edge) => - edge.id - ? [ - { - edgeId: edge.id, - source: edge.source, - target: edge.target, - }, - ] - : [], - ); - if (edges.length > 0) result.edges = edges; - } else if (r.applied && r.command.type === 'MERGE_NODE_DATA') { - // Echo final image dimensions when a MERGE rewrote an image src. - const nodes = r.command.patches - .filter((p) => typeof p.patch?.['src'] === 'string') - .map((p) => { - const node = finalById.get(p.nodeId); - if (node?.type !== 'image') return null; - const style = (node.style ?? {}) as Record; - return { - nodeId: p.nodeId, - width: typeof style.width === 'number' ? style.width : 0, - height: typeof style.height === 'number' ? style.height : 0, - src: (node.data?.src as string) || '', - }; - }) - .filter((n): n is NonNullable => n !== null); - - if (nodes.length > 0) result.nodes = nodes; - } + // Built once: id → final node, used to echo image dimensions back so + // agents can lay out follow-up nodes with exact geometry. + const finalById = new Map(); + for (const node of finalNodes) finalById.set(node.id as string, node); - return result; - }); + const results = commandResults.map((r) => { + const result: ExecuteOnServerOutput['results'][0] = { + command: r.command, + applied: r.applied, + ...(r.reason ? { reason: r.reason } : {}), + }; - // Detect order-only mutations that `diffCanvasState` cannot see. - // - // `diffCanvasState` is id-keyed: it returns INSERT/DELETE/REPLACE rows - // by comparing id sets and per-id reference identity. Commands whose - // only effect is to reshuffle the nodes/edges array (today only - // `REORDER_NODES`, which rebuilds the array with the same refs in a - // new order) therefore emit zero structural deltas. Without this - // guard the no-op fast path below would skip persistence entirely, - // leaving the agent with `applied: true` while persisted topology - // is unchanged. - // - // We do NOT synthesise a delta — Phase A has no order-aware delta - // type, and cross-tab broadcast (M3) is not shipped yet. We just - // fall through to the persistence branch so topology and the - // delta-log version both reflect that something happened. Catch-up - // clients on M3 will see the version bump and need to refetch the - // full canvas; that's an acceptable Phase-A trade-off. - const orderChanged = - prestateNodes.length !== finalNodes.length || - prestateEdges.length !== finalEdges.length || - prestateNodes.some((n, i) => n.id !== finalNodes[i]?.id) || - prestateEdges.some((e, i) => e.id !== finalEdges[i]?.id); - - // No-op fast path. Returning early preserves the invariant that - // `toVersion === fromVersion` IFF no row was appended to the log. - if (deltas.length === 0 && !orderChanged) { - return { - canvasId, - fromVersion, - toVersion: fromVersion, - deltas, - results, - commands, - pendingEffects: { - mutatedNodes: pendingEffects.mutatedNodes, - deletedNodeIds: pendingEffects.deletedNodeIds, - contentEditedNodeIds: pendingEffects.contentEditedNodeIds, - deferredFitFrameIds: pendingEffects.deferredFitFrameIds, - }, - }; + // Echo created node ids (+labels) so the agent can wire them up in a + // follow-up CONNECT_NODES / SET_NODE_PARENT call with the real, + // server-assigned ids instead of inventing ids that collide across + // runs. Image nodes also carry server-derived dimensions/src. + if (r.applied && r.command.type === 'CREATE_NODES') { + const nodes = r.command.nodes + .map((n) => { + const node = finalById.get(n.id as string); + if (!node) return null; + const style = (node.style ?? {}) as Record; + const label = node.data?.label; + return { + nodeId: node.id as string, + ...(typeof label === 'string' ? { label } : {}), + width: typeof style.width === 'number' ? style.width : 0, + height: typeof style.height === 'number' ? style.height : 0, + ...(node.type === 'image' && typeof node.data?.src === 'string' + ? { src: node.data.src } + : {}), + }; + }) + .filter((n): n is NonNullable => n !== null); + + if (nodes.length > 0) result.nodes = nodes; + } else if (r.applied && r.command.type === 'CONNECT_NODES') { + const edges = r.command.edges.flatMap((edge) => + edge.id + ? [ + { + edgeId: edge.id, + source: edge.source, + target: edge.target, + }, + ] + : [], + ); + if (edges.length > 0) result.edges = edges; + } else if (r.applied && r.command.type === 'MERGE_NODE_DATA') { + // Echo final image dimensions when a MERGE rewrote an image src. + const nodes = r.command.patches + .filter((p) => typeof p.patch?.['src'] === 'string') + .map((p) => { + const node = finalById.get(p.nodeId); + if (node?.type !== 'image') return null; + const style = (node.style ?? {}) as Record; + return { + nodeId: p.nodeId, + width: typeof style.width === 'number' ? style.width : 0, + height: typeof style.height === 'number' ? style.height : 0, + src: (node.data?.src as string) || '', + }; + }) + .filter((n): n is NonNullable => n !== null); + + if (nodes.length > 0) result.nodes = nodes; } - const toVersion = fromVersion + 1; - - // Persist .md sidecars first so topology never references a markdown - // file that does not exist on disk. The synchronous commit section is - // wrapped in a before-image rollback: if topology or delta-log persistence - // fails, the sidecars, record, and log prefix all return to `fromVersion`. - // - // `writeNode` throws `CanvasStoreIOError` on environmental failures - // (ENOSPC, EACCES, …); we deliberately do NOT catch it so the - // batch aborts before topology is mutated. The exception bubbles - // through `handleCanvasCommands` and surfaces as an `isError: true` - // tool result to the LLM (and as a 500 / error event upstream). - // Structural `conflict` / `not-found` results are programmer errors - // in the agent path (engine should have rejected them upstream and - // `strictRename` is rarely set for agent-authored labels); we throw - // a regular Error rather than letting the in-memory mutation drift - // away from disk. - // Pending effects preserve command order and can mention the same id in - // both collections (DELETE then CREATE, or mutate then DELETE). Persist - // only the effect matching the authoritative final topology so a - // re-created node is not written and then immediately unlinked. - const finalNodeIds = new Set(finalNodes.map((node) => node.id)); - const mutatedNodesToPersist = pendingEffects.mutatedNodes.filter((node) => - finalNodeIds.has(node.id), - ); - const nodeIdsToDelete = pendingEffects.deletedNodeIds.filter( - (nodeId) => !finalNodeIds.has(nodeId), - ); - const insertedIds = insertedNodeIds(deltas); - const nodeMutations: SpaceNodeMutation[] = []; - for (const node of mutatedNodesToPersist) { - const record = buildNodeContent(node); - if (!record) continue; - nodeMutations.push({ - kind: 'put', - nodeId: record.nodeId, - record, - strictLabel: record['labelSource'] === 'user', - authoritativeInsert: insertedIds.has(record.nodeId), - }); - } - for (const nodeId of nodeIdsToDelete) { - nodeMutations.push({ kind: 'delete', nodeId }); - } + return result; + }); - const nextCanvas: CanvasFile = { - ...canvas, - version: toVersion, - state: { - ...canvas.state, - nodes: stripNodesForCanvas(finalNodes), - edges: finalEdges, + // Detect order-only mutations that `diffCanvasState` cannot see. + // + // `diffCanvasState` is id-keyed: it returns INSERT/DELETE/REPLACE rows + // by comparing id sets and per-id reference identity. Commands whose + // only effect is to reshuffle the nodes/edges array (today only + // `REORDER_NODES`, which rebuilds the array with the same refs in a + // new order) therefore emit zero structural deltas. Without this + // guard the no-op fast path below would skip persistence entirely, + // leaving the agent with `applied: true` while persisted topology + // is unchanged. + // + // We do NOT synthesise a delta — Phase A has no order-aware delta + // type, and cross-tab broadcast (M3) is not shipped yet. We just + // fall through to the persistence branch so topology and the + // delta-log version both reflect that something happened. Catch-up + // clients on M3 will see the version bump and need to refetch the + // full canvas; that's an acceptable Phase-A trade-off. + const orderChanged = + prestateNodes.length !== finalNodes.length || + prestateEdges.length !== finalEdges.length || + prestateNodes.some((n, i) => n.id !== finalNodes[i]?.id) || + prestateEdges.some((e, i) => e.id !== finalEdges[i]?.id); + + // No-op fast path. Returning early preserves the invariant that + // `toVersion === fromVersion` IFF no row was appended to the log. + if (deltas.length === 0 && !orderChanged) { + return { + canvasId, + fromVersion, + toVersion: fromVersion, + deltas, + results, + commands, + pendingEffects: { + mutatedNodes: pendingEffects.mutatedNodes, + deletedNodeIds: pendingEffects.deletedNodeIds, + contentEditedNodeIds: pendingEffects.contentEditedNodeIds, + deferredFitFrameIds: pendingEffects.deferredFitFrameIds, }, - updatedAt: Date.now(), - }; - const logEntry: DeltaLogEntry = { - version: toVersion, - ts: Date.now(), - ...(runId ? { runId } : {}), - commands: commands as unknown[], - deltas: deltas as unknown[], - originator, }; - const write = await handle.write({ - expectedVersion: fromVersion, - nextRecord: nextCanvas, - nodeMutations, - delta: logEntry, + } + + const toVersion = fromVersion + 1; + + // Persist .md sidecars first so topology never references a markdown + // file that does not exist on disk. The synchronous commit section is + // wrapped in a before-image rollback: if topology or delta-log persistence + // fails, the sidecars, record, and log prefix all return to `fromVersion`. + // + // `writeNode` throws `CanvasStoreIOError` on environmental failures + // (ENOSPC, EACCES, …); we deliberately do NOT catch it so the + // batch aborts before topology is mutated. The exception bubbles + // through `handleCanvasCommands` and surfaces as an `isError: true` + // tool result to the LLM (and as a 500 / error event upstream). + // Structural `conflict` / `not-found` results are programmer errors + // in the agent path (engine should have rejected them upstream and + // `strictRename` is rarely set for agent-authored labels); we throw + // a regular Error rather than letting the in-memory mutation drift + // away from disk. + // Pending effects preserve command order and can mention the same id in + // both collections (DELETE then CREATE, or mutate then DELETE). Persist + // only the effect matching the authoritative final topology so a + // re-created node is not written and then immediately unlinked. + const finalNodeIds = new Set(finalNodes.map((node) => node.id)); + const mutatedNodesToPersist = pendingEffects.mutatedNodes.filter((node) => + finalNodeIds.has(node.id), + ); + const nodeIdsToDelete = pendingEffects.deletedNodeIds.filter( + (nodeId) => !finalNodeIds.has(nodeId), + ); + const insertedIds = insertedNodeIds(deltas); + const nodeMutations: SpaceNodeMutation[] = []; + for (const node of mutatedNodesToPersist) { + const record = buildNodeContent(node); + if (!record) continue; + nodeMutations.push({ + kind: 'put', + nodeId: record.nodeId, + record, + strictLabel: record['labelSource'] === 'user', + authoritativeInsert: insertedIds.has(record.nodeId), }); - if (!write.ok) { - if (write.reason === 'not-found') { - throw new CanvasNotFoundError(canvasId); - } - throw new Error( - `[canvas-executor] ordered Space write rejected: ${write.reason}`, - ); + } + for (const nodeId of nodeIdsToDelete) { + nodeMutations.push({ kind: 'delete', nodeId }); + } + + const nextCanvas: CanvasFile = { + ...canvas, + version: toVersion, + state: { + ...canvas.state, + nodes: stripNodesForCanvas(finalNodes), + edges: finalEdges, + }, + updatedAt: Date.now(), + }; + const logEntry: DeltaLogEntry = { + version: toVersion, + ts: Date.now(), + ...(runId ? { runId } : {}), + commands: commands as unknown[], + deltas: deltas as unknown[], + originator, + }; + const write = await handle.write({ + expectedVersion: fromVersion, + nextRecord: nextCanvas, + nodeMutations, + delta: logEntry, + }); + if (!write.ok) { + if (write.reason === 'not-found') { + throw new CanvasNotFoundError(canvasId); } + throw new Error( + `[canvas-executor] ordered Space write rejected: ${write.reason}`, + ); + } - // Derive review records (ACP change cards) only when asked. Edge - // endpoint labels are resolved against the post-state nodes. - let changes: CanvasChangeRecord[] | undefined; - if (input.computeChanges) { - const labelById = new Map(); - for (const node of finalNodes) { - const lbl = (node.data as Record | undefined)?.[ - 'label' - ]; - if (typeof lbl === 'string' && lbl) labelById.set(node.id, lbl); - } - changes = extractCanvasChanges(deltas, { nodeLabelById: labelById }); + // Derive review records (ACP change cards) only when asked. Edge + // endpoint labels are resolved against the post-state nodes. + let changes: CanvasChangeRecord[] | undefined; + if (input.computeChanges) { + const labelById = new Map(); + for (const node of finalNodes) { + const lbl = (node.data as Record | undefined)?.['label']; + if (typeof lbl === 'string' && lbl) labelById.set(node.id, lbl); } + changes = extractCanvasChanges(deltas, { nodeLabelById: labelById }); + } - // Broadcast the delta to live frontends and persist review records to - // the originating thread's sidecar. Every accepted write broadcasts — - // the initiating tab applies it from the sync stream, not the tool - // result. No-op fast path above already returned for empty diffs. - // - // When attributed to a thread, fold this batch's records into the - // thread's coalesced change list (one net record per entity) and - // broadcast that full list so live cards replace their state with it — - // matching what GET /changes returns. - let broadcastChanges = changes; - if (originator.threadId && changes && changes.length > 0) { - try { - broadcastChanges = await handle.changes.append( - originator.threadId, - changes, - ); - } catch { - /* sidecar persistence is best-effort — never fail the write */ - } + // Broadcast the delta to live frontends and persist review records to + // the originating thread's sidecar. Every accepted write broadcasts — + // the initiating tab applies it from the sync stream, not the tool + // result. No-op fast path above already returned for empty diffs. + // + // When attributed to a thread, fold this batch's records into the + // thread's coalesced change list (one net record per entity) and + // broadcast that full list so live cards replace their state with it — + // matching what GET /changes returns. + let broadcastChanges = changes; + if (originator.threadId && changes && changes.length > 0) { + try { + broadcastChanges = await handle.changes.append( + originator.threadId, + changes, + ); + } catch { + /* sidecar persistence is best-effort — never fail the write */ } + } + if (input.publish !== false) { publishCanvasUpdate(canvasId, { type: 'update', data: { @@ -1065,23 +1082,23 @@ export async function executeOnServer( ...(broadcastChanges ? { changes: broadcastChanges } : {}), }, }); + } - return { - canvasId, - fromVersion, - toVersion, - deltas, - results, - commands, - pendingEffects: { - mutatedNodes: pendingEffects.mutatedNodes, - deletedNodeIds: pendingEffects.deletedNodeIds, - contentEditedNodeIds: pendingEffects.contentEditedNodeIds, - deferredFitFrameIds: pendingEffects.deferredFitFrameIds, - }, - ...(changes ? { changes } : {}), - }; - }); + return { + canvasId, + fromVersion, + toVersion, + deltas, + results, + commands, + pendingEffects: { + mutatedNodes: pendingEffects.mutatedNodes, + deletedNodeIds: pendingEffects.deletedNodeIds, + contentEditedNodeIds: pendingEffects.contentEditedNodeIds, + deferredFitFrameIds: pendingEffects.deferredFitFrameIds, + }, + ...(changes ? { changes } : {}), + }; } /** @@ -1112,125 +1129,76 @@ export async function applyDeltasOnServer(input: { deferredFitFrameIds: string[]; }; }> { - const { canvasId, originator, runId } = input; + const { canvasId } = input; - return await withCanvasMutex(canvasId, async () => { - const handle = space(canvasId); - const canvas = await handle.read(); - if (!canvas) throw new CanvasNotFoundError(canvasId); - - // Executor prestate is whole-Space work: every md-backed node in the - // topology needs its stored content before the engine sees it. - const records = await handle.nodes.list(); - - const fromVersion = canvas.version; - const prestateNodes = hydrateNodes( - records, - canvas.state.nodes as CanvasNode[], - ); - const prestateEdges = (canvas.state.edges ?? []) as CanvasEdge[]; + return await withCanvasMutex(canvasId, () => + applyDeltasOnServerAlreadyLocked(input), + ); +} - const final = applyDeltas( - { nodes: prestateNodes, edges: prestateEdges }, - input.deltas, - ); - const finalNodes = final.nodes; - const finalEdges = final.edges; - - // Recompute the authoritative diff so the log row and broadcast - // reflect exactly what landed (tolerates already-applied / missing - // targets in the input deltas). - const deltas = diffCanvasState( - { nodes: prestateNodes, edges: prestateEdges }, - { nodes: finalNodes, edges: finalEdges }, - ); +/** + * Apply inverse or forward deltas while the caller already owns the Canvas + * mutex. This is the compensation counterpart of + * {@link executeOnServerAlreadyLocked}. + */ +export async function applyDeltasOnServerAlreadyLocked(input: { + canvasId: string; + deltas: readonly Delta[]; + originator: ExecuteOriginator; + runId?: string; +}): Promise<{ + canvasId: string; + fromVersion: number; + toVersion: number; + deltas: Delta[]; + pendingEffects: { + mutatedNodes: CanvasNode[]; + deletedNodeIds: string[]; + contentEditedNodeIds: string[]; + deferredFitFrameIds: string[]; + }; +}> { + const { canvasId, originator, runId } = input; - const mutatedNodes: CanvasNode[] = []; - const deletedNodeIds: string[] = []; - const contentEditedNodeIds: string[] = []; + const handle = space(canvasId); + const canvas = await handle.read(); + if (!canvas) throw new CanvasNotFoundError(canvasId); - if (deltas.length === 0) { - return { - canvasId, - fromVersion, - toVersion: fromVersion, - deltas, - pendingEffects: { - mutatedNodes, - deletedNodeIds, - contentEditedNodeIds, - deferredFitFrameIds: [], - }, - }; - } + // Executor prestate is whole-Space work: every md-backed node in the + // topology needs its stored content before the engine sees it. + const records = await handle.nodes.list(); - const toVersion = fromVersion + 1; + const fromVersion = canvas.version; + const prestateNodes = hydrateCanvasNodes( + records, + canvas.state.nodes as CanvasNode[], + ); + const prestateEdges = (canvas.state.edges ?? []) as CanvasEdge[]; - for (const d of deltas) { - if (d.type === 'INSERT_NODE' || d.type === 'REPLACE_NODE') { - const node = d.type === 'INSERT_NODE' ? d.node : d.next; - mutatedNodes.push(node); - if (d.type === 'REPLACE_NODE') contentEditedNodeIds.push(node.id); - } else if (d.type === 'DELETE_NODE') { - deletedNodeIds.push(d.node.id); - } - } - const insertedIds = insertedNodeIds(deltas); - const nodeMutations: SpaceNodeMutation[] = []; - for (const d of deltas) { - if (d.type === 'INSERT_NODE' || d.type === 'REPLACE_NODE') { - const node = d.type === 'INSERT_NODE' ? d.node : d.next; - const record = buildNodeContent(node); - if (record) { - nodeMutations.push({ - kind: 'put', - nodeId: record.nodeId, - record, - strictLabel: record['labelSource'] === 'user', - authoritativeInsert: insertedIds.has(record.nodeId), - }); - } - } else if (d.type === 'DELETE_NODE') { - nodeMutations.push({ kind: 'delete', nodeId: d.node.id }); - } - } + const final = applyDeltas( + { nodes: prestateNodes, edges: prestateEdges }, + input.deltas, + ); + const finalNodes = final.nodes; + const finalEdges = final.edges; + + // Recompute the authoritative diff so the log row and broadcast + // reflect exactly what landed (tolerates already-applied / missing + // targets in the input deltas). + const deltas = diffCanvasState( + { nodes: prestateNodes, edges: prestateEdges }, + { nodes: finalNodes, edges: finalEdges }, + ); - const nextRecord: CanvasFile = { - ...canvas, - version: toVersion, - state: { - ...canvas.state, - nodes: stripNodesForCanvas(finalNodes), - edges: finalEdges, - }, - updatedAt: Date.now(), - }; - const write = await handle.write({ - expectedVersion: fromVersion, - nextRecord, - nodeMutations, - delta: { - version: toVersion, - ts: Date.now(), - ...(runId ? { runId } : {}), - commands: [], - deltas: deltas as unknown[], - originator, - }, - }); - if (!write.ok) { - if (write.reason === 'not-found') { - throw new CanvasNotFoundError(canvasId); - } - throw new Error( - `[canvas-executor] ordered Space write rejected: ${write.reason}`, - ); - } + const mutatedNodes: CanvasNode[] = []; + const deletedNodeIds: string[] = []; + const contentEditedNodeIds: string[] = []; + if (deltas.length === 0) { return { canvasId, fromVersion, - toVersion, + toVersion: fromVersion, deltas, pendingEffects: { mutatedNodes, @@ -1239,5 +1207,81 @@ export async function applyDeltasOnServer(input: { deferredFitFrameIds: [], }, }; + } + + const toVersion = fromVersion + 1; + + for (const d of deltas) { + if (d.type === 'INSERT_NODE' || d.type === 'REPLACE_NODE') { + const node = d.type === 'INSERT_NODE' ? d.node : d.next; + mutatedNodes.push(node); + if (d.type === 'REPLACE_NODE') contentEditedNodeIds.push(node.id); + } else if (d.type === 'DELETE_NODE') { + deletedNodeIds.push(d.node.id); + } + } + const insertedIds = insertedNodeIds(deltas); + const nodeMutations: SpaceNodeMutation[] = []; + for (const d of deltas) { + if (d.type === 'INSERT_NODE' || d.type === 'REPLACE_NODE') { + const node = d.type === 'INSERT_NODE' ? d.node : d.next; + const record = buildNodeContent(node); + if (record) { + nodeMutations.push({ + kind: 'put', + nodeId: record.nodeId, + record, + strictLabel: record['labelSource'] === 'user', + authoritativeInsert: insertedIds.has(record.nodeId), + }); + } + } else if (d.type === 'DELETE_NODE') { + nodeMutations.push({ kind: 'delete', nodeId: d.node.id }); + } + } + + const nextRecord: CanvasFile = { + ...canvas, + version: toVersion, + state: { + ...canvas.state, + nodes: stripNodesForCanvas(finalNodes), + edges: finalEdges, + }, + updatedAt: Date.now(), + }; + const write = await handle.write({ + expectedVersion: fromVersion, + nextRecord, + nodeMutations, + delta: { + version: toVersion, + ts: Date.now(), + ...(runId ? { runId } : {}), + commands: [], + deltas: deltas as unknown[], + originator, + }, }); + if (!write.ok) { + if (write.reason === 'not-found') { + throw new CanvasNotFoundError(canvasId); + } + throw new Error( + `[canvas-executor] ordered Space write rejected: ${write.reason}`, + ); + } + + return { + canvasId, + fromVersion, + toVersion, + deltas, + pendingEffects: { + mutatedNodes, + deletedNodeIds, + contentEditedNodeIds, + deferredFitFrameIds: [], + }, + }; } diff --git a/apps/server/src/modules/canvas/canvas.route.ts b/apps/server/src/modules/canvas/canvas.route.ts index 4dd1b1324..9f35c0716 100644 --- a/apps/server/src/modules/canvas/canvas.route.ts +++ b/apps/server/src/modules/canvas/canvas.route.ts @@ -18,6 +18,7 @@ import { getCanvasEventsQuerySchema, postCanvasEventsBodySchema, postCanvasExecuteBodySchema, + moveSelectionBodySchema, preprocessNodeBodySchema, putCanvasBodySchema, putNodeContentBodySchema, @@ -33,6 +34,7 @@ import { import { CanvasNotFoundError, applyDeltasOnServer } from './canvas-executor.js'; import { searchCanvas } from './canvas-search.js'; import { publishCanvasUpdate } from './canvas-sync.js'; +import { moveCanvasSelection, SpaceMoveError } from './space-move.service.js'; import { getSpacePreviewScene, SpacePreviewSceneError, @@ -89,6 +91,7 @@ import type { PostCanvasEventsResponse, PostCanvasExecuteRequest, PostCanvasExecuteResponse, + MoveSelectionResponse, PreprocessNodeBody, PreprocessNodeRequest, PreprocessNodeResponse, @@ -1283,6 +1286,32 @@ const canvasRoutes: FastifyPluginAsync = async (fastify) => { // Atomic per-canvas (the executor owns a mutex keyed by canvasId). // Idempotent no-op batches do not bump the version. + fastify.post<{ + Params: { canvasId: string }; + Body: unknown; + Reply: ApiResult; + }>('/:canvasId/move-selection', async function (request, reply) { + const parsed = moveSelectionBodySchema.safeParse(request.body); + if (!parsed.success) { + return reply.code(400).send({ + message: parsed.error.issues[0]?.message ?? 'Invalid request body', + }); + } + try { + return reply.send( + await moveCanvasSelection(request.params.canvasId, parsed.data), + ); + } catch (error) { + if (error instanceof SpaceMoveError) { + return reply.code(error.statusCode).send({ + code: error.code, + message: error.message, + }); + } + throw error; + } + }); + fastify.post<{ Params: { canvasId: string }; Body: PostCanvasExecuteRequest; diff --git a/apps/server/src/modules/canvas/space-move-plan.test.ts b/apps/server/src/modules/canvas/space-move-plan.test.ts new file mode 100644 index 000000000..9516c17b7 --- /dev/null +++ b/apps/server/src/modules/canvas/space-move-plan.test.ts @@ -0,0 +1,123 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import { describe, expect, it } from 'vitest'; + +import { buildSpaceMovePlan, SpaceMovePlanError } from './space-move-plan.js'; + +import type { CanvasEdge, CanvasNode } from '@huabu/shared/canvas-engine'; + +function node( + id: string, + type: string, + x: number, + y: number, + parentId?: string, +): CanvasNode { + return { + id, + type, + position: { x, y }, + data: { type, label: id }, + style: { width: 100, height: 80 }, + ...(parentId ? { parentId } : {}), + }; +} + +describe('buildSpaceMovePlan', () => { + it('deduplicates selected Frame descendants and preserves local geometry', () => { + const frame = node('frame-a', 'frame', 50, 60); + const child = node('node-a', 'note', 10, 20, frame.id); + const plan = buildSpaceMovePlan({ + sourceNodes: [frame, child], + sourceEdges: [], + destinationNodes: [], + selectedNodeIds: [frame.id, child.id], + }); + + expect(plan.rootIds).toEqual([frame.id]); + expect(plan.movedIds).toEqual(new Set([frame.id, child.id])); + const create = plan.commands[0]; + expect(create.type).toBe('CREATE_NODES'); + if (create.type !== 'CREATE_NODES') return; + const movedFrame = create.nodes.find((item) => item.nodeType === 'frame'); + const movedChild = create.nodes.find((item) => item.nodeType === 'note'); + expect(movedFrame?.position).toEqual({ x: 0, y: 0 }); + expect(movedChild?.position).toEqual({ x: 10, y: 20 }); + expect(movedChild?.parentId).toBe(movedFrame?.id); + }); + + it('preserves internal edges and reports boundary edges', () => { + const first = node('node-a', 'note', 0, 0); + const second = node('node-b', 'note', 120, 0); + const outside = node('node-c', 'note', 240, 0); + const edges: CanvasEdge[] = [ + { + id: 'edge-internal', + source: first.id, + target: second.id, + data: { edgeStyle: { label: 'kept', strokeWidth: 4 } }, + }, + { + id: 'edge-boundary', + source: second.id, + target: outside.id, + }, + ]; + const plan = buildSpaceMovePlan({ + sourceNodes: [first, second, outside], + sourceEdges: edges, + destinationNodes: [], + selectedNodeIds: [first.id, second.id], + }); + + expect(plan.commands[1]).toMatchObject({ + type: 'CONNECT_NODES', + edges: [{ style: { label: 'kept', strokeWidth: 4 } }], + }); + expect(plan.omittedBoundaryEdges).toEqual([ + { + edgeId: 'edge-boundary', + source: second.id, + target: outside.id, + }, + ]); + }); + + it('keeps Agent thread identity while assigning a fresh node id', () => { + const agent = node('node-agent', 'question', 0, 0); + agent.data = { + type: 'question', + label: 'Agent', + threadId: 'thread-agent', + status: 'done', + }; + const plan = buildSpaceMovePlan({ + sourceNodes: [agent], + sourceEdges: [], + destinationNodes: [], + selectedNodeIds: [agent.id], + }); + + expect(plan.movedThreadIds).toEqual(['thread-agent']); + expect(plan.nodeIdMap.get(agent.id)).not.toBe(agent.id); + const create = plan.commands[0]; + expect(create.type).toBe('CREATE_NODES'); + if (create.type !== 'CREATE_NODES') return; + expect(create.nodes[0]?.data).toMatchObject({ + threadId: 'thread-agent', + status: 'done', + }); + }); + + it('rejects managed reference nodes', () => { + expect(() => + buildSpaceMovePlan({ + sourceNodes: [node('node-ref', 'nodeRef', 0, 0)], + sourceEdges: [], + destinationNodes: [], + selectedNodeIds: ['node-ref'], + }), + ).toThrow(SpaceMovePlanError); + }); +}); diff --git a/apps/server/src/modules/canvas/space-move-plan.ts b/apps/server/src/modules/canvas/space-move-plan.ts new file mode 100644 index 000000000..ca45fc9da --- /dev/null +++ b/apps/server/src/modules/canvas/space-move-plan.ts @@ -0,0 +1,287 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import { + CANVAS_NODE_TYPES, + createId, + type CanvasCommand, + type CanvasEdgeId, + type CanvasNodeId, + type CanvasNodeType, + type EdgeStyle, +} from '@huabu/shared'; +import { + deduplicateLabel, + getAbsolutePosition, + type CanvasEdge, + type CanvasNode, + type NestableNode, +} from '@huabu/shared/canvas-engine'; + +const DESTINATION_GAP = 160; +const MOVABLE_TYPES = new Set( + CANVAS_NODE_TYPES.filter( + (type) => + type !== 'spacePreview' && + type !== 'canvasRef' && + type !== 'frameRef' && + type !== 'nodeRef', + ), +); + +export class SpaceMovePlanError extends Error { + constructor( + readonly code: 'missing-node' | 'not-movable' | 'invalid-hierarchy', + readonly nodeId: string, + ) { + super(`Cannot move node ${nodeId}: ${code}`); + this.name = 'SpaceMovePlanError'; + } +} + +export interface SpaceMovePlan { + commands: CanvasCommand[]; + deleteCommand: Extract; + rootIds: string[]; + movedIds: Set; + nodeIdMap: Map; + movedFrameCount: number; + movedThreadIds: string[]; + omittedBoundaryEdges: Array<{ + edgeId: string; + source: string; + target: string; + }>; + renamedNodes: Array<{ sourceNodeId: string; from: string; to: string }>; +} + +function nodeSize(node: CanvasNode): { width: number; height?: number } | null { + const width = node.style?.width; + const height = node.style?.height; + if (typeof width !== 'number') return null; + return { + width, + ...(typeof height === 'number' ? { height } : {}), + }; +} + +function assertAcyclic(nodes: readonly CanvasNode[]): void { + const byId = new Map(nodes.map((node) => [node.id, node])); + for (const node of nodes) { + const visited = new Set([node.id]); + let parentId = node.parentId; + while (parentId) { + if (visited.has(parentId)) { + throw new SpaceMovePlanError('invalid-hierarchy', node.id); + } + visited.add(parentId); + parentId = byId.get(parentId)?.parentId; + } + } +} + +function normalizedRoots( + selectedNodeIds: readonly string[], + byId: ReadonlyMap, +): string[] { + const selected = new Set(selectedNodeIds); + return [...selected].filter((nodeId) => { + let parentId = byId.get(nodeId)?.parentId; + while (parentId) { + const parent = byId.get(parentId); + if (!parent) break; + if (selected.has(parentId) && parent.type === 'frame') return false; + parentId = parent.parentId; + } + return true; + }); +} + +function expandedMoveIds( + rootIds: readonly string[], + nodes: readonly CanvasNode[], +): Set { + const children = new Map(); + for (const node of nodes) { + if (!node.parentId) continue; + const values = children.get(node.parentId) ?? []; + values.push(node.id); + children.set(node.parentId, values); + } + + const moved = new Set(rootIds); + const stack = rootIds.filter( + (nodeId) => nodes.find((node) => node.id === nodeId)?.type === 'frame', + ); + while (stack.length > 0) { + const parentId = stack.pop(); + if (!parentId) continue; + for (const childId of children.get(parentId) ?? []) { + if (moved.has(childId)) continue; + moved.add(childId); + if (nodes.find((node) => node.id === childId)?.type === 'frame') { + stack.push(childId); + } + } + } + return moved; +} + +export function buildSpaceMovePlan(input: { + sourceNodes: readonly CanvasNode[]; + sourceEdges: readonly CanvasEdge[]; + destinationNodes: readonly CanvasNode[]; + selectedNodeIds: readonly string[]; +}): SpaceMovePlan { + const { sourceNodes, sourceEdges, destinationNodes, selectedNodeIds } = input; + assertAcyclic(sourceNodes); + const byId = new Map(sourceNodes.map((node) => [node.id, node])); + for (const nodeId of new Set(selectedNodeIds)) { + const node = byId.get(nodeId); + if (!node) throw new SpaceMovePlanError('missing-node', nodeId); + if (!MOVABLE_TYPES.has(node.type as CanvasNodeType)) { + throw new SpaceMovePlanError('not-movable', nodeId); + } + } + + const rootIds = normalizedRoots(selectedNodeIds, byId); + const movedIds = expandedMoveIds(rootIds, sourceNodes); + for (const nodeId of movedIds) { + const node = byId.get(nodeId); + if (!node || !MOVABLE_TYPES.has(node.type as CanvasNodeType)) { + throw new SpaceMovePlanError('not-movable', nodeId); + } + } + + const nodeIdMap = new Map(); + for (const nodeId of movedIds) nodeIdMap.set(nodeId, createId('node')); + + const destinationRight = destinationNodes.reduce((right, node) => { + const position = getAbsolutePosition( + destinationNodes as NestableNode[], + node.id, + ); + const width = typeof node.style?.width === 'number' ? node.style.width : 0; + return position ? Math.max(right, position.x + width) : right; + }, -DESTINATION_GAP); + const rootPositions = rootIds.map((nodeId) => { + const position = getAbsolutePosition(sourceNodes as NestableNode[], nodeId); + if (!position) { + throw new SpaceMovePlanError('invalid-hierarchy', nodeId); + } + return position; + }); + const sourceLeft = Math.min(...rootPositions.map((position) => position.x)); + const sourceTop = Math.min(...rootPositions.map((position) => position.y)); + const offset = { + x: destinationRight + DESTINATION_GAP - sourceLeft, + y: -sourceTop, + }; + + const labels = destinationNodes.map((node) => + typeof node.data?.label === 'string' ? node.data.label : undefined, + ); + const renamedNodes: SpaceMovePlan['renamedNodes'] = []; + const creates: Extract['nodes'] = []; + const movedThreadIds: string[] = []; + + for (const node of sourceNodes) { + if (!movedIds.has(node.id)) continue; + const nodeId = nodeIdMap.get(node.id); + if (!nodeId) continue; + const nodeType = node.type as CanvasNodeType; + const data = structuredClone(node.data ?? {}); + const originalLabel = + typeof data.label === 'string' ? data.label.trim() : ''; + const label = deduplicateLabel(originalLabel || nodeType, labels); + labels.push(label); + data.label = label; + if (label !== originalLabel) { + renamedNodes.push({ + sourceNodeId: node.id, + from: originalLabel, + to: label, + }); + } + if (nodeType === 'sketch' && Array.isArray(data.strokes)) { + data.strokes = data.strokes.map((stroke: Record) => ({ + ...stroke, + id: createId('stroke'), + })); + } + if ( + nodeType === 'question' && + typeof data.threadId === 'string' && + data.threadId + ) { + movedThreadIds.push(data.threadId); + } + + const remappedParent = + node.parentId && movedIds.has(node.parentId) + ? nodeIdMap.get(node.parentId) + : undefined; + const absolute = remappedParent + ? node.position + : getAbsolutePosition(sourceNodes as NestableNode[], node.id); + if (!absolute) { + throw new SpaceMovePlanError('invalid-hierarchy', node.id); + } + creates.push({ + id: nodeId, + nodeType, + data, + position: remappedParent + ? { ...absolute } + : { x: absolute.x + offset.x, y: absolute.y + offset.y }, + ...(nodeSize(node) ? { size: nodeSize(node) ?? undefined } : {}), + ...(remappedParent ? { parentId: remappedParent } : {}), + }); + } + + const edges: Extract['edges'] = []; + const omittedBoundaryEdges: SpaceMovePlan['omittedBoundaryEdges'] = []; + for (const edge of sourceEdges) { + const sourceMoved = movedIds.has(edge.source); + const targetMoved = movedIds.has(edge.target); + if (sourceMoved && targetMoved) { + const source = nodeIdMap.get(edge.source); + const target = nodeIdMap.get(edge.target); + if (!source || !target) continue; + const style = (edge.data as { edgeStyle?: EdgeStyle } | undefined) + ?.edgeStyle; + edges.push({ + id: createId('edge') as CanvasEdgeId, + source, + target, + ...(style ? { style: structuredClone(style) } : {}), + }); + } else if (sourceMoved !== targetMoved) { + omittedBoundaryEdges.push({ + edgeId: edge.id, + source: edge.source, + target: edge.target, + }); + } + } + + const commands: CanvasCommand[] = [{ type: 'CREATE_NODES', nodes: creates }]; + if (edges.length > 0) commands.push({ type: 'CONNECT_NODES', edges }); + + return { + commands, + deleteCommand: { + type: 'DELETE_NODES', + nodeIds: rootIds.map((nodeId) => nodeId as CanvasNodeId), + }, + rootIds, + movedIds, + nodeIdMap, + movedFrameCount: [...movedIds].filter( + (nodeId) => byId.get(nodeId)?.type === 'frame', + ).length, + movedThreadIds: [...new Set(movedThreadIds)].sort(), + omittedBoundaryEdges, + renamedNodes, + }; +} diff --git a/apps/server/src/modules/canvas/space-move.service.test.ts b/apps/server/src/modules/canvas/space-move.service.test.ts new file mode 100644 index 000000000..a9015e3ed --- /dev/null +++ b/apps/server/src/modules/canvas/space-move.service.test.ts @@ -0,0 +1,167 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +import { executeOnServer } from './canvas-executor.js'; +import { moveCanvasSelection } from './space-move.service.js'; +import { createCanvas } from '../storage/compatibility/canvas.js'; +import { resetStorageCache, space } from '../storage/index.js'; +import { setWorkspacePath } from '../workspace.js'; + +import type { SpaceMoveError } from './space-move.service.js'; + +let workspace: string; + +beforeEach(() => { + workspace = mkdtempSync(join(tmpdir(), 'huabu-space-move-')); + setWorkspacePath(workspace); + resetStorageCache(); +}); + +afterEach(() => { + resetStorageCache(); + rmSync(workspace, { recursive: true, force: true }); +}); + +async function seedSource() { + createCanvas('source', 'Source'); + createCanvas('destination', 'Destination'); + return executeOnServer({ + canvasId: 'source', + originator: { source: 'ui' }, + commands: [ + { + type: 'CREATE_NODES', + nodes: [ + { + id: 'node-frame', + nodeType: 'frame', + data: { label: 'Frame' }, + position: { x: 20, y: 40 }, + size: { width: 400, height: 300 }, + }, + { + id: 'node-child', + nodeType: 'note', + data: { label: 'Note', content: 'Hello' }, + position: { x: 30, y: 50 }, + parentId: 'node-frame', + size: { width: 200, height: 100 }, + }, + { + id: 'node-outside', + nodeType: 'note', + data: { label: 'Outside', content: 'Outside' }, + position: { x: 600, y: 40 }, + size: { width: 200, height: 100 }, + }, + ], + }, + { + type: 'CONNECT_NODES', + edges: [ + { + id: 'edge-boundary', + source: 'node-child', + target: 'node-outside', + }, + ], + }, + ], + }); +} + +describe('moveCanvasSelection', () => { + it('moves a Frame subtree and reports omitted boundary edges', async () => { + const seeded = await seedSource(); + + const result = await moveCanvasSelection('source', { + selectedNodeIds: ['node-frame'], + destinationCanvasId: 'destination', + expectedSourceVersion: seeded.toVersion, + }); + + expect(result).toMatchObject({ + movedNodeCount: 2, + movedFrameCount: 1, + preservedEdgeCount: 0, + movedConversationCount: 0, + omittedBoundaryEdges: [ + { + edgeId: 'edge-boundary', + source: 'node-child', + target: 'node-outside', + }, + ], + }); + const source = await space('source').read(); + const destination = await space('destination').read(); + expect(source?.state.nodes).toHaveLength(1); + expect(destination?.state.nodes).toHaveLength(2); + const child = ( + destination?.state.nodes as Array<{ parentId?: string }> + ).find((node) => node.parentId); + expect(child?.parentId).toBe(result.roots[0]?.destinationNodeId); + }); + + it('rejects a stale source version without changing either Space', async () => { + await seedSource(); + + await expect( + moveCanvasSelection('source', { + selectedNodeIds: ['node-frame'], + destinationCanvasId: 'destination', + expectedSourceVersion: 0, + }), + ).rejects.toMatchObject({ + code: 'MOVE_SOURCE_STALE', + } satisfies Partial); + expect((await space('source').read())?.state.nodes).toHaveLength(3); + expect((await space('destination').read())?.state.nodes).toHaveLength(0); + }); + + it('copies and rewrites required artifacts before deleting the source node', async () => { + createCanvas('source', 'Source'); + createCanvas('destination', 'Destination'); + await space('source').blobs.put('artifact-old.png', Buffer.from('image')); + const seeded = await executeOnServer({ + canvasId: 'source', + originator: { source: 'ui' }, + commands: [ + { + type: 'CREATE_NODES', + nodes: [ + { + id: 'node-image', + nodeType: 'image', + data: { label: 'Image', src: 'artifact-old.png' }, + position: { x: 0, y: 0 }, + size: { width: 100, height: 100 }, + }, + ], + }, + ], + }); + + await moveCanvasSelection('source', { + selectedNodeIds: ['node-image'], + destinationCanvasId: 'destination', + expectedSourceVersion: seeded.toVersion, + }); + + const [record] = [...(await space('destination').nodes.list()).values()]; + expect(record?.record.src).toMatch(/^artifact-.+\.png$/); + expect(record?.record.src).not.toBe('artifact-old.png'); + expect( + await space('destination').blobs.read(record?.record.src ?? ''), + ).toEqual(Buffer.from('image')); + expect(await space('source').blobs.read('artifact-old.png')).toEqual( + Buffer.from('image'), + ); + }); +}); diff --git a/apps/server/src/modules/canvas/space-move.service.ts b/apps/server/src/modules/canvas/space-move.service.ts new file mode 100644 index 000000000..fe091cd7f --- /dev/null +++ b/apps/server/src/modules/canvas/space-move.service.ts @@ -0,0 +1,456 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import path from 'node:path'; + +import { + ARTIFACT_DATA_FIELDS, + collectMarkdownArtifactRefs, + createId, + markdownArtifactFields, + parseArtifactRef, + rewriteMarkdownArtifactRefs, + type MoveSelectionBody, + type MoveSelectionErrorCode, + type MoveSelectionResponse, +} from '@huabu/shared'; +import { + invertDeltas, + type CanvasEdge, + type CanvasNode, +} from '@huabu/shared/canvas-engine'; + +import { + applyDeltasOnServerAlreadyLocked, + executeOnServerAlreadyLocked, + hydrateCanvasNodes, + type ExecuteOnServerOutput, +} from './canvas-executor.js'; +import { publishCanvasUpdate } from './canvas-sync.js'; +import { + buildSpaceMovePlan, + SpaceMovePlanError, + type SpaceMovePlan, +} from './space-move-plan.js'; +import { withCanvasMutexes } from './write-coordinator.js'; +import { buildReachbackEnv } from '../agent/acp/reachback-env.js'; +import { + agenetes, + EXTERNAL_DRIVER_KIND, + INTERNAL_DRIVER_KIND, +} from '../agent/agenetes/drivers.js'; +import { agentThreadService } from '../agent/agent-thread.service.js'; +import { acquireAgentTurn } from '../agent/turn-lease.js'; +import { getBlobStore, isWorldCanvasId, space } from '../storage/index.js'; +import { canvasAcpNamespace } from '../workspace/paths.js'; +import { acquireWorkspaceOperationLease } from '../workspace.js'; + +import type { WorkloadSpec } from '@agenetes/protocol'; + +export class SpaceMoveError extends Error { + constructor( + readonly code: MoveSelectionErrorCode, + message: string, + readonly statusCode = 409, + ) { + super(message); + this.name = 'SpaceMoveError'; + } +} + +function movedWorkloadSpec( + source: WorkloadSpec, + destinationCanvasId: string, +): WorkloadSpec { + const cloned = + source.spec && typeof source.spec === 'object' + ? structuredClone(source.spec as Record) + : source.spec; + if (cloned && typeof cloned === 'object') { + const spec = cloned as Record; + if (source.kind === INTERNAL_DRIVER_KIND) { + const hostContext = + spec.hostContext && typeof spec.hostContext === 'object' + ? spec.hostContext + : {}; + spec.hostContext = { ...hostContext, canvasId: destinationCanvasId }; + } else if (source.kind === EXTERNAL_DRIVER_KIND) { + spec.env = { + ...(spec.env && typeof spec.env === 'object' ? spec.env : {}), + ...buildReachbackEnv(source.threadId, destinationCanvasId), + }; + } + } + return { + ...source, + namespace: canvasAcpNamespace(destinationCanvasId), + spec: cloned, + }; +} + +async function cloneArtifacts( + sourceCanvasId: string, + destinationCanvasId: string, + nodes: readonly CanvasNode[], +): Promise { + const blobs = getBlobStore(); + const destination = blobs.scope({ + kind: 'canvas', + canvasId: destinationCanvasId, + }); + const cloned = new Map(); + + const cloneRef = async (raw: unknown): Promise => { + const ref = parseArtifactRef(raw); + if (!ref) return undefined; + const owner = ref.canvasId ?? sourceCanvasId; + if (owner === destinationCanvasId) return ref.key; + const cacheKey = `${owner}/${ref.key}`; + const existing = cloned.get(cacheKey); + if (existing) return existing; + const body = await blobs + .scope({ kind: 'canvas', canvasId: owner }) + .read(ref.key); + if (!body) { + throw new SpaceMoveError( + 'MOVE_ARTIFACT_MISSING', + `Required artifact is missing: ${ref.key}`, + ); + } + const key = `${createId('artifact')}${path.extname(ref.key)}`; + await destination.put(key, body); + cloned.set(cacheKey, key); + return key; + }; + + return Promise.all( + nodes.map(async (node) => { + const data = structuredClone( + (node.data ?? {}) as Record, + ); + for (const field of ARTIFACT_DATA_FIELDS) { + const key = await cloneRef(data[field]); + if (key) data[field] = key; + } + for (const field of markdownArtifactFields(data)) { + const markdown = data[field]; + if (typeof markdown !== 'string') continue; + const rewrites = new Map(); + await Promise.all( + collectMarkdownArtifactRefs(markdown).map(async (raw) => { + const key = await cloneRef(raw); + if (key) rewrites.set(raw, key); + }), + ); + data[field] = rewriteMarkdownArtifactRefs(markdown, (raw) => + rewrites.get(raw), + ); + } + return { ...node, data }; + }), + ); +} + +function publishExecution(output: ExecuteOnServerOutput): void { + if (output.toVersion === output.fromVersion) return; + publishCanvasUpdate(output.canvasId, { + type: 'update', + data: { + fromVersion: output.fromVersion, + toVersion: output.toVersion, + deltas: output.deltas, + pendingEffects: output.pendingEffects, + }, + }); +} + +export async function moveCanvasSelection( + sourceCanvasId: string, + input: MoveSelectionBody, +): Promise { + if (sourceCanvasId === input.destinationCanvasId) { + throw new SpaceMoveError( + 'MOVE_DESTINATION_SAME_AS_SOURCE', + 'Source and destination Spaces must be different', + ); + } + if ( + isWorldCanvasId(sourceCanvasId) || + isWorldCanvasId(input.destinationCanvasId) + ) { + throw new SpaceMoveError( + 'MOVE_WORLD_NOT_ALLOWED', + 'World cannot participate in a move', + 403, + ); + } + + const workspaceLease = acquireWorkspaceOperationLease(); + try { + return await withCanvasMutexes( + [sourceCanvasId, input.destinationCanvasId], + async () => { + const sourceHandle = space(sourceCanvasId); + const destinationHandle = space(input.destinationCanvasId); + const [source, destination, sourceRecords, destinationRecords] = + await Promise.all([ + sourceHandle.read(), + destinationHandle.read(), + sourceHandle.nodes.list(), + destinationHandle.nodes.list(), + ]); + if (!source) { + throw new SpaceMoveError( + 'MOVE_SOURCE_NODE_MISSING', + 'Source Space was not found', + 404, + ); + } + if (!destination) { + throw new SpaceMoveError( + 'MOVE_DESTINATION_MISSING', + 'Destination Space was not found', + 404, + ); + } + if (source.version !== input.expectedSourceVersion) { + throw new SpaceMoveError( + 'MOVE_SOURCE_STALE', + 'Source Space changed before the move started', + ); + } + + const hydratedSource = hydrateCanvasNodes( + sourceRecords, + source.state.nodes as CanvasNode[], + ); + const hydratedDestination = hydrateCanvasNodes( + destinationRecords, + destination.state.nodes as CanvasNode[], + ); + let plan: SpaceMovePlan; + try { + plan = buildSpaceMovePlan({ + sourceNodes: hydratedSource, + sourceEdges: (source.state.edges ?? []) as CanvasEdge[], + destinationNodes: hydratedDestination, + selectedNodeIds: input.selectedNodeIds, + }); + } catch (error) { + if (error instanceof SpaceMovePlanError) { + const code = + error.code === 'missing-node' + ? 'MOVE_SOURCE_NODE_MISSING' + : 'MOVE_NODE_NOT_MOVABLE'; + throw new SpaceMoveError(code, error.message); + } + throw error; + } + + const taskSnapshot = await sourceHandle.tasks.read(); + if ( + taskSnapshot.runs.some( + (run) => + (run.rootNodeId && plan.movedIds.has(run.rootNodeId)) || + (run.rootThreadId && + plan.movedThreadIds.includes(run.rootThreadId)), + ) + ) { + throw new SpaceMoveError( + 'MOVE_AGENT_TASK_OWNED', + 'The selection contains an Agent owned by a Task Run', + ); + } + const releaseThreads: Array<() => void> = []; + const threadMoves: Array<{ + threadId: string; + sourceSpec: WorkloadSpec; + targetSpec: WorkloadSpec; + }> = []; + try { + for (const threadId of plan.movedThreadIds) { + if (agentThreadService.isActive(threadId, sourceCanvasId)) { + throw new SpaceMoveError( + 'MOVE_AGENT_RUNNING', + `Agent conversation ${threadId} is running`, + ); + } + const release = acquireAgentTurn(threadId); + if (!release) { + throw new SpaceMoveError( + 'MOVE_AGENT_RUNNING', + `Agent conversation ${threadId} is busy`, + ); + } + releaseThreads.push(release); + if ((await sourceHandle.changes.read(threadId)).length > 0) { + throw new SpaceMoveError( + 'MOVE_AGENT_PENDING_CHANGES', + `Agent conversation ${threadId} has pending changes`, + ); + } + const namespace = canvasAcpNamespace(sourceCanvasId); + const record = agenetes.record(namespace, threadId); + if (!record) { + throw new SpaceMoveError( + 'MOVE_AGENT_HISTORY_INVALID', + `Agent conversation ${threadId} has no durable record`, + ); + } + threadMoves.push({ + threadId, + sourceSpec: record.spec, + targetSpec: movedWorkloadSpec( + record.spec, + input.destinationCanvasId, + ), + }); + } + + const movedNodes = hydratedSource.filter((node) => + plan.movedIds.has(node.id), + ); + const rewrittenNodes = await cloneArtifacts( + sourceCanvasId, + input.destinationCanvasId, + movedNodes, + ); + const rewrittenById = new Map( + rewrittenNodes.map((node) => [node.id, node]), + ); + plan = buildSpaceMovePlan({ + sourceNodes: hydratedSource.map( + (node) => rewrittenById.get(node.id) ?? node, + ), + sourceEdges: (source.state.edges ?? []) as CanvasEdge[], + destinationNodes: hydratedDestination, + selectedNodeIds: input.selectedNodeIds, + }); + + const destinationWrite = await executeOnServerAlreadyLocked({ + canvasId: input.destinationCanvasId, + commands: plan.commands, + originator: { source: 'system' }, + publish: false, + }); + if ( + destinationWrite.results.some((result) => !result.applied) || + destinationWrite.toVersion === destinationWrite.fromVersion + ) { + throw new SpaceMoveError( + 'MOVE_DESTINATION_CONFLICT', + 'Destination rejected the moved nodes', + ); + } + + const completedThreads: typeof threadMoves = []; + try { + for (const move of threadMoves) { + agenetes.rehome( + { + namespace: canvasAcpNamespace(sourceCanvasId), + threadId: move.threadId, + }, + move.targetSpec, + ); + completedThreads.push(move); + } + const sourceWrite = await executeOnServerAlreadyLocked({ + canvasId: sourceCanvasId, + commands: [plan.deleteCommand], + originator: { source: 'system' }, + publish: false, + }); + if (sourceWrite.results.some((result) => !result.applied)) { + throw new SpaceMoveError( + 'MOVE_SOURCE_STALE', + 'Source rejected deletion of the moved nodes', + ); + } + publishExecution(destinationWrite); + publishExecution(sourceWrite); + const create = plan.commands.find( + (command) => command.type === 'CREATE_NODES', + ); + return { + transferId: createId('transfer'), + destination: { + canvasId: input.destinationCanvasId, + title: destination.title, + }, + sourceVersion: sourceWrite.toVersion, + destinationVersion: destinationWrite.toVersion, + roots: + create?.type === 'CREATE_NODES' + ? plan.rootIds.flatMap((sourceNodeId) => { + const destinationNodeId = + plan.nodeIdMap.get(sourceNodeId); + if (!destinationNodeId) return []; + const node = create.nodes.find( + (candidate) => candidate.id === destinationNodeId, + ); + return [ + { + sourceNodeId, + destinationNodeId, + label: + typeof node?.data?.label === 'string' + ? node.data.label + : '', + }, + ]; + }) + : [], + movedNodeCount: plan.movedIds.size, + movedFrameCount: plan.movedFrameCount, + preservedEdgeCount: + plan.commands.find( + (command) => command.type === 'CONNECT_NODES', + )?.type === 'CONNECT_NODES' + ? ( + plan.commands.find( + (command) => command.type === 'CONNECT_NODES', + ) as Extract< + (typeof plan.commands)[number], + { type: 'CONNECT_NODES' } + > + ).edges.length + : 0, + omittedBoundaryEdges: plan.omittedBoundaryEdges, + renamedNodes: plan.renamedNodes, + movedConversationCount: threadMoves.length, + }; + } catch (error) { + try { + for (const move of completedThreads.reverse()) { + agenetes.rehome( + { + namespace: canvasAcpNamespace(input.destinationCanvasId), + threadId: move.threadId, + }, + move.sourceSpec, + ); + } + await applyDeltasOnServerAlreadyLocked({ + canvasId: input.destinationCanvasId, + deltas: invertDeltas(destinationWrite.deltas), + originator: { source: 'system' }, + }); + } catch (compensationError) { + throw new SpaceMoveError( + 'MOVE_OUTCOME_UNKNOWN', + `Move failed and compensation also failed: ${String(compensationError)}`, + 500, + ); + } + throw error; + } + } finally { + for (const release of releaseThreads.reverse()) release(); + } + }, + ); + } finally { + workspaceLease.release(); + } +} diff --git a/apps/server/src/modules/canvas/write-coordinator.ts b/apps/server/src/modules/canvas/write-coordinator.ts index 68d9968c6..7dc6e7f59 100644 --- a/apps/server/src/modules/canvas/write-coordinator.ts +++ b/apps/server/src/modules/canvas/write-coordinator.ts @@ -36,6 +36,27 @@ export async function withCanvasMutex( } } +/** + * Acquire several Canvas mutexes in stable order. + * + * Sorting prevents two cross-Canvas operations from waiting on each other + * while each holds the opposite first lock. + */ +export async function withCanvasMutexes( + canvasIds: readonly string[], + task: () => Promise, +): Promise { + const ordered = [...new Set(canvasIds)].sort(); + + const acquire = async (index: number): Promise => { + const canvasId = ordered[index]; + if (!canvasId) return task(); + return withCanvasMutex(canvasId, () => acquire(index + 1)); + }; + + return acquire(0); +} + type NodeRejection = Exclude< Extract, { reason: 'revision-conflict' | 'write-suppressed' } diff --git a/apps/web/src/api/_routes.ts b/apps/web/src/api/_routes.ts index 627f64816..972dd41d1 100644 --- a/apps/web/src/api/_routes.ts +++ b/apps/web/src/api/_routes.ts @@ -54,6 +54,8 @@ export const routes = { canvasImport: '/canvas/import', canvas: (canvasId: string) => `/canvas/${enc(canvasId)}`, canvasExecute: (canvasId: string) => `/canvas/${enc(canvasId)}/execute`, + canvasMoveSelection: (canvasId: string) => + `/canvas/${enc(canvasId)}/move-selection`, canvasReferences: (canvasId: string) => `/canvas/${enc(canvasId)}/references`, canvasPreviewScene: (canvasId: string) => `/canvas/${enc(canvasId)}/preview-scene`, diff --git a/apps/web/src/api/canvas.ts b/apps/web/src/api/canvas.ts index f1373b40a..471942b7b 100644 --- a/apps/web/src/api/canvas.ts +++ b/apps/web/src/api/canvas.ts @@ -27,6 +27,8 @@ import type { RevealNodesFolderResponse, PostCanvasExecuteRequest, PostCanvasExecuteResponse, + MoveSelectionBody, + MoveSelectionResponse, } from '@huabu/shared'; /** @@ -166,6 +168,17 @@ export async function postCanvasExecute( }); } +export async function moveCanvasSelection( + canvasId: string, + request: MoveSelectionBody, +): Promise { + return apiFetch(routes.canvasMoveSelection(canvasId), { + method: 'POST', + json: request, + fallbackMessage: 'Failed to move selection', + }); +} + export async function putCanvas( canvasId: string, request: PutCanvasRequest, diff --git a/apps/web/src/components/Panels/Canvas/Canvas.tsx b/apps/web/src/components/Panels/Canvas/Canvas.tsx index 0020e5e13..f76a32826 100644 --- a/apps/web/src/components/Panels/Canvas/Canvas.tsx +++ b/apps/web/src/components/Panels/Canvas/Canvas.tsx @@ -106,6 +106,7 @@ import { import { EdgeStyleToolbar } from './FloatingToolbars/EdgeStyleToolbar.tsx'; import { MultiSelectToolbar } from './FloatingToolbars/MultiSelectToolbar.tsx'; import { StrokeSelectionToolbar } from './FloatingToolbars/StrokeSelectionToolbar.tsx'; +import { MoveSelectionModal } from './MoveSelectionModal.tsx'; import { MultiSelectResizer } from './MultiSelectResizer.tsx'; import { SelectionOutlines } from './SelectionOutlines.tsx'; import { SnapGuidesOverlay } from './SnapGuidesOverlay.tsx'; @@ -1628,6 +1629,7 @@ export const Canvas: React.FC = ({ {!isBoxSelecting && } {!isBoxSelecting && } {!isBoxSelecting && } + { const setNoteHeightMode = useCanvasStore((s) => s.setNoteHeightMode); const beginGesture = useCanvasStore((s) => s.beginGesture); const deleteNodes = useCanvasStore((s) => s.deleteNodes); + const setMoveSelectionDialogOpen = useCanvasStore( + (s) => s.setMoveSelectionDialogOpen, + ); const isNotMouse = useIsNotMouse(); const selectedNodes = useMemo( @@ -66,6 +69,11 @@ export const MultiSelectToolbar = () => { const hasManagedSizeSelection = selectedNodes.some( (node) => node.type === 'canvasRef' || node.type === 'frameRef', ); + const hasNonMovableSelection = selectedNodes.some((node) => + ['spacePreview', 'canvasRef', 'frameRef', 'nodeRef'].includes( + node.type ?? '', + ), + ); // Edges whose endpoints are both in the node selection participate in // multi-selection styling. This matches the derived edge highlighting in @@ -325,6 +333,18 @@ export const MultiSelectToolbar = () => { /> )} + {!hasNonMovableSelection && ( + <> + + setMoveSelectionDialogOpen(true)} + > + + + + )} + {/* Non-mouse only: mouse users have keyboard Delete / Backspace. */} {isNotMouse && ( <> diff --git a/apps/web/src/components/Panels/Canvas/FloatingToolbars/NodeFloatingToolbar.tsx b/apps/web/src/components/Panels/Canvas/FloatingToolbars/NodeFloatingToolbar.tsx index a3144c335..c0f90d96d 100644 --- a/apps/web/src/components/Panels/Canvas/FloatingToolbars/NodeFloatingToolbar.tsx +++ b/apps/web/src/components/Panels/Canvas/FloatingToolbars/NodeFloatingToolbar.tsx @@ -2,7 +2,7 @@ // Licensed under the MIT license. import { useInternalNode } from '@xyflow/react'; -import { Trash2 } from 'lucide-react'; +import { MoveRight, Trash2 } from 'lucide-react'; import { memo, useCallback, useMemo, type ReactNode } from 'react'; import { useTranslation } from 'react-i18next'; @@ -99,6 +99,9 @@ export const NodeFloatingToolbar = memo( const updateNodeData = useCanvasStore((s) => s.updateNodeData); const convertNodeType = useCanvasStore((s) => s.convertNodeType); const deleteNodes = useCanvasStore((s) => s.deleteNodes); + const setMoveSelectionDialogOpen = useCanvasStore( + (s) => s.setMoveSelectionDialogOpen, + ); const setNodeGeometry = useCanvasStore((s) => s.setNodeGeometry); const setNoteHeightMode = useCanvasStore((s) => s.setNoteHeightMode); const isOpenInPreview = usePreviewWorkspaceStore((s) => @@ -384,6 +387,20 @@ export const NodeFloatingToolbar = memo( )} + {!['spacePreview', 'canvasRef', 'frameRef', 'nodeRef'].includes( + type, + ) && ( + <> + + setMoveSelectionDialogOpen(true)} + > + + + + )} + {/* Non-mouse only: mouse users have keyboard Delete / Backspace. */} {isNotMouse && ( <> diff --git a/apps/web/src/components/Panels/Canvas/MoveSelectionModal.tsx b/apps/web/src/components/Panels/Canvas/MoveSelectionModal.tsx new file mode 100644 index 000000000..f8826552d --- /dev/null +++ b/apps/web/src/components/Panels/Canvas/MoveSelectionModal.tsx @@ -0,0 +1,167 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import { useEffect, useMemo, useState } from 'react'; +import { useTranslation } from 'react-i18next'; +import { useNavigate } from 'react-router-dom'; + +import { listCanvases, moveCanvasSelection } from '@/api/canvas'; +import { Button } from '@/components/Common/Button'; +import { Modal } from '@/components/Common/Modal'; +import { Select, type SelectOption } from '@/components/Common/Select'; +import { toast } from '@/components/Common/Toast'; +import useCanvasStore, { drainPendingSaves } from '@/store/canvasStore'; + +export function MoveSelectionModal() { + const { t } = useTranslation(); + const navigate = useNavigate(); + const isOpen = useCanvasStore((state) => state.moveSelectionDialogOpen); + const setOpen = useCanvasStore((state) => state.setMoveSelectionDialogOpen); + const canvasId = useCanvasStore((state) => state.canvasId); + const selectedNodes = useCanvasStore((state) => + state.nodes.filter((node) => node.selected), + ); + const [options, setOptions] = useState[]>([]); + const [destinationCanvasId, setDestinationCanvasId] = useState(''); + const [loading, setLoading] = useState(false); + const [submitting, setSubmitting] = useState(false); + const [loadError, setLoadError] = useState(false); + + const selectedNodeIds = useMemo( + () => selectedNodes.map((node) => node.id), + [selectedNodes], + ); + + useEffect(() => { + if (!isOpen) return; + let active = true; + setLoading(true); + setLoadError(false); + void listCanvases() + .then(({ canvases }) => { + if (!active) return; + const next = canvases + .filter((canvas) => canvas.canvasId !== canvasId) + .map((canvas) => ({ + value: canvas.canvasId, + label: canvas.title || t('moveSelection.untitledSpace'), + })); + setOptions(next); + setDestinationCanvasId((current) => + next.some((option) => option.value === current) + ? current + : (next[0]?.value ?? ''), + ); + }) + .catch(() => { + if (active) setLoadError(true); + }) + .finally(() => { + if (active) setLoading(false); + }); + return () => { + active = false; + }; + }, [canvasId, isOpen, t]); + + const close = () => { + if (!submitting) setOpen(false); + }; + + const submit = async () => { + if (!canvasId || !destinationCanvasId || selectedNodeIds.length === 0) { + return; + } + setSubmitting(true); + try { + await drainPendingSaves(); + const expectedSourceVersion = useCanvasStore.getState().version; + const result = await moveCanvasSelection(canvasId, { + selectedNodeIds, + destinationCanvasId, + expectedSourceVersion, + }); + setOpen(false); + toast( + t('moveSelection.success', { + count: result.movedNodeCount, + conversations: result.movedConversationCount, + }), + { + tone: 'success', + action: { + label: t('moveSelection.openDestination'), + onClick: () => navigate(`/canvas/${result.destination.canvasId}`), + }, + }, + ); + } catch (error) { + toast( + error instanceof Error ? error.message : t('moveSelection.failed'), + { tone: 'danger', duration: 0 }, + ); + } finally { + setSubmitting(false); + } + }; + + return ( + + + + + } + > +
+ {loadError ? ( +

+ {t('moveSelection.targetsUnavailable')} +

+ ) : options.length === 0 && !loading ? ( +

+ {t('moveSelection.noTargets')} +

+ ) : ( + + {destinationKind === 'new' ? ( + setNewSpaceTitle(event.target.value)} + disabled={submitting} + placeholder={t('moveSelection.newSpaceName')} + aria-label={t('moveSelection.newSpaceName')} + autoFocus + /> + ) : loadError ? (

{t('moveSelection.targetsUnavailable')}

@@ -147,7 +186,7 @@ export function MoveSelectionModal() {

) : ( setCreateSourcePreview(event.target.checked)} + disabled={submitting} + /> + {t('moveSelection.createSourcePreview')} +
); diff --git a/apps/web/src/i18n/resources/en/common.json b/apps/web/src/i18n/resources/en/common.json index 791d33c76..5da0d71c2 100644 --- a/apps/web/src/i18n/resources/en/common.json +++ b/apps/web/src/i18n/resources/en/common.json @@ -41,7 +41,7 @@ "targetsUnavailable": "Destination Spaces could not be loaded.", "noTargets": "Create another Space before moving this selection.", "boundaryNotice": "Connections to nodes outside the selection will be removed.", - "previewNotice": "A preview linking to the destination will remain in the original location." + "createSourcePreview": "Leave a preview linking to the destination in the original location." }, "canvasControls": { "resetZoom": "Reset zoom to 100%", diff --git a/apps/web/src/i18n/resources/zh-CN/common.json b/apps/web/src/i18n/resources/zh-CN/common.json index ad5dda1fe..ebd51473c 100644 --- a/apps/web/src/i18n/resources/zh-CN/common.json +++ b/apps/web/src/i18n/resources/zh-CN/common.json @@ -41,7 +41,7 @@ "targetsUnavailable": "无法加载目标 Space。", "noTargets": "请先创建另一个 Space，再移动所选内容。", "boundaryNotice": "与所选内容之外节点的连接将被移除。", - "previewNotice": "原位置将保留一个指向目标 Space 的预览。" + "createSourcePreview": "在原位置保留一个指向目标 Space 的预览。" }, "canvasControls": { "resetZoom": "重置缩放至 100%", diff --git a/docs/architecture/canvas-command-architecture.md b/docs/architecture/canvas-command-architecture.md index 598bb8684..4a9c77942 100644 --- a/docs/architecture/canvas-command-architecture.md +++ b/docs/architecture/canvas-command-architecture.md @@ -240,7 +240,7 @@ Before the shared engine applies an agent batch, `importForeignNodeSources` norm Same engine runs both sides; the only authority is the server. `POST /api/canvas/:canvasId/execute` is the shared entry, guarded by a per-canvas mutex (headless executor, M2). -Cross-Space Move is the bounded exception that coordinates two otherwise independent executor batches. `SpaceMoveService` acquires both Canvas mutexes in lexical order, uses `executeOnServerAlreadyLocked()` for destination creation and the source batch, and withholds both sync publications until the complete move succeeds. The source batch deletes the moved roots and creates one `spacePreview` breadcrumb pointing to the destination; its absolute position and clamped dimensions derive from the authoritative moved-set bounds. Determinate failure applies any source and destination inverse deltas through `applyDeltasOnServerAlreadyLocked()` while the same locks remain held. The service still expresses topology changes only as ordinary `CREATE_NODES`, `CONNECT_NODES`, and `DELETE_NODES` commands; the coordinator owns selection expansion, optional destination lifecycle, artifact transfer, Agent rehome, ordering, and compensation. +Cross-Space Move is the bounded exception that coordinates two otherwise independent executor batches. `SpaceMoveService` acquires both Canvas mutexes in lexical order, uses `executeOnServerAlreadyLocked()` for destination creation and the source batch, and withholds both sync publications until the complete move succeeds. The source batch always deletes the moved roots and optionally creates one `spacePreview` breadcrumb pointing to the destination; when requested, its absolute position and clamped dimensions derive from the authoritative moved-set bounds. Determinate failure applies any source and destination inverse deltas through `applyDeltasOnServerAlreadyLocked()` while the same locks remain held. The service still expresses topology changes only as ordinary `CREATE_NODES`, `CONNECT_NODES`, and `DELETE_NODES` commands; the coordinator owns selection expansion, optional destination lifecycle, artifact transfer, Agent rehome, ordering, and compensation. World `canvasRef` Portals add a host-level ownership policy before shared-engine execution. Only system reconciliation may create them; UI and agent batches cannot repoint them, manually resize them, or delete a Portal whose target is still a live Space. A broken Portal remains removable. Movement and ordinary Container parenting still use the same shared geometry and `SET_NODE_PARENT` semantics as other Canvas nodes. diff --git a/docs/architecture/canvas-storage.md b/docs/architecture/canvas-storage.md index d50a8f8b6..a70e9e31d 100644 --- a/docs/architecture/canvas-storage.md +++ b/docs/architecture/canvas-storage.md @@ -100,7 +100,7 @@ Canvas persistence DTOs and the write coordinator live under `modules/canvas/`; Space deletion is serialized against composed blob puts by a writer-preferring admission coordinator and holds an active-Workspace lease across blob cleanup and structured destruction. `beginDelete()` acquires the exclusive session before blob I/O; `finish()` removes structured state, while `abort()` releases the fence without doing so. Blobs are swept before structure so a failed sweep can be retried while the Space record still names them. Puts already admitted may finish; a put queued behind a successful deletion rechecks existence and fails without recreating blobs, while a failed blob sweep leaves the record available for retry. Mutations through existing Space handles and repositories reject while deletion is active or queued; reads remain available for cleanup. Residual direct-filesystem capabilities such as ZIP import, RFS upload/delete, and external-note claim are outside this repository fence and remain blockers for a non-Disk profile. -Cross-Space Move is an application-level compensated operation, not a new storage transaction. It holds an active-Workspace lease and both Canvas write mutexes, optionally creates a named destination Space, copies every referenced Blob to a fresh destination key before structural mutation, commits the destination Space, then replaces the source selection with a `spacePreview` breadcrumb in one source executor batch. A determinate later failure applies inverse source and destination deltas; when the destination was created by the request, failure also deletes that complete Space and its blobs. Failure to remove the new destination is surfaced explicitly rather than reported as an ordinary rejected move. BlobScope intentionally has no per-key deletion, so a failed move into an existing Space may leave freshly copied unreferenced blobs until that Space is deleted; referenced source blobs are retained because the source Space may still contain other references. The guarantee remains all-or-compensated while the Server continues running and receives determinate backend outcomes, matching the existing `SpaceHandle.write()` boundary. +Cross-Space Move is an application-level compensated operation, not a new storage transaction. It holds an active-Workspace lease and both Canvas write mutexes, optionally creates a named destination Space, copies every referenced Blob to a fresh destination key before structural mutation, commits the destination Space, then deletes the source selection and optionally creates a `spacePreview` breadcrumb in one source executor batch. A determinate later failure applies inverse source and destination deltas; when the destination was created by the request, failure also deletes that complete Space and its blobs. Failure to remove the new destination is surfaced explicitly rather than reported as an ordinary rejected move. BlobScope intentionally has no per-key deletion, so a failed move into an existing Space may leave freshly copied unreferenced blobs until that Space is deleted; referenced source blobs are retained because the source Space may still contain other references. The guarantee remains all-or-compensated while the Server continues running and receives determinate backend outcomes, matching the existing `SpaceHandle.write()` boundary. Retained Disk Space repository and handle instances, blob scopes, and legacy `CanvasStore` instances reject use after the active Workspace changes. Each `spaces()` call returns a fresh Workspace-bound handle and each read rescans current Disk state. The Workspace-qualified LRU is cleared and rebuilt on the next lookup after a switch. The delete-session contract covers overlapping operations through one configured backend instance. Disk realizes it with the shared process-local coordinator; it is not a multi-process transaction or distributed lock, and a SQL adapter must supply an equivalent backend-instance fence using its own mechanisms. diff --git a/docs/architecture/space-preview.md b/docs/architecture/space-preview.md index f48cb8557..f607e6761 100644 --- a/docs/architecture/space-preview.md +++ b/docs/architecture/space-preview.md @@ -37,7 +37,7 @@ Explicit Open Space navigation is the only Phase 1 entry transition. Gesture-dri Ordinary Spaces expose Add Space Preview from the Canvas toolbar's Add Content dropdown. World omits this action because its preview membership is server-managed. -Moving content between Spaces also creates an ordinary source-owned `spacePreview` breadcrumb. It occupies the moved set's former absolute top-left and derives its width and height from the authoritative deduplicated transfer bounds, clamped to `480 × 320` minimum and `2400 × 1600` maximum. It is created in the same source executor batch that deletes the moved roots, and a later move to the same target creates another breadcrumb rather than reusing one at a different historical location. +Moving content between Spaces can also create an ordinary source-owned `spacePreview` breadcrumb when the default-enabled Move option remains selected. It occupies the moved set's former absolute top-left and derives its width and height from the authoritative deduplicated transfer bounds, clamped to `480 × 320` minimum and `2400 × 1600` maximum. It is created in the same source executor batch that deletes the moved roots, and a later move to the same target creates another breadcrumb rather than reusing one at a different historical location. Disabling the option leaves no breadcrumb and does not change boundary-edge removal or compensation. ## Code entry points diff --git a/docs/architecture/web-architecture.md b/docs/architecture/web-architecture.md index df45da9fe..c9622c1ef 100644 --- a/docs/architecture/web-architecture.md +++ b/docs/architecture/web-architecture.md @@ -119,7 +119,7 @@ The Markdown walk is what keeps images inside a `note` alive across Canvases — ### Moving a selection between Spaces -Move is an explicit server-coordinated action rather than a clipboard operation. The single-node and multi-selection floating toolbars open one Canvas-level `MoveSelectionModal`, which lets the user choose an existing ordinary Space or enter the name of a new Space, drains pending Canvas saves, and submits the selected node ids plus the current source version to `POST /api/canvas/:canvasId/move-selection`. A successful toast reports the durable outcome and links to the destination; typed server failures are shown without applying optimistic local mutations. The source keeps a `spacePreview` breadcrumb at the moved set's former absolute bounds. Managed reference nodes do not expose the action. +Move is an explicit server-coordinated action rather than a clipboard operation. The single-node and multi-selection floating toolbars open one Canvas-level `MoveSelectionModal`, which lets the user choose an existing ordinary Space or enter the name of a new Space, and provides a default-enabled checkbox for leaving a source `spacePreview` breadcrumb. The modal resets that choice to enabled each time it opens, drains pending Canvas saves, and submits the selected node ids, Preview choice, and current source version to `POST /api/canvas/:canvasId/move-selection`. A successful toast reports the durable outcome and links to the destination; typed server failures are shown without applying optimistic local mutations. Managed reference nodes do not expose the action. ## Workspace routes and World diff --git a/docs/proposals/move-selected-nodes-between-spaces.md b/docs/proposals/move-selected-nodes-between-spaces.md index 342f80e9d..d3e835b6b 100644 --- a/docs/proposals/move-selected-nodes-between-spaces.md +++ b/docs/proposals/move-selected-nodes-between-spaces.md @@ -6,7 +6,7 @@ Last updated: 2026-09-01 Tracking issue: [#142](https://github.com/microsoft/Huabu/issues/142) -> **Scope.** This proposal adds the smallest complete user-facing operation for moving selected Canvas nodes and Frame subtrees between existing or newly created ordinary Spaces in the active Workspace and leaving a source `spacePreview` breadcrumb. It includes moving an eligible Agent Node's existing conversation identity instead of resetting or copying it. It deliberately does not introduce a general multi-Space transaction API, filesystem WAL, crash recovery, Blob reference counting, garbage collection, or multi-backend transaction protocol. +> **Scope.** This proposal adds the smallest complete user-facing operation for moving selected Canvas nodes and Frame subtrees between existing or newly created ordinary Spaces in the active Workspace, with an optional source `spacePreview` breadcrumb. It includes moving an eligible Agent Node's existing conversation identity instead of resetting or copying it. It deliberately does not introduce a general multi-Space transaction API, filesystem WAL, crash recovery, Blob reference counting, garbage collection, or multi-backend transaction protocol. > **Reliability boundary.** The operation provides user-visible all-or-compensated behavior while the Server process continues running and returns a determinate result. Process termination, power loss, and an unknown remote-backend outcome remain outside #142, matching the current `SpaceHandle.write()` contract. @@ -31,7 +31,7 @@ If a determinate failure occurs after the destination write, the service compens ## 3. Goals - Move one or more selected ordinary Canvas nodes into an existing or newly created ordinary Space. -- Leave one `spacePreview` breadcrumb at the moved set's former source bounds. +- Offer a default-enabled choice to leave one `spacePreview` breadcrumb at the moved set's former source bounds. - Treat selected Frames as subtree roots and preserve every descendant exactly once. - Preserve parent-child hierarchy, parent-local child geometry, root-to-root relative geometry, node style, Frame layout data, and internal edge style. - Omit edges that cross the transfer boundary and report them explicitly. @@ -174,6 +174,7 @@ interface MoveSelectionRequest { destination: | { kind: 'existing'; canvasId: string } | { kind: 'new'; title: string }; + createSourcePreview: boolean; expectedSourceVersion: number; } ``` @@ -190,7 +191,7 @@ The response contains: interface MoveSelectionResponse { transferId: string; destination: { canvasId: string; title: string | null; created: boolean }; - sourcePreviewNodeId: string; + sourcePreviewNodeId: string | null; sourceVersion: number; destinationVersion: number; roots: Array<{ @@ -362,7 +363,7 @@ The expected implementation size is approximately 700–1,000 production lines a - Single and multi-selection toolbars open the same modal. - Existing and new destination modes submit their strict discriminated request variants. - A request-created destination is removed when later validation or execution fails. -- The source replacement leaves one correctly targeted preview at the authoritative moved bounds and clamps extreme dimensions. +- The default-enabled source Preview choice leaves one correctly targeted preview at the authoritative moved bounds and clamps extreme dimensions; disabling it leaves no preview. - The confirmation shows normalized roots, descendants, preserved edges, omitted edges, and moved Agent conversation count. - Ineligible Agent selections disable confirmation and explain the blocking reason. - Pending writes drain before submission. diff --git a/packages/shared/src/types/api/space-move.test.ts b/packages/shared/src/types/api/space-move.test.ts index f7bc05b89..bd1605e29 100644 --- a/packages/shared/src/types/api/space-move.test.ts +++ b/packages/shared/src/types/api/space-move.test.ts @@ -14,11 +14,13 @@ describe('moveSelectionBodySchema', () => { moveSelectionBodySchema.parse({ selectedNodeIds: ['node-a', 'node-b'], destination: { kind: 'existing', canvasId: 'canvas-b' }, + createSourcePreview: true, expectedSourceVersion: 7, }), ).toEqual({ selectedNodeIds: ['node-a', 'node-b'], destination: { kind: 'existing', canvasId: 'canvas-b' }, + createSourcePreview: true, expectedSourceVersion: 7, }); }); @@ -28,6 +30,7 @@ describe('moveSelectionBodySchema', () => { moveSelectionBodySchema.safeParse({ selectedNodeIds: [], destination: { kind: 'existing', canvasId: 'canvas-b' }, + createSourcePreview: true, expectedSourceVersion: 7, }).success, ).toBe(false); @@ -35,6 +38,17 @@ describe('moveSelectionBodySchema', () => { moveSelectionBodySchema.safeParse({ selectedNodeIds: ['node-a'], destination: { kind: 'new', title: ' ' }, + createSourcePreview: true, + expectedSourceVersion: 7, + }).success, + ).toBe(false); + }); + + it('requires an explicit source Preview choice', () => { + expect( + moveSelectionBodySchema.safeParse({ + selectedNodeIds: ['node-a'], + destination: { kind: 'existing', canvasId: 'canvas-b' }, expectedSourceVersion: 7, }).success, ).toBe(false); @@ -70,4 +84,27 @@ describe('moveSelectionResponseSchema', () => { }).success, ).toBe(true); }); + + it('accepts an outcome without a source Preview', () => { + expect( + moveSelectionResponseSchema.safeParse({ + transferId: 'transfer-a', + destination: { + canvasId: 'canvas-b', + title: 'Destination', + created: false, + }, + sourcePreviewNodeId: null, + sourceVersion: 8, + destinationVersion: 4, + roots: [], + movedNodeCount: 1, + movedFrameCount: 0, + preservedEdgeCount: 0, + omittedBoundaryEdges: [], + renamedNodes: [], + movedConversationCount: 0, + }).success, + ).toBe(true); + }); }); diff --git a/packages/shared/src/types/api/space-move.ts b/packages/shared/src/types/api/space-move.ts index 5d3a834ed..9c19ad273 100644 --- a/packages/shared/src/types/api/space-move.ts +++ b/packages/shared/src/types/api/space-move.ts @@ -20,6 +20,7 @@ export const moveSelectionBodySchema = z }) .strict(), ]), + createSourcePreview: z.boolean(), expectedSourceVersion: z.number().int().nonnegative(), }) .strict(); @@ -81,7 +82,7 @@ export const moveSelectionResponseSchema = z created: z.boolean(), }) .strict(), - sourcePreviewNodeId: z.string().min(1), + sourcePreviewNodeId: z.string().min(1).nullable(), sourceVersion: z.number().int().nonnegative(), destinationVersion: z.number().int().nonnegative(), roots: z.array(movedRootSchema), From 33d67b484222d5db8a044dcba76686a4fbb7f4a5 Mon Sep 17 00:00:00 2001 From: Yuqing Yang Date: Thu, 3 Sep 2026 03:27:46 +0000 Subject: [PATCH 08/12] fix: confine Space directory renames Validate both rename operands as direct Workspace children at the filesystem boundary. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../storage/backends/disk/canvas-dirs.ts | 56 +++++++++++-------- .../backends/disk/canvas-dirs.world.test.ts | 18 ++++++ docs/architecture/canvas-storage.md | 2 + 3 files changed, 54 insertions(+), 22 deletions(-) diff --git a/apps/server/src/modules/storage/backends/disk/canvas-dirs.ts b/apps/server/src/modules/storage/backends/disk/canvas-dirs.ts index b77b9e376..86d74a76a 100644 --- a/apps/server/src/modules/storage/backends/disk/canvas-dirs.ts +++ b/apps/server/src/modules/storage/backends/disk/canvas-dirs.ts @@ -240,6 +240,36 @@ export type CanvasDirRenameResult = | { ok: false; reason: 'not-found' } | { ok: false; reason: 'fs-error'; message: string }; +function renameWorkspaceChild( + workspacePath: string, + fromName: string, + toName: string, +): Extract | null { + const workspaceRoot = path.resolve(workspacePath); + const from = path.resolve(workspaceRoot, fromName); + const to = path.resolve(workspaceRoot, toName); + if ( + path.dirname(from) !== workspaceRoot || + path.dirname(to) !== workspaceRoot + ) { + return { + ok: false, + reason: 'fs-error', + message: 'Space directory rename must remain within the Workspace root', + }; + } + try { + renameSync(from, to); + return null; + } catch (err) { + return { + ok: false, + reason: 'fs-error', + message: err instanceof Error ? err.message : String(err), + }; + } +} + /** * Rename a canvas directory both on disk and in the index. Same-slot * renames (case-only) update the stored casing without touching the @@ -269,17 +299,8 @@ export function renameCanvasDirOnDisk( // filesystems (APFS / NTFS) `renameSync` updates the casing in // place; on case-sensitive ones it's a regular rename. const ws = getWorkspacePath(); - const from = path.join(ws, entry.filename); - const to = path.join(ws, newDirName); - try { - renameSync(from, to); - } catch (err) { - return { - ok: false, - reason: 'fs-error', - message: err instanceof Error ? err.message : String(err), - }; - } + const renameError = renameWorkspaceChild(ws, entry.filename, newDirName); + if (renameError) return renameError; index.rename(canvasId, newDirName); } return { ok: true, dirName: newDirName }; @@ -291,17 +312,8 @@ export function renameCanvasDirOnDisk( } const ws = getWorkspacePath(); - const from = path.join(ws, entry.filename); - const to = path.join(ws, newDirName); - try { - renameSync(from, to); - } catch (err) { - return { - ok: false, - reason: 'fs-error', - message: err instanceof Error ? err.message : String(err), - }; - } + const renameError = renameWorkspaceChild(ws, entry.filename, newDirName); + if (renameError) return renameError; index.rename(canvasId, newDirName); return { ok: true, dirName: newDirName }; diff --git a/apps/server/src/modules/storage/backends/disk/canvas-dirs.world.test.ts b/apps/server/src/modules/storage/backends/disk/canvas-dirs.world.test.ts index e71c32fd4..b90023923 100644 --- a/apps/server/src/modules/storage/backends/disk/canvas-dirs.world.test.ts +++ b/apps/server/src/modules/storage/backends/disk/canvas-dirs.world.test.ts @@ -104,6 +104,24 @@ describe('World canvas directory indexing', () => { expect(persisted.title).toBe('World'); }); + it.each(['../escaped', 'nested/escaped', '/tmp/escaped'])( + 'rejects a rename outside the Workspace root: %s', + (newDirName) => { + expect(renameCanvasDirOnDisk('canvas-a', newDirName)).toEqual({ + ok: false, + reason: 'fs-error', + message: 'Space directory rename must remain within the Workspace root', + }); + expect(canvasDirName('canvas-a')).toBe('Project A'); + expect( + readFileSync( + path.join(workspaceState.path, 'Project A', 'space.json'), + 'utf8', + ), + ).toContain('"canvasId":"canvas-a"'); + }, + ); + it('rejects malformed World topology during a runtime rescan', () => { writeFileSync( path.join(workspaceState.path, '.world', 'space.json'), diff --git a/docs/architecture/canvas-storage.md b/docs/architecture/canvas-storage.md index a70e9e31d..67df6b930 100644 --- a/docs/architecture/canvas-storage.md +++ b/docs/architecture/canvas-storage.md @@ -98,6 +98,8 @@ The Disk structured adapter and compatibility facade resolve the same cached leg Canvas persistence DTOs and the write coordinator live under `modules/canvas/`; Workspace identity, durable membership, and Disk locators live under `modules/storage/`, while active-Workspace lifecycle and boot migrations remain under `modules/workspace/`; generic filesystem and Markdown codecs live under `utils/`. `canvasRoot()` validates the identifier and then verifies that the resolved Space directory remains a strict descendant of the active Workspace before any downstream Disk operation receives it. `module-boundaries.test.ts` enforces the storage dependency direction, prevents new consumers of the forwarding shims, and holds the neutrality guard: no production file outside `storage/` may import a Disk layout symbol or a legacy `CanvasStore` symbol. The check is import-level and symbol-level — a local variable that happens to be called `artifactPath` is not a violation, while importing `canvasRoot`, or reaching `getCanvasStore` through the barrel, is. Migrations are exempt because they rewrite frozen historical on-disk shapes; tests are exempt for the same reason they may name an adapter. +Disk Space directory rename resolves both basenames against the Workspace root at the final filesystem boundary and requires each result to be a direct child before calling `renameSync`. This defense-in-depth check applies even though user-facing titles are already converted with `toSafeFilename`, so no upstream caller can turn a nested or absolute path into a rename target. + Space deletion is serialized against composed blob puts by a writer-preferring admission coordinator and holds an active-Workspace lease across blob cleanup and structured destruction. `beginDelete()` acquires the exclusive session before blob I/O; `finish()` removes structured state, while `abort()` releases the fence without doing so. Blobs are swept before structure so a failed sweep can be retried while the Space record still names them. Puts already admitted may finish; a put queued behind a successful deletion rechecks existence and fails without recreating blobs, while a failed blob sweep leaves the record available for retry. Mutations through existing Space handles and repositories reject while deletion is active or queued; reads remain available for cleanup. Residual direct-filesystem capabilities such as ZIP import, RFS upload/delete, and external-note claim are outside this repository fence and remain blockers for a non-Disk profile. Cross-Space Move is an application-level compensated operation, not a new storage transaction. It holds an active-Workspace lease and both Canvas write mutexes, optionally creates a named destination Space, copies every referenced Blob to a fresh destination key before structural mutation, commits the destination Space, then deletes the source selection and optionally creates a `spacePreview` breadcrumb in one source executor batch. A determinate later failure applies inverse source and destination deltas; when the destination was created by the request, failure also deletes that complete Space and its blobs. Failure to remove the new destination is surfaced explicitly rather than reported as an ordinary rejected move. BlobScope intentionally has no per-key deletion, so a failed move into an existing Space may leave freshly copied unreferenced blobs until that Space is deleted; referenced source blobs are retained because the source Space may still contain other references. The guarantee remains all-or-compensated while the Server continues running and receives determinate backend outcomes, matching the existing `SpaceHandle.write()` boundary. From ba8ee6019bff6d411f83e7523200ef22898cef35 Mon Sep 17 00:00:00 2001 From: Yuqing Date: Thu, 3 Sep 2026 12:02:14 +0800 Subject: [PATCH 09/12] Potential fix for pull request finding 'CodeQL / Uncontrolled data used in path expression' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- .../storage/backends/disk/canvas-dirs.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/apps/server/src/modules/storage/backends/disk/canvas-dirs.ts b/apps/server/src/modules/storage/backends/disk/canvas-dirs.ts index 86d74a76a..ee371c3c8 100644 --- a/apps/server/src/modules/storage/backends/disk/canvas-dirs.ts +++ b/apps/server/src/modules/storage/backends/disk/canvas-dirs.ts @@ -240,11 +240,30 @@ export type CanvasDirRenameResult = | { ok: false; reason: 'not-found' } | { ok: false; reason: 'fs-error'; message: string }; +function isDirectWorkspaceChildName(name: string): boolean { + if (!name || name === '.' || name === '..') return false; + if (path.basename(name) !== name) return false; + if (name.includes(path.sep)) return false; + if (path.sep !== '/' && name.includes('/')) return false; + if (path.sep !== '\\' && name.includes('\\')) return false; + return true; +} + function renameWorkspaceChild( workspacePath: string, fromName: string, toName: string, ): Extract | null { + if ( + !isDirectWorkspaceChildName(fromName) || + !isDirectWorkspaceChildName(toName) + ) { + return { + ok: false, + reason: 'fs-error', + message: 'Space directory rename requires direct Workspace child names', + }; + } const workspaceRoot = path.resolve(workspacePath); const from = path.resolve(workspaceRoot, fromName); const to = path.resolve(workspaceRoot, toName); From 35bfc742091fa2cd4058851494b0dc24d414544e Mon Sep 17 00:00:00 2001 From: Yuqing Yang Date: Thu, 3 Sep 2026 04:12:22 +0000 Subject: [PATCH 10/12] Revert "Potential fix for pull request finding 'CodeQL / Uncontrolled data used in path expression'" This reverts commit ba8ee6019bff6d411f83e7523200ef22898cef35. --- .../storage/backends/disk/canvas-dirs.ts | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/apps/server/src/modules/storage/backends/disk/canvas-dirs.ts b/apps/server/src/modules/storage/backends/disk/canvas-dirs.ts index ee371c3c8..86d74a76a 100644 --- a/apps/server/src/modules/storage/backends/disk/canvas-dirs.ts +++ b/apps/server/src/modules/storage/backends/disk/canvas-dirs.ts @@ -240,30 +240,11 @@ export type CanvasDirRenameResult = | { ok: false; reason: 'not-found' } | { ok: false; reason: 'fs-error'; message: string }; -function isDirectWorkspaceChildName(name: string): boolean { - if (!name || name === '.' || name === '..') return false; - if (path.basename(name) !== name) return false; - if (name.includes(path.sep)) return false; - if (path.sep !== '/' && name.includes('/')) return false; - if (path.sep !== '\\' && name.includes('\\')) return false; - return true; -} - function renameWorkspaceChild( workspacePath: string, fromName: string, toName: string, ): Extract | null { - if ( - !isDirectWorkspaceChildName(fromName) || - !isDirectWorkspaceChildName(toName) - ) { - return { - ok: false, - reason: 'fs-error', - message: 'Space directory rename requires direct Workspace child names', - }; - } const workspaceRoot = path.resolve(workspacePath); const from = path.resolve(workspaceRoot, fromName); const to = path.resolve(workspaceRoot, toName); From 941e33d9a08ba00b6b2347aad47131c78b44fcae Mon Sep 17 00:00:00 2001 From: Yuqing Yang Date: Thu, 3 Sep 2026 05:43:49 +0000 Subject: [PATCH 11/12] fix: reuse a CodeQL-aware path guard Resolve direct Workspace children through a shared containment helper recognized by the path-injection query. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../storage/backends/disk/canvas-dirs.ts | 19 ++++++++------ apps/server/src/utils/fs.test.ts | 21 ++++++++++++++++ apps/server/src/utils/fs.ts | 25 +++++++++++++++++++ docs/architecture/canvas-storage.md | 2 +- 4 files changed, 58 insertions(+), 9 deletions(-) diff --git a/apps/server/src/modules/storage/backends/disk/canvas-dirs.ts b/apps/server/src/modules/storage/backends/disk/canvas-dirs.ts index 86d74a76a..39e25ebee 100644 --- a/apps/server/src/modules/storage/backends/disk/canvas-dirs.ts +++ b/apps/server/src/modules/storage/backends/disk/canvas-dirs.ts @@ -12,7 +12,11 @@ import path from 'node:path'; import { SPACE_JSON_FILENAME, WORLD_CANVAS_DIR_NAME } from './layout.js'; import { NameIndex, type NameIndexResult } from './name-index.js'; -import { readJsonStrict, sanitizeId } from '../../../../utils/fs.js'; +import { + readJsonStrict, + resolveDirectChildPath, + sanitizeId, +} from '../../../../utils/fs.js'; import { dedupeName, normalizeForCompare, @@ -245,13 +249,12 @@ function renameWorkspaceChild( fromName: string, toName: string, ): Extract | null { - const workspaceRoot = path.resolve(workspacePath); - const from = path.resolve(workspaceRoot, fromName); - const to = path.resolve(workspaceRoot, toName); - if ( - path.dirname(from) !== workspaceRoot || - path.dirname(to) !== workspaceRoot - ) { + let from: string; + let to: string; + try { + from = resolveDirectChildPath(workspacePath, fromName); + to = resolveDirectChildPath(workspacePath, toName); + } catch { return { ok: false, reason: 'fs-error', diff --git a/apps/server/src/utils/fs.test.ts b/apps/server/src/utils/fs.test.ts index 18c6101ac..ebf3b1358 100644 --- a/apps/server/src/utils/fs.test.ts +++ b/apps/server/src/utils/fs.test.ts @@ -21,6 +21,7 @@ import { readJsonLines, readJsonLinesStrict, readJsonStrict, + resolveDirectChildPath, } from './fs.js'; let root = ''; @@ -60,6 +61,26 @@ describe('readJsonStrict', () => { }); }); +describe('resolveDirectChildPath', () => { + it('resolves a direct child beneath the root', () => { + expect(resolveDirectChildPath(root, 'Space name')).toBe( + path.join(root, 'Space name'), + ); + }); + + it.each([ + '', + '.', + '..', + '../escaped', + 'nested/escaped', + 'nested\\escaped', + path.resolve(root, '..', 'escaped'), + ])('rejects a non-child path: %s', (childName) => { + expect(() => resolveDirectChildPath(root, childName)).toThrow(); + }); +}); + describe('atomicWriteText', () => { it('does not share or consume the legacy fixed .tmp sibling', () => { const target = path.join(root, 'record.json'); diff --git a/apps/server/src/utils/fs.ts b/apps/server/src/utils/fs.ts index 371c11557..eefee03d5 100644 --- a/apps/server/src/utils/fs.ts +++ b/apps/server/src/utils/fs.ts @@ -63,6 +63,31 @@ export function safeJoin(base: string, ...segments: string[]): string { return joined; } +/** + * Resolve one direct child of a root directory. + * + * The containment check intentionally uses the path.resolve + startsWith + * pattern recognized by CodeQL's js/path-injection query. + */ +export function resolveDirectChildPath( + base: string, + childName: string, +): string { + const baseResolved = path.resolve(base); + const resolved = path.resolve(baseResolved, childName); + if (!resolved.startsWith(`${baseResolved}${path.sep}`)) { + throw new Error('Path must remain within its root directory'); + } + if ( + childName.includes('/') || + childName.includes('\\') || + path.dirname(resolved) !== baseResolved + ) { + throw new Error('Path must be a direct child of its root directory'); + } + return resolved; +} + /** Create a directory recursively (no-op if it already exists). */ export function mkdirp(dir: string): void { mkdirSync(dir, { recursive: true }); diff --git a/docs/architecture/canvas-storage.md b/docs/architecture/canvas-storage.md index 67df6b930..b99c64e72 100644 --- a/docs/architecture/canvas-storage.md +++ b/docs/architecture/canvas-storage.md @@ -98,7 +98,7 @@ The Disk structured adapter and compatibility facade resolve the same cached leg Canvas persistence DTOs and the write coordinator live under `modules/canvas/`; Workspace identity, durable membership, and Disk locators live under `modules/storage/`, while active-Workspace lifecycle and boot migrations remain under `modules/workspace/`; generic filesystem and Markdown codecs live under `utils/`. `canvasRoot()` validates the identifier and then verifies that the resolved Space directory remains a strict descendant of the active Workspace before any downstream Disk operation receives it. `module-boundaries.test.ts` enforces the storage dependency direction, prevents new consumers of the forwarding shims, and holds the neutrality guard: no production file outside `storage/` may import a Disk layout symbol or a legacy `CanvasStore` symbol. The check is import-level and symbol-level — a local variable that happens to be called `artifactPath` is not a violation, while importing `canvasRoot`, or reaching `getCanvasStore` through the barrel, is. Migrations are exempt because they rewrite frozen historical on-disk shapes; tests are exempt for the same reason they may name an adapter. -Disk Space directory rename resolves both basenames against the Workspace root at the final filesystem boundary and requires each result to be a direct child before calling `renameSync`. This defense-in-depth check applies even though user-facing titles are already converted with `toSafeFilename`, so no upstream caller can turn a nested or absolute path into a rename target. +Disk Space directory rename resolves both basenames through the shared `resolveDirectChildPath()` boundary before calling `renameSync`. The helper uses the CodeQL-recognized normalized-root containment pattern and then requires a separator-free direct child on both POSIX and Windows. This defense-in-depth check applies even though user-facing titles are already converted with `toSafeFilename`, so no upstream caller can turn a nested or absolute path into a rename target. Space deletion is serialized against composed blob puts by a writer-preferring admission coordinator and holds an active-Workspace lease across blob cleanup and structured destruction. `beginDelete()` acquires the exclusive session before blob I/O; `finish()` removes structured state, while `abort()` releases the fence without doing so. Blobs are swept before structure so a failed sweep can be retried while the Space record still names them. Puts already admitted may finish; a put queued behind a successful deletion rechecks existence and fails without recreating blobs, while a failed blob sweep leaves the record available for retry. Mutations through existing Space handles and repositories reject while deletion is active or queued; reads remain available for cleanup. Residual direct-filesystem capabilities such as ZIP import, RFS upload/delete, and external-note claim are outside this repository fence and remain blockers for a non-Disk profile. From e947403b0045bb881c9f1586d2764d7362d9ca1b Mon Sep 17 00:00:00 2001 From: Yuqing Yang Date: Thu, 3 Sep 2026 06:45:53 +0000 Subject: [PATCH 12/12] docs: add filesystem path safety guidance Direct agents to the shared CodeQL-aware path confinement helpers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/copilot-instructions.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index bbb05ba1b..223da522c 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -7,6 +7,7 @@ - **Markdown Formatting**: In Markdown files (docs, prompts, READMEs) write **one line per paragraph** (soft-wrap) — do not hard-wrap prose at ~80 columns. Let the editor wrap; only insert a real line break to start a new paragraph or list item. Tables, code blocks, and ASCII diagrams are exempt. - **Common UI Primitives**: Before creating UI elements, inspect `apps/web/src/components/Common` and use or extend a suitable component. Treat each component's exported props and implementation as its current contract. Always use `