From 812e64cf2c09346c45ce7e4d06fe2da22af8a53a Mon Sep 17 00:00:00 2001 From: gsdali <51393997+gsdali@users.noreply.github.com> Date: Fri, 21 Aug 2026 14:46:26 +1000 Subject: [PATCH 1/2] Add get_selection / highlight_selection: the agent-to-viewport selection bridge (#189, #190) Implements the two new MCP tools per both issues' refined-spec comments, against the wire format in SecondMouseAU/OCCTSwiftInteraction#17 (in review in parallel; schema treated as final regardless of later wording changes there). No host implements the writer/watcher side yet, so this only consumes hand-written fixture sidecar files. get_selection reads selection.json + host.lock from the resolved output directory (Paths.swift) and returns a three-state result: noHost (selections: null), hostRunning with selections: [] (host live, nothing selected), or hostRunning with selections: [...]. Each entry is resolved against this server's own scene the same way select_topology resolves a match, reusing SelectionTools.graphIndex(...) and SelectionRegistry.record so the minted selectionId composes with remap_selection/measure_distance/ etc. A per-entry resolution failure (bad bodyId, out-of-range index) is reported inline without failing the whole response; a running host with selection.json missing or malformed is reported as an explicit tool error, never swallowed into an empty result. highlight_selection writes highlight_requests/.json atomically (Data.write(to:options:.atomic)) and polls highlight_requests/handled/.json for a bounded timeout, returning the host's real applied/rejected/superseded outcome, an explicit timeout, or noHost immediately (no request written) when no host is running. kind/scheme are validated against their closed wire-format enums before writing; bodyId/index are written through unvalidated against the live scene, since this tool has no other access to check them. Shared plumbing: HostLock.checkLiveness(outputDir:) probes host.lock with a non-blocking shared flock (success means no host, EWOULDBLOCK means one is running), added in the new Sources/OCCTMCPCore/Tools/SelectionBridgeTools.swift per the code-structure policy (one file per tool family). Registers both tools in Server.swift's catalog + dispatch. Updates the tool count (77 -> 79) across README.md, CLAUDE.md, and the docs site, adds the two tools to docs/reference/selection.md and its Families table, and documents the new file in CLAUDE.md's Tools/ list. 13 new fixture-driven tests in SwiftTests/OCCTMCPCoreTests/SelectionBridgeToolsTests.swift cover all three liveness states, per-entry resolution failures, missing/malformed selection.json, atomic request writes, handled/ polling (applied + rejected), the timeout path, the noHost fast path, unvalidated-scene-reference writes, and client-side kind/scheme enum rejection. Fixes PingTests' hardcoded tool count to match. Full suite: 232/232 passing. Co-Authored-By: Claude Sonnet 5 --- CLAUDE.md | 7 +- README.md | 8 +- Sources/OCCTMCPCore/Server.swift | 72 +++ .../Tools/SelectionBridgeTools.swift | 496 ++++++++++++++++++ SwiftTests/OCCTMCPCoreTests/PingTests.swift | 4 +- .../SelectionBridgeToolsTests.swift | 420 +++++++++++++++ docs/_config.yml | 2 +- docs/guides/architecture.md | 4 +- docs/guides/getting-started.md | 7 +- docs/index.md | 2 +- docs/reference/README.md | 4 +- docs/reference/selection.md | 108 +++- okf/components/index.md | 9 +- okf/index.md | 7 +- 14 files changed, 1124 insertions(+), 26 deletions(-) create mode 100644 Sources/OCCTMCPCore/Tools/SelectionBridgeTools.swift create mode 100644 SwiftTests/OCCTMCPCoreTests/SelectionBridgeToolsTests.swift diff --git a/CLAUDE.md b/CLAUDE.md index 9e41599..dae5cf4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -6,7 +6,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co MCP server that gives LLMs the ability to author, inspect, and iterate on 3D CAD models with OpenCASCADE via the OCCTSwift family. Two implementations live side-by-side: -- **Swift** (`Sources/`, `Package.swift`): the **primary**, in-process server. Uses the official Swift MCP SDK, calls OCCTSwift / OCCTSwiftMesh / OCCTSwiftTools / OCCTSwiftAIS / DrawingComposer directly. 77 typed tools. macOS 15+. +- **Swift** (`Sources/`, `Package.swift`): the **primary**, in-process server. Uses the official Swift MCP SDK, calls OCCTSwift / OCCTSwiftMesh / OCCTSwiftTools / OCCTSwiftAIS / DrawingComposer directly. 79 typed tools. macOS 15+. - **Node / TypeScript** (`src/`, `dist/`): the original implementation. Shells out to the `occtkit` CLI via `OCCTSwiftScripts`. 37 tools (the pre-v0.4 surface; selection / remap / annotations are Swift-only). Both speak stdio MCP and read/write the same `manifest.json` + `annotations.json` files in the output directory. Pick whichever fits the host: the Swift binary eliminates JSONL marshalling and per-call subprocess spawn; the Node server runs anywhere a Node 18+ runtime exists, but needs `occtkit` on `$PATH`. @@ -41,7 +41,7 @@ npm run test:integration # node:test end-to-end chain through occtkit (slow; ~3 `Sources/OCCTMCPCore/` (library) + `Sources/OCCTMCPServer/` (executable that connects stdio). -- `Server.swift`: `createServer()` factory: registers all 77 tools with their JSON Schemas, returns an `MCP.Server` ready to bind to a transport. Tests import `createServer()` to introspect the registry without binding stdio. The `get_api_reference` tool's `mcp_tools` category dumps the live registry as JSON Schema for LLM auto-discovery. +- `Server.swift`: `createServer()` factory: registers all 79 tools with their JSON Schemas, returns an `MCP.Server` ready to bind to a transport. Tests import `createServer()` to introspect the registry without binding stdio. The `get_api_reference` tool's `mcp_tools` category dumps the live registry as JSON Schema for LLM auto-discovery. - `Tools/`: one file per tool family: - `CoreTools.swift`: `get_scene`, `get_script`, `export_model`, `get_api_reference` - `ExecuteScriptTool.swift`: `execute_script` (writes Swift to tempfile, `occtkit run` via the resolved binary, parses manifest) @@ -77,6 +77,7 @@ npm run test:integration # node:test end-to-end chain through occtkit (slow; ~3 - `AnalysisTools.swift`: graph-level: `graph_validate`, `graph_compact`, `graph_dedup`, `graph_ml`, `feature_recognize` - `SelectionTools.swift`: `select_topology`. #91: the index embedded in a `selectionId` (`sel:#face[]`) has to be a `BRepGraph` node index (that's what `RemapTools.remapViaHistory` feeds into `findDerivedOrSelf(of:)`), not a raw `Shape.faces()/.edges()/.vertices()` enumeration index. Those turned out NOT to be the same index space for edges/vertices (verified false on a plain box: `TopologyIdentityTests`; true only for faces, apparently by coincidence). `SelectionTools.graphIndex(for:kind:in:fallback:)` resolves the real graph index via `BRepGraph.findNode(for:)` for every kind. `edgeGeometryFields(edge:)` (#119) computes `endpoints`/`direction`/`circleCenter`/`radius`/`axis`/`startAngle`/`endAngle`, shared by `select_topology`'s edge anchors and `IntrospectionTools.queryTopology`'s edge results. GOTCHA pinned by `EdgeGeometryFieldsTests`: `edge.curve3D?.circleProperties` used to crash (SIGSEGV) when chained in one expression, since `CircleProperties` wraps the SAME native handle as its parent `Curve3D` WITHOUT retaining it, so the temporary `Curve3D` deallocates (releasing the handle) the instant the optional-chain expression finishes; fixed by binding `edge.curve3D` to a local `let` that outlives every use of `.circleProperties` on it - `RemapTools.swift`: `remap_selection` (consults `HistoryRegistry`, falls back to centroid heuristic). The centroid-heuristic path also mints selectionIds (via `pickClosest` → `registry.record`), so `remapOne` uses the same `SelectionTools.graphIndex(...)` resolution as `select_topology`: both selectionId-minting paths have to agree on the graph-index convention (#91) + - `SelectionBridgeTools.swift`: `get_selection`, `highlight_selection` (#189/#190): the agent-to-viewport-host selection bridge, per the wire format in `SecondMouseAU/OCCTSwiftInteraction#17` (in review in parallel with this work; schema treated as final regardless of later wording changes there). OCCTMCP is a stdio MCP process with no persistent link to any app's live `InteractiveContext`/`ViewportService`, so neither tool calls into a live selection directly: both speak sidecar files in the resolved output directory (`Paths.swift`), the same convention every other OCCTMCP sidecar already uses. `HostLock.checkLiveness(outputDir:)` is the shared liveness primitive both tools consult: a host holds an exclusive `flock` on `host.lock` for its whole lifetime, and a non-blocking SHARED lock probe distinguishes `.noHost` (probe succeeds, nothing holds the exclusive lock) from `.hostRunning` (probe fails, something does) without ever blocking the calling thread. `get_selection` reads `selection.json` (`{selections:[{bodyId,kind,index,uid?}], revision, updatedAt}`) and returns a three-state result (`state: "noHost"` with `selections: null`, vs `state: "hostRunning"` with `selections: []` or `[...]`) so an agent can never confuse "no viewport" with "viewport, nothing selected". Each entry resolves against THIS server's own scene the same way `select_topology` resolves a match: enumerate the body's faces/edges/vertices in the same order `select_topology` does, index into that enumeration at the wire entry's `index`, then convert to a `BRepGraph` node index via the existing `SelectionTools.graphIndex(...)` convention before minting the `SelectionRegistry` entry, so the resulting `selectionId` composes with `remap_selection`/`measure_distance`/etc. exactly like one `select_topology` minted itself. A per-entry resolution failure (unknown `bodyId`, out-of-range `index`) is reported inline on that entry (`error` set, `selectionId`/`anchor` nil) rather than failing the whole response; a RUNNING host with `selection.json` missing or malformed (a torn read, per the ADR's atomicity rule not yet being honored by some future writer) is reported as an explicit tool error instead, since the host is expected to maintain that sidecar for its whole lifetime and a missing/torn file while live is anomalous, not "nothing selected". `highlight_selection` writes `highlight_requests/.json` (`{id,bodyId,kind,index,scheme,question?}`, `scheme` matching `OCCTSwiftAIS.SelectionScheme` exactly: `replace`/`add`/`remove`/`xor`) atomically (`Data.write(to:options:.atomic)`, i.e. temp name + rename) and polls `highlight_requests/handled/.json` for a bounded timeout (default 5s), returning the host's real `applied`/`rejected`/`superseded` outcome, an explicit `timeout` if nothing answers, or `noHost` immediately (no request written) if no host is running at all, rather than hanging out the full timeout for a request nobody will ever read. `kind`/`scheme` are validated against their closed wire-format enums before writing (a malformed request is never written), but `bodyId`/`index` are written through UNVALIDATED against the live scene: this tool has no other access to check them, so a bad reference still round-trips as the host's own `rejected` outcome through the same poll, never a client-side pre-check. `id` is generated by the tool itself (never caller-supplied), so a caller never needs to invent or coordinate its own id across concurrent highlight calls - `AnnotationsTools.swift`: `add_dimension`, `add_scene_primitive`, `remove_scene_annotation` - `GapFillerTools.swift`: `show_bounding_box`, `diff_overlay`, `select_by_feature` - `IntrospectionRegistryTools.swift`: `list_selections`, `clear_selections`, `list_annotations`, `list_zones`, `clear_zones` (#101, `RegistryIntrospectionTools`; the latter two mirror `list_selections`/`clear_selections` against `ZoneRegistry`) @@ -358,7 +359,7 @@ Verify what a fresh clone / CI actually resolves (not the local sibling-checkout ## MCP Tools -77 tools in Swift; 37 in Node (no selection / remap / annotations / history / reconstruct / mesh-zone analysis / mesh inspection / alignment / curvature / mesh features). See README.md for the categorized table: that's the LLM-facing surface and stays canonical. +79 tools in Swift; 37 in Node (no selection / remap / annotations / history / reconstruct / mesh-zone analysis / mesh inspection / alignment / curvature / mesh features / viewport selection bridge). See README.md for the categorized table: that's the LLM-facing surface and stays canonical. ## Script Template diff --git a/README.md b/README.md index c030c53..ca27f4b 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ MCP server that gives LLMs the ability to author, inspect, and iterate on 3D CAD Part of the [OCCTSwift ecosystem](https://github.com/SecondMouseAU/OCCTSwift/blob/main/docs/ecosystem.md) — see the ecosystem map for how this package sits on top of the kernel, viewport, bridge, and AIS layers. SemVer-stable from v1.0.0. -The Swift implementation calls OCCT directly in-process (no subprocess, no JSONL marshalling) and exposes 77 typed MCP tools that cover authoring, scene reads, mutation, introspection, construction, analysis, I/O, mesh, drawing, selection / remap, mesh-zone analysis, mesh inspection, alignment, and dimension overlays. +The Swift implementation calls OCCT directly in-process (no subprocess, no JSONL marshalling) and exposes 79 typed MCP tools that cover authoring, scene reads, mutation, introspection, construction, analysis, I/O, mesh, drawing, selection / remap, mesh-zone analysis, mesh inspection, alignment, dimension overlays, and the agent-to-viewport-host selection bridge. ## How It Works @@ -23,7 +23,7 @@ For novel geometry the typed tools don't cover, the LLM falls back to `execute_s ## Tools -77 tools, organized below. Call `get_api_reference({ category: "mcp_tools" })` to dump every tool's JSON Schema in one shot, useful for LLM auto-discovery. Most flows can answer "what's the volume?", "make it red", "boolean-subtract these", "render a preview", "add a dimension between these two faces", "export to STEP", and "draw this" without ever touching `execute_script`. +79 tools, organized below. Call `get_api_reference({ category: "mcp_tools" })` to dump every tool's JSON Schema in one shot, useful for LLM auto-discovery. Most flows can answer "what's the volume?", "make it red", "boolean-subtract these", "render a preview", "add a dimension between these two faces", "export to STEP", and "draw this" without ever touching `execute_script`. ### Authoring @@ -139,6 +139,8 @@ The mesh-domain check-list / measurement surface (Phase 2 of the mesh-analysis e | `select_by_feature` | Bulk pick by feature kind (e.g. all hole edges) | | `list_selections` | Inspect the in-memory selection registry | | `clear_selections` | Wipe the registry | +| `get_selection` (#189) | Read a live viewport host's current selection (`/selection.json` + `host.lock`, per `SecondMouseAU/OCCTSwiftInteraction#17`). Three-state result: `noHost` vs `hostRunning` with an empty or populated selection list; each entry resolves against this server's own scene the same way `select_topology` does and mints a composable `selectionId` | +| `highlight_selection` (#190) | Ask a live viewport host to highlight one sub-shape (`replace`/`add`/`remove`/`xor`, mirroring `OCCTSwiftAIS.SelectionScheme`). Writes `highlight_requests/.json`, polls `highlight_requests/handled/.json` for the host's real `applied`/`rejected`/`superseded` outcome, or an explicit `timeout`/`noHost` | ### Annotations & overlays @@ -199,7 +201,7 @@ LLM read/write over an attributed reconstruction graph — annotate per-node dec This repo ships two implementations side-by-side: -- **Swift** (`Sources/`, `Package.swift`): the **primary** server. In-process against OCCTSwift / OCCTSwiftMesh / OCCTSwiftTools / OCCTSwiftAIS / DrawingComposer using the [official Swift MCP SDK](https://swiftpackageindex.com/modelcontextprotocol/swift-sdk). 77 tools. macOS 15+ (the OCCT.xcframework arm64 platform). +- **Swift** (`Sources/`, `Package.swift`): the **primary** server. In-process against OCCTSwift / OCCTSwiftMesh / OCCTSwiftTools / OCCTSwiftAIS / DrawingComposer using the [official Swift MCP SDK](https://swiftpackageindex.com/modelcontextprotocol/swift-sdk). 79 tools. macOS 15+ (the OCCT.xcframework arm64 platform). - **Node / TypeScript** (`src/`, `dist/`) — the original implementation. Shells out to the `occtkit` CLI for everything Swift-side. 37 tools (the pre-v0.4 surface; selection / remap / annotations are Swift-only). Useful if you can't run a macOS binary. Both speak stdio MCP and read/write the same manifest format. diff --git a/Sources/OCCTMCPCore/Server.swift b/Sources/OCCTMCPCore/Server.swift index 10fb4a6..cccde5f 100644 --- a/Sources/OCCTMCPCore/Server.swift +++ b/Sources/OCCTMCPCore/Server.swift @@ -790,6 +790,56 @@ func catalogTools() -> [Tool] { "additionalProperties": .bool(false), ]) ), + Tool( + name: "get_selection", + description: + "Read the live viewport host's current selection (SecondMouseAU/OCCTSwiftInteraction#17: /selection.json + host.lock). Three-state result: state=\"noHost\" (no viewport host is running at all, selections is null) vs state=\"hostRunning\" with selections=[] (host live, nothing picked) vs selections=[...] (host live, N items picked): never collapse those into one boolean/empty-array reading. Each selection is resolved against this server's own scene the same way select_topology resolves a match (area/bounds/centroid for a face, length/curveType/endpoints for an edge, position for a vertex) and mints a selectionId that composes with remap_selection/measure_distance/add_dimension/etc. A host running but missing or malformed selection.json is reported as an explicit error, never swallowed into an empty selection list.", + inputSchema: .object([ + "type": .string("object"), + "properties": .object([:]), + "additionalProperties": .bool(false), + ]) + ), + Tool( + name: "highlight_selection", + description: + "Ask the live viewport host to highlight one sub-shape (SecondMouseAU/OCCTSwiftInteraction#17: writes /highlight_requests/.json, polls highlight_requests/handled/.json for the real outcome). scheme mirrors OCCTSwiftAIS.SelectionScheme exactly: \"replace\" swaps the host's whole selection, \"add\"/\"remove\" adjust it, \"xor\" toggles. bodyId/kind/index are written through unvalidated against the live scene (this tool has no other access to check them); an unknown bodyId or out-of-range index still comes back as the host's own rejected outcome through the same poll, not a client-side pre-check. Returns outcome=\"noHost\" immediately (no request written) if no viewport host is running, \"timeout\" if the host never writes a handled/ response within the deadline, or the host's own applied/rejected/superseded outcome.", + inputSchema: .object([ + "type": .string("object"), + "properties": .object([ + "bodyId": .object(["type": .string("string")]), + "kind": .object([ + "type": .string("string"), + "enum": .array([ + .string("body"), .string("face"), .string("edge"), .string("vertex"), + ]), + ]), + "index": .object(["type": .string("integer")]), + "scheme": .object([ + "type": .string("string"), + "enum": .array([ + .string("replace"), .string("add"), .string("remove"), .string("xor"), + ]), + ]), + "question": .object([ + "type": .string("string"), + "description": .string( + "Optional natural-language context for the host to show alongside the highlight, e.g. when confirming an ambiguous pick with a human." + ), + ]), + "timeoutSeconds": .object([ + "type": .string("number"), + "description": .string( + "How long to poll handled/.json before returning outcome=\"timeout\". Default 5.0." + ), + ]), + ]), + "required": .array([ + .string("bodyId"), .string("kind"), .string("index"), .string("scheme"), + ]), + "additionalProperties": .bool(false), + ]) + ), Tool( name: "ping", description: @@ -2366,6 +2416,28 @@ func dispatch(callName: String, arguments: [String: Value]) async -> CallTool.Re bodyId: bodyId, kind: kind, filter: filter, limit: limit ).asCallToolResult() + case "get_selection": + return await SelectionBridgeTools.getSelection().asCallToolResult() + + case "highlight_selection": + guard let bodyId = arguments["bodyId"]?.stringValue, + let kind = arguments["kind"]?.stringValue, + let index = arguments["index"]?.intValue, + let scheme = arguments["scheme"]?.stringValue + else { + return ToolText( + "highlight_selection requires `bodyId`, `kind`, `index`, and `scheme`.", + isError: true + ).asCallToolResult() + } + let timeout = + arguments["timeoutSeconds"]?.numberValue ?? SelectionBridgeTools.defaultTimeoutSeconds + return await SelectionBridgeTools.highlightSelection( + bodyId: bodyId, kind: kind, index: index, scheme: scheme, + question: arguments["question"]?.stringValue, + timeoutSeconds: timeout + ).asCallToolResult() + case "set_assembly_metadata": guard let inputPath = arguments["inputPath"]?.stringValue, let outputPath = arguments["outputPath"]?.stringValue diff --git a/Sources/OCCTMCPCore/Tools/SelectionBridgeTools.swift b/Sources/OCCTMCPCore/Tools/SelectionBridgeTools.swift new file mode 100644 index 0000000..671400a --- /dev/null +++ b/Sources/OCCTMCPCore/Tools/SelectionBridgeTools.swift @@ -0,0 +1,496 @@ +// SelectionBridgeTools: get_selection / highlight_selection, the read and +// write halves of the agent <-> viewport-host selection bridge (#189/#190). +// +// OCCTMCP is a stdio MCP process with no persistent link to any app's live +// InteractiveContext/ViewportService, so this doesn't call into a live +// selection directly. Instead it speaks the sidecar-file wire format from +// SecondMouseAU/OCCTSwiftInteraction#17 (the ADR): a host process (e.g. +// ACADStudio) writes `selection.json` and watches `highlight_requests/` in +// the SAME resolved output directory every other OCCTMCP sidecar already +// uses. That host also holds an exclusive flock on `host.lock` for its +// whole lifetime, which is this file's one piece of shared plumbing: a +// non-blocking shared-lock probe distinguishes "no host is running" from +// "a host is running but nothing (yet) selected": the three-state +// distinction #189's own acceptance criteria calls out explicitly, since an +// agent that reads "nothing selected" when the truth is "no viewport" +// inherits a wrong premise into every later call. +// +// File layout (all under the resolved output directory): +// host.lock : empty file, host holds LOCK_EX for its lifetime +// host.json : {pid, startedAt, hostName, hostVersion, schemaVersion} +// selection.json : {selections: [{bodyId,kind,index,uid?}], revision, updatedAt} +// highlight_requests/.json : written here: {id,bodyId,kind,index,scheme,question?} +// highlight_requests/handled/.json : written by the host: {outcome,reason?} +// +// Every writer (ours included) uses atomic write (temp name + rename, i.e. +// `Data.write(to:options:.atomic)`); reads tolerate a missing file (no host +// has run yet) without crashing, except where a running host demonstrably +// should be maintaining the file (see `getSelection` below). + +import Foundation +import OCCTSwift +import ScriptHarness + +#if canImport(Darwin) + import Darwin +#elseif canImport(Glibc) + import Glibc +#endif + +/// Tri-state result of probing `host.lock`. +enum HostLiveness: Sendable, Equatable { + case noHost + case hostRunning +} + +/// Non-blocking `host.lock` probe (#189 ADR): a host holds an exclusive +/// flock on this file for its whole lifetime. +/// +/// Trying a non-blocking SHARED +/// lock and seeing whether that succeeds is the documented check: success +/// means nothing holds the exclusive lock (no host), failure means +/// something does (a host is running). A missing lock file is the same as +/// no host, trivially: nothing can be holding a lock on a file that was +/// never created. An `open` failure for any other reason (permissions, a +/// TOCTOU race with a deletion) fails open to `.noHost` rather than +/// claiming a positive "host running" signal this probe never actually +/// observed. +enum HostLock { + static func checkLiveness(outputDir: String) -> HostLiveness { + let path = "\(outputDir)/host.lock" + guard FileManager.default.fileExists(atPath: path) else { + return .noHost + } + let fd = open(path, O_RDONLY) + guard fd >= 0 else { + return .noHost + } + defer { close(fd) } + if flock(fd, LOCK_SH | LOCK_NB) == 0 { + flock(fd, LOCK_UN) + return .noHost + } + return .hostRunning + } +} + +public enum SelectionBridgeTools { + + // MARK: - Wire types (SecondMouseAU/OCCTSwiftInteraction#17) + + public struct HostInfo: Codable, Sendable { + public let pid: Int + public let startedAt: String + public let hostName: String + public let hostVersion: String + public let schemaVersion: Int + } + + /// One entry from `selection.json`, exactly as the host wrote it. + /// + /// `uid` here is the HOST's own opaque identity string, from whatever + /// `BRepGraph.GraphUID` its own process minted; it is passed through + /// as-is and never treated as a uid this server's own + /// `SelectionRegistry`/`HistoryRegistry` graph could resolve (those are + /// separate process-local graph instances, and a `GraphUID` is + /// documented as instance-scoped). + public struct SelectionJSONEntry: Codable, Sendable { + public let bodyId: String + public let kind: String + public let index: Int + public let uid: String? + } + + public struct SelectionSidecar: Codable, Sendable { + public let selections: [SelectionJSONEntry] + public let revision: Int + public let updatedAt: String + } + + /// A `highlight_requests/.json` request, before a host has consumed it. + public struct HighlightRequest: Codable, Sendable { + public let id: String + public let bodyId: String + public let kind: String + public let index: Int + public let scheme: String + public let question: String? + } + + /// A `highlight_requests/handled/.json` response, written by the host. + public struct HandledOutcome: Codable, Sendable { + public let outcome: String + public let reason: String? + } + + // MARK: - get_selection + + /// One resolved selection entry in the `get_selection` response. + /// + /// `index`/`uid` mirror the wire entry as-received; `selectionId`/`anchor` + /// are populated only when resolution against this server's OWN scene + /// succeeded (bad `bodyId`, or an `index` outside this body's current + /// topology, leaves both nil and populates `error` instead, without + /// failing the rest of the response). + public struct ResolvedSelection: Encodable { + public let selectionId: String? + public let bodyId: String + public let kind: String + public let index: Int + public let uid: String? + public let anchor: AnchorSnapshot? + public let error: String? + } + + public struct GetSelectionResult: Encodable { + /// "noHost" | "hostRunning". + /// + /// Never a bare bool/empty-array: an agent + /// must not be able to confuse "no viewport" with "viewport, nothing + /// selected". + public let state: String + /// nil for `noHost`; an array (possibly empty) for `hostRunning`. + public let selections: [ResolvedSelection]? + public let revision: Int? + public let updatedAt: String? + public let host: HostInfo? + } + + public static func getSelection( + store: ManifestStore = ManifestStore(), + registry: SelectionRegistry = .shared + ) async -> ToolText { + let outputDir = (store.path as NSString).deletingLastPathComponent + + guard HostLock.checkLiveness(outputDir: outputDir) == .hostRunning else { + return IntrospectionTools.encode( + GetSelectionResult( + state: "noHost", selections: nil, revision: nil, updatedAt: nil, host: nil)) + } + + let selectionPath = "\(outputDir)/selection.json" + guard FileManager.default.fileExists(atPath: selectionPath) else { + // A host is running (host.lock held) but never wrote its + // sidecar: per the ADR the host is expected to maintain this + // file for its whole lifetime, so this is anomalous, not + // "nothing selected", and must not be swallowed into an empty + // result. + return ToolText( + "get_selection: a host is running (host.lock held at \(outputDir)/host.lock) " + + "but selection.json is missing at \(selectionPath). The host is expected " + + "to maintain this sidecar for its whole lifetime; this looks like a startup " + + "race or a host that hasn't wired up the writer yet, not \"nothing selected\".", + isError: true + ) + } + + let data: Data + do { + data = try Data(contentsOf: URL(fileURLWithPath: selectionPath)) + } catch { + return ToolText( + "get_selection: could not read selection.json: \(error.localizedDescription)", + isError: true + ) + } + + let sidecar: SelectionSidecar + do { + sidecar = try JSONDecoder().decode(SelectionSidecar.self, from: data) + } catch { + // A torn read (the ADR's atomicity rule not yet honored by some + // future writer) or genuinely corrupt JSON both land here. Either + // way this is a real failure to surface, not an empty selection. + return ToolText( + "get_selection: selection.json is malformed (a torn/partial write, or invalid " + + "JSON): \(error.localizedDescription). Not treating this as \"nothing selected\".", + isError: true + ) + } + + let host = readHostInfo(outputDir: outputDir) + + var resolved: [ResolvedSelection] = [] + resolved.reserveCapacity(sidecar.selections.count) + for entry in sidecar.selections { + resolved.append(await resolveSelection(entry: entry, store: store, registry: registry)) + } + + return IntrospectionTools.encode( + GetSelectionResult( + state: "hostRunning", selections: resolved, + revision: sidecar.revision, updatedAt: sidecar.updatedAt, host: host + )) + } + + static func readHostInfo(outputDir: String) -> HostInfo? { + let path = "\(outputDir)/host.json" + guard let data = try? Data(contentsOf: URL(fileURLWithPath: path)) else { return nil } + return try? JSONDecoder().decode(HostInfo.self, from: data) + } + + /// Resolve one `selection.json` entry against THIS server's own scene, + /// the same way `select_topology` resolves a match: enumerate the + /// body's faces/edges/vertices in the same order `select_topology` does, + /// index into that enumeration, then convert to a `BRepGraph` node index + /// via `SelectionTools.graphIndex(...)` before minting the + /// `SelectionRegistry` entry, so the resulting `selectionId` composes + /// with `remap_selection`/`measure_distance`/etc. exactly like one + /// `select_topology` minted itself. + static func resolveSelection( + entry: SelectionJSONEntry, + store: ManifestStore, + registry: SelectionRegistry + ) async -> ResolvedSelection { + func failure(_ message: String) -> ResolvedSelection { + ResolvedSelection( + selectionId: nil, bodyId: entry.bodyId, kind: entry.kind, index: entry.index, + uid: entry.uid, anchor: nil, error: message) + } + + let loaded: (manifest: ScriptManifest, body: BodyDescriptor, shape: Shape, path: String) + do { + loaded = try IntrospectionTools.loadShape(bodyId: entry.bodyId, store: store) + } catch { + return failure("\(error)") + } + + let lineage: (shape: Shape, graph: BRepGraph, root: BRepGraph.NodeRef, isFreshLoad: Bool) + do { + lineage = try await HistoryRegistry.shared.currentInput( + bodyId: entry.bodyId, path: loaded.path) + } catch { + return failure("\(error)") + } + let shape = lineage.shape + let graph = lineage.graph + + switch entry.kind { + case "body": + guard let bb = shape.bounds else { + return failure("no bounding box for body \(entry.bodyId)") + } + let center = [ + (bb.min.x + bb.max.x) * 0.5, + (bb.min.y + bb.max.y) * 0.5, + (bb.min.z + bb.max.z) * 0.5, + ] + let anchor = TopologyAnchor.body(bodyId: entry.bodyId) + let snapshot = AnchorSnapshot(center: center) + await registry.record(anchor: anchor, snapshot: snapshot) + return ResolvedSelection( + selectionId: anchor.selectionId, bodyId: entry.bodyId, kind: entry.kind, + index: entry.index, uid: entry.uid, anchor: snapshot, error: nil) + + case "face": + let faces = shape.faces() + guard entry.index >= 0, entry.index < faces.count else { + return failure( + "face index \(entry.index) out of range (\(faces.count) faces on \(entry.bodyId))" + ) + } + let face = faces[entry.index] + let (center, normal) = SelectionTools.faceCenterAndNormal(face: face) + let area = face.area() + let surfaceType = String(describing: face.surfaceType) + let graphIdx = SelectionTools.graphIndex( + for: Shape.fromFace(face), kind: .face, in: graph, fallback: entry.index) + let uid = graph.uid(ofNodeKind: Int(BRepGraph.NodeKind.face.rawValue), index: graphIdx) + let anchor = TopologyAnchor.face(bodyId: entry.bodyId, index: graphIdx, uid: uid) + let snapshot = AnchorSnapshot( + center: [center.x, center.y, center.z], + normal: normal.map { [$0.x, $0.y, $0.z] }, + area: area, + surfaceType: surfaceType + ) + await registry.record(anchor: anchor, snapshot: snapshot) + return ResolvedSelection( + selectionId: anchor.selectionId, bodyId: entry.bodyId, kind: entry.kind, + index: entry.index, uid: entry.uid, anchor: snapshot, error: nil) + + case "edge": + let edges = shape.edges() + guard entry.index >= 0, entry.index < edges.count else { + return failure( + "edge index \(entry.index) out of range (\(edges.count) edges on \(entry.bodyId))" + ) + } + let edge = edges[entry.index] + let length = SelectionTools.edgeLength(edge: edge) + let curveType = String(describing: edge.curveType) + let center = SelectionTools.edgeMidpoint(edge: edge) + let geom = SelectionTools.edgeGeometryFields(edge: edge) + let graphIdx = SelectionTools.graphIndex( + for: Shape.fromEdge(edge), kind: .edge, in: graph, fallback: entry.index) + let uid = graph.uid(ofNodeKind: Int(BRepGraph.NodeKind.edge.rawValue), index: graphIdx) + let anchor = TopologyAnchor.edge(bodyId: entry.bodyId, index: graphIdx, uid: uid) + let snapshot = AnchorSnapshot( + center: center.map { [$0.x, $0.y, $0.z] } ?? [0, 0, 0], + length: length, + curveType: curveType, + circleCenter: geom.circleCenter, + endpoints: geom.endpoints, + direction: geom.direction, + radius: geom.radius, + axis: geom.axis, + startAngle: geom.startAngle, + endAngle: geom.endAngle + ) + await registry.record(anchor: anchor, snapshot: snapshot) + return ResolvedSelection( + selectionId: anchor.selectionId, bodyId: entry.bodyId, kind: entry.kind, + index: entry.index, uid: entry.uid, anchor: snapshot, error: nil) + + case "vertex": + let vertices = shape.subShapes(ofType: .vertex) + guard entry.index >= 0, entry.index < vertices.count else { + return failure( + "vertex index \(entry.index) out of range (\(vertices.count) vertices on \(entry.bodyId))" + ) + } + let vertexShape = vertices[entry.index] + guard let point = SelectionTools.vertexPoint(vertexShape) else { + return failure( + "could not resolve vertex position for \(entry.bodyId)#vertex[\(entry.index)]") + } + let graphIdx = SelectionTools.graphIndex( + for: vertexShape, kind: .vertex, in: graph, fallback: entry.index) + let uid = graph.uid( + ofNodeKind: Int(BRepGraph.NodeKind.vertex.rawValue), index: graphIdx) + let anchor = TopologyAnchor.vertex(bodyId: entry.bodyId, index: graphIdx, uid: uid) + let snapshot = AnchorSnapshot(center: [point.x, point.y, point.z]) + await registry.record(anchor: anchor, snapshot: snapshot) + return ResolvedSelection( + selectionId: anchor.selectionId, bodyId: entry.bodyId, kind: entry.kind, + index: entry.index, uid: entry.uid, anchor: snapshot, error: nil) + + default: + return failure( + "unknown kind '\(entry.kind)'. Expected one of: body, face, edge, vertex.") + } + } + + // MARK: - highlight_selection + + static let validKinds = ["body", "face", "edge", "vertex"] + static let validSchemes = ["replace", "add", "remove", "xor"] + public static let defaultTimeoutSeconds: Double = 5.0 + public static let defaultPollIntervalSeconds: Double = 0.1 + + public struct HighlightSelectionResult: Encodable { + /// nil only for the `noHost` outcome: no request was written, so + /// there is no id to report. + public let id: String? + /// "applied" | "rejected" | "superseded" (from the host's handled/ + /// file) | "timeout" (no handled/ file within the deadline) | + /// "noHost" (no viewport host is running at all). + public let outcome: String + public let reason: String? + } + + /// Write a `highlight_requests/.json` request and poll + /// `highlight_requests/handled/.json` for the real outcome. + /// + /// `bodyId`/`kind`/`index` are written through UNVALIDATED against this + /// server's own scene (per #190's own scope: this tool has no other + /// access to the live viewport's scene to validate against, so a bad + /// reference is still written and comes back as the host's own + /// `rejected` outcome through the same poll, not a client-side + /// pre-check). `kind`/`scheme` ARE validated against the wire format's + /// own closed enums before writing anything, since those aren't a scene + /// fact to defer, they're the request's own shape. + /// + /// The id is generated + /// here (never supplied by the caller), so a caller never needs to + /// invent or coordinate its own id across concurrent highlight calls; + /// combined with the atomic (temp name + rename) write, a single call's + /// own request write can never land torn or collide with another call's. + public static func highlightSelection( + bodyId: String, + kind: String, + index: Int, + scheme: String, + question: String? = nil, + store: ManifestStore = ManifestStore(), + timeoutSeconds: Double = defaultTimeoutSeconds, + pollIntervalSeconds: Double = defaultPollIntervalSeconds + ) async -> ToolText { + guard validKinds.contains(kind) else { + return ToolText( + "highlight_selection: unknown kind '\(kind)'. Expected one of: " + + validKinds.joined(separator: ", ") + ".", + isError: true + ) + } + guard validSchemes.contains(scheme) else { + return ToolText( + "highlight_selection: unknown scheme '\(scheme)'. Expected one of: " + + validSchemes.joined(separator: ", ") + ".", + isError: true + ) + } + + let outputDir = (store.path as NSString).deletingLastPathComponent + + guard HostLock.checkLiveness(outputDir: outputDir) == .hostRunning else { + // Fail fast rather than writing a request nothing will ever read + // and hanging out the full poll timeout for no reason. + return IntrospectionTools.encode( + HighlightSelectionResult( + id: nil, outcome: "noHost", + reason: + "No host.lock is held at \(outputDir)/host.lock: no viewport host is running to consume highlight_requests/." + )) + } + + let id = UUID().uuidString + let request = HighlightRequest( + id: id, bodyId: bodyId, kind: kind, index: index, scheme: scheme, question: question) + + let requestsDir = "\(outputDir)/highlight_requests" + let handledDir = "\(requestsDir)/handled" + do { + try FileManager.default.createDirectory( + atPath: requestsDir, withIntermediateDirectories: true) + let encoder = JSONEncoder() + encoder.outputFormatting = [.sortedKeys] + let data = try encoder.encode(request) + try data.write(to: URL(fileURLWithPath: "\(requestsDir)/\(id).json"), options: .atomic) + } catch { + return ToolText( + "highlight_selection: failed to write request: \(error.localizedDescription)", + isError: true + ) + } + + let handledPath = "\(handledDir)/\(id).json" + let deadline = Date().addingTimeInterval(timeoutSeconds) + while Date() < deadline { + if let outcome = readHandledOutcome(path: handledPath) { + return IntrospectionTools.encode( + HighlightSelectionResult( + id: id, outcome: outcome.outcome, reason: outcome.reason) + ) + } + try? await Task.sleep( + nanoseconds: UInt64(max(pollIntervalSeconds, 0.001) * 1_000_000_000)) + } + + return IntrospectionTools.encode( + HighlightSelectionResult( + id: id, outcome: "timeout", + reason: + "No response written to highlight_requests/handled/\(id).json within \(timeoutSeconds)s." + )) + } + + static func readHandledOutcome(path: String) -> HandledOutcome? { + guard FileManager.default.fileExists(atPath: path), + let data = try? Data(contentsOf: URL(fileURLWithPath: path)), + let decoded = try? JSONDecoder().decode(HandledOutcome.self, from: data) + else { + return nil + } + return decoded + } +} diff --git a/SwiftTests/OCCTMCPCoreTests/PingTests.swift b/SwiftTests/OCCTMCPCoreTests/PingTests.swift index 0752ee3..1d862dc 100644 --- a/SwiftTests/OCCTMCPCoreTests/PingTests.swift +++ b/SwiftTests/OCCTMCPCoreTests/PingTests.swift @@ -10,9 +10,9 @@ struct PingTests { #expect(tools.contains(where: { $0.name == "ping" })) } - @Test("server exposes exactly 77 tools (#118 adds measure_vertex_fit, #121 adds fit_edge_chain)") + @Test("server exposes exactly 79 tools (#189 adds get_selection, #190 adds highlight_selection)") func toolCount() async throws { - #expect(catalogTools().count == 77) + #expect(catalogTools().count == 79) } @Test("ping handler returns pong") diff --git a/SwiftTests/OCCTMCPCoreTests/SelectionBridgeToolsTests.swift b/SwiftTests/OCCTMCPCoreTests/SelectionBridgeToolsTests.swift new file mode 100644 index 0000000..638c93c --- /dev/null +++ b/SwiftTests/OCCTMCPCoreTests/SelectionBridgeToolsTests.swift @@ -0,0 +1,420 @@ +// SelectionBridgeToolsTests (#189/#190): get_selection / highlight_selection +// against hand-written fixture files in a tempdir, exactly as both +// refined-spec comments describe. No host actually implements the writer +// side yet (SecondMouseAU/OCCTSwiftInteraction#16/ACADStudio#16 are still +// upstream), so every test here plays the host itself: it holds host.lock, +// writes selection.json / handled/.json by hand, and asserts the tool's +// response against that fixture. + +import Foundation +import Testing +import OCCTSwift +import ScriptHarness +@testable import OCCTMCPCore + +#if canImport(Darwin) + import Darwin +#elseif canImport(Glibc) + import Glibc +#endif + +/// Holds an exclusive flock on a file for the lifetime of the test, playing +/// the part of a live viewport host per the ADR (`host.lock`). +final class HeldLock { + private let fd: Int32 + + init?(path: String) { + FileManager.default.createFile(atPath: path, contents: nil) + let opened = open(path, O_RDWR) + guard opened >= 0 else { return nil } + guard flock(opened, LOCK_EX) == 0 else { + close(opened) + return nil + } + fd = opened + } + + func release() { + flock(fd, LOCK_UN) + close(fd) + } +} + +@Suite("SelectionBridgeTools (#189/#190)") +struct SelectionBridgeToolsTests { + + // MARK: - fixture scene + + func scene(_ bodies: [(id: String, shape: Shape)]) throws -> ManifestStore { + let dir = NSTemporaryDirectory() + "occtmcp-selbridge-\(UUID().uuidString)" + try FileManager.default.createDirectory(atPath: dir, withIntermediateDirectories: true) + let descriptors = bodies.map { + BodyDescriptor(id: $0.id, file: "\($0.id).brep", color: [1, 1, 1, 1]) + } + let manifest = ScriptManifest( + version: 1, timestamp: Date(), description: "selbridge", bodies: descriptors) + let store = ManifestStore(path: "\(dir)/manifest.json") + try store.write(manifest) + for b in bodies { + try Exporter.writeBREP(shape: b.shape, to: URL(fileURLWithPath: "\(dir)/\(b.id).brep")) + } + return store + } + + func dirOf(_ store: ManifestStore) -> String { (store.path as NSString).deletingLastPathComponent } + + func writeSelectionSidecar( + dir: String, + selections: [(bodyId: String, kind: String, index: Int, uid: String?)], + revision: Int = 1 + ) throws { + let entries = selections.map { + SelectionBridgeTools.SelectionJSONEntry( + bodyId: $0.bodyId, kind: $0.kind, index: $0.index, uid: $0.uid) + } + let sidecar = SelectionBridgeTools.SelectionSidecar( + selections: entries, revision: revision, updatedAt: "2026-08-21T00:00:00Z") + let data = try JSONEncoder().encode(sidecar) + try data.write(to: URL(fileURLWithPath: "\(dir)/selection.json"), options: .atomic) + } + + // MARK: - decode mirrors + + struct ResolvedSelectionMirror: Decodable { + let selectionId: String? + let bodyId: String + let kind: String + let index: Int + let uid: String? + let error: String? + } + struct GetSelectionResultMirror: Decodable { + let state: String + let selections: [ResolvedSelectionMirror]? + let revision: Int? + let updatedAt: String? + } + struct HighlightResultMirror: Decodable { + let id: String? + let outcome: String + let reason: String? + } + + // ── get_selection: three liveness states ───────────────────────────── + + @Test("get_selection: no host.lock at all -> state=noHost, selections=nil") + func getSelectionNoHost() async throws { + let store = try scene([]) + defer { try? FileManager.default.removeItem(atPath: dirOf(store)) } + + let result = await SelectionBridgeTools.getSelection(store: store) + #expect(!result.isError) + let r = try JSONDecoder().decode(GetSelectionResultMirror.self, from: Data(result.text.utf8)) + #expect(r.state == "noHost") + #expect(r.selections == nil) + } + + @Test("get_selection: host running, selection.json has zero entries -> hostRunning([])") + func getSelectionHostRunningEmpty() async throws { + let store = try scene([]) + let dir = dirOf(store) + defer { try? FileManager.default.removeItem(atPath: dir) } + + let lock = try #require(HeldLock(path: "\(dir)/host.lock")) + defer { lock.release() } + try writeSelectionSidecar(dir: dir, selections: []) + + let result = await SelectionBridgeTools.getSelection(store: store) + #expect(!result.isError) + let r = try JSONDecoder().decode(GetSelectionResultMirror.self, from: Data(result.text.utf8)) + #expect(r.state == "hostRunning") + #expect(r.selections?.isEmpty == true, "must be an empty array, not nil, when a host is live") + } + + @Test("get_selection: host running, selection.json has entries -> hostRunning([...]), resolved + registered") + func getSelectionHostRunningWithEntries() async throws { + let box = try #require(Shape.box(width: 10, height: 20, depth: 30)) + let store = try scene([("box", box)]) + let dir = dirOf(store) + defer { try? FileManager.default.removeItem(atPath: dir) } + + let lock = try #require(HeldLock(path: "\(dir)/host.lock")) + defer { lock.release() } + try writeSelectionSidecar( + dir: dir, + selections: [(bodyId: "box", kind: "face", index: 0, uid: "host-uid-abc123")]) + + let registry = SelectionRegistry() + let result = await SelectionBridgeTools.getSelection(store: store, registry: registry) + #expect(!result.isError, "unexpected error: \(result.text)") + let r = try JSONDecoder().decode(GetSelectionResultMirror.self, from: Data(result.text.utf8)) + #expect(r.state == "hostRunning") + let selections = try #require(r.selections) + #expect(selections.count == 1) + let entry = selections[0] + #expect(entry.error == nil, "resolution should have succeeded: \(entry.error ?? "")") + #expect(entry.uid == "host-uid-abc123", "the host's own wire uid passes through unchanged") + let selectionId = try #require(entry.selectionId) + #expect(selectionId.hasPrefix("sel:box#face[")) + + // #189 criterion: the minted selectionId must round-trip through + // SelectionRegistry, so remap_selection/measure_distance/etc. can + // consume it exactly like one select_topology minted itself. + let anchor = await registry.anchor(for: selectionId) + #expect(anchor != nil, "selectionId must resolve through SelectionRegistry") + let snapshot = await registry.snapshot(for: selectionId) + #expect(snapshot != nil) + #expect(snapshot?.area != nil, "a face selection should carry an area, resolved like select_topology's own") + } + + @Test("get_selection: an entry with a bad bodyId or out-of-range index is reported per-entry, not fatal") + func getSelectionPartialResolutionFailure() async throws { + let box = try #require(Shape.box(width: 10, height: 20, depth: 30)) + let store = try scene([("box", box)]) + let dir = dirOf(store) + defer { try? FileManager.default.removeItem(atPath: dir) } + + let lock = try #require(HeldLock(path: "\(dir)/host.lock")) + defer { lock.release() } + try writeSelectionSidecar( + dir: dir, + selections: [ + (bodyId: "box", kind: "face", index: 0, uid: nil), + (bodyId: "does-not-exist", kind: "face", index: 0, uid: nil), + (bodyId: "box", kind: "face", index: 9999, uid: nil), + ]) + + let registry = SelectionRegistry() + let result = await SelectionBridgeTools.getSelection(store: store, registry: registry) + #expect(!result.isError, "a per-entry failure must not fail the whole call") + let r = try JSONDecoder().decode(GetSelectionResultMirror.self, from: Data(result.text.utf8)) + let selections = try #require(r.selections) + #expect(selections.count == 3) + #expect(selections[0].error == nil) + #expect(selections[0].selectionId != nil) + #expect(selections[1].error != nil, "bad bodyId should surface a per-entry error") + #expect(selections[1].selectionId == nil) + #expect(selections[2].error != nil, "out-of-range index should surface a per-entry error") + #expect(selections[2].selectionId == nil) + } + + // ── get_selection: torn/malformed selection.json ───────────────────── + + @Test("get_selection: host running but selection.json missing -> explicit error, not empty result") + func getSelectionMissingSidecarIsError() async throws { + let store = try scene([]) + let dir = dirOf(store) + defer { try? FileManager.default.removeItem(atPath: dir) } + + let lock = try #require(HeldLock(path: "\(dir)/host.lock")) + defer { lock.release() } + // Deliberately never write selection.json. + + let result = await SelectionBridgeTools.getSelection(store: store) + #expect(result.isError, "a running host with no selection.json at all must be an explicit error") + } + + @Test("get_selection: torn/malformed selection.json -> explicit error, not swallowed into an empty result") + func getSelectionMalformedSidecarIsError() async throws { + let store = try scene([]) + let dir = dirOf(store) + defer { try? FileManager.default.removeItem(atPath: dir) } + + let lock = try #require(HeldLock(path: "\(dir)/host.lock")) + defer { lock.release() } + // A torn write: valid JSON syntax truncated mid-object, simulating a + // non-atomic writer caught mid-write. + let torn = Data("{\"selections\": [{\"bodyId\": \"box\", \"kind\"".utf8) + try torn.write(to: URL(fileURLWithPath: "\(dir)/selection.json")) + + let result = await SelectionBridgeTools.getSelection(store: store) + #expect(result.isError, "malformed JSON must be reported as an explicit error") + } + + // ── highlight_selection: writes request, generates id, atomic write ── + + @Test("highlight_selection: no host at all -> outcome=noHost immediately, no request written") + func highlightNoHost() async throws { + let store = try scene([]) + let dir = dirOf(store) + defer { try? FileManager.default.removeItem(atPath: dir) } + + let result = await SelectionBridgeTools.highlightSelection( + bodyId: "box", kind: "face", index: 0, scheme: "replace", store: store, + timeoutSeconds: 1.0, pollIntervalSeconds: 0.02) + #expect(!result.isError) + let r = try JSONDecoder().decode(HighlightResultMirror.self, from: Data(result.text.utf8)) + #expect(r.outcome == "noHost") + #expect(r.id == nil) + #expect( + !FileManager.default.fileExists(atPath: "\(dir)/highlight_requests"), + "must not write a request nothing will ever consume") + } + + @Test("highlight_selection: rejects an unknown kind/scheme before writing anything") + func highlightRejectsBadEnumsClientSide() async throws { + let store = try scene([]) + let dir = dirOf(store) + defer { try? FileManager.default.removeItem(atPath: dir) } + let lock = try #require(HeldLock(path: "\(dir)/host.lock")) + defer { lock.release() } + + let badKind = await SelectionBridgeTools.highlightSelection( + bodyId: "box", kind: "diamond", index: 0, scheme: "replace", store: store) + #expect(badKind.isError) + + let badScheme = await SelectionBridgeTools.highlightSelection( + bodyId: "box", kind: "face", index: 0, scheme: "toggle-ish", store: store) + #expect(badScheme.isError) + + #expect( + !FileManager.default.fileExists(atPath: "\(dir)/highlight_requests"), + "a wire-format-invalid request must never be written") + } + + @Test("highlight_selection: a bad bodyId / out-of-range index is still written, not pre-checked client-side") + func highlightWritesUnvalidatedSceneReferences() async throws { + let store = try scene([]) + let dir = dirOf(store) + defer { try? FileManager.default.removeItem(atPath: dir) } + let lock = try #require(HeldLock(path: "\(dir)/host.lock")) + defer { lock.release() } + + let result = await SelectionBridgeTools.highlightSelection( + bodyId: "does-not-exist", kind: "face", index: 999, scheme: "xor", store: store, + timeoutSeconds: 0.2, pollIntervalSeconds: 0.02) + let r = try JSONDecoder().decode(HighlightResultMirror.self, from: Data(result.text.utf8)) + #expect(r.outcome == "timeout", "no host consumed it in this test, so it should time out, not fail up front") + let id = try #require(r.id) + + let requestPath = "\(dir)/highlight_requests/\(id).json" + #expect(FileManager.default.fileExists(atPath: requestPath)) + let data = try Data(contentsOf: URL(fileURLWithPath: requestPath)) + let written = try JSONDecoder().decode(SelectionBridgeTools.HighlightRequest.self, from: data) + #expect(written.bodyId == "does-not-exist") + #expect(written.index == 999) + #expect(written.scheme == "xor") + } + + @Test("highlight_selection: polls handled/.json and returns the host's real outcome") + func highlightPollsAndReturnsHandledOutcome() async throws { + let store = try scene([]) + let dir = dirOf(store) + defer { try? FileManager.default.removeItem(atPath: dir) } + let lock = try #require(HeldLock(path: "\(dir)/host.lock")) + defer { lock.release() } + + async let resultTask = SelectionBridgeTools.highlightSelection( + bodyId: "box", kind: "face", index: 0, scheme: "replace", store: store, + timeoutSeconds: 5.0, pollIntervalSeconds: 0.02) + + // Play the host: wait for the request file to land, read its + // generated id, then write handled/.json by hand. + let requestsDir = "\(dir)/highlight_requests" + var requestId: String? + for _ in 0..<200 { + if let files = try? FileManager.default.contentsOfDirectory(atPath: requestsDir), + let match = files.first(where: { $0.hasSuffix(".json") }) + { + requestId = String(match.dropLast(".json".count)) + break + } + try await Task.sleep(nanoseconds: 15_000_000) + } + let id = try #require(requestId, "highlight_selection never wrote a request file") + + let handledDir = "\(requestsDir)/handled" + try FileManager.default.createDirectory(atPath: handledDir, withIntermediateDirectories: true) + let handled = SelectionBridgeTools.HandledOutcome(outcome: "applied", reason: nil) + let data = try JSONEncoder().encode(handled) + try data.write(to: URL(fileURLWithPath: "\(handledDir)/\(id).json"), options: .atomic) + + let result = await resultTask + #expect(!result.isError) + let r = try JSONDecoder().decode(HighlightResultMirror.self, from: Data(result.text.utf8)) + #expect(r.id == id) + #expect(r.outcome == "applied") + } + + @Test("highlight_selection: rejected outcome (with reason) round-trips from handled/") + func highlightRejectedOutcomeRoundTrips() async throws { + let store = try scene([]) + let dir = dirOf(store) + defer { try? FileManager.default.removeItem(atPath: dir) } + let lock = try #require(HeldLock(path: "\(dir)/host.lock")) + defer { lock.release() } + + async let resultTask = SelectionBridgeTools.highlightSelection( + bodyId: "box", kind: "face", index: 0, scheme: "replace", store: store, + timeoutSeconds: 5.0, pollIntervalSeconds: 0.02) + + let requestsDir = "\(dir)/highlight_requests" + var requestId: String? + for _ in 0..<200 { + if let files = try? FileManager.default.contentsOfDirectory(atPath: requestsDir), + let match = files.first(where: { $0.hasSuffix(".json") }) + { + requestId = String(match.dropLast(".json".count)) + break + } + try await Task.sleep(nanoseconds: 15_000_000) + } + let id = try #require(requestId) + + let handledDir = "\(requestsDir)/handled" + try FileManager.default.createDirectory(atPath: handledDir, withIntermediateDirectories: true) + let handled = SelectionBridgeTools.HandledOutcome( + outcome: "rejected", reason: "bodyId not found in the live scene") + let data = try JSONEncoder().encode(handled) + try data.write(to: URL(fileURLWithPath: "\(handledDir)/\(id).json"), options: .atomic) + + let result = await resultTask + let r = try JSONDecoder().decode(HighlightResultMirror.self, from: Data(result.text.utf8)) + #expect(r.outcome == "rejected") + #expect(r.reason == "bodyId not found in the live scene") + } + + @Test("highlight_selection: times out with an explicit result when nothing consumes the request") + func highlightTimesOutExplicitly() async throws { + let store = try scene([]) + let dir = dirOf(store) + defer { try? FileManager.default.removeItem(atPath: dir) } + let lock = try #require(HeldLock(path: "\(dir)/host.lock")) + defer { lock.release() } + + let result = await SelectionBridgeTools.highlightSelection( + bodyId: "box", kind: "face", index: 0, scheme: "add", store: store, + timeoutSeconds: 0.3, pollIntervalSeconds: 0.05) + #expect(!result.isError) + let r = try JSONDecoder().decode(HighlightResultMirror.self, from: Data(result.text.utf8)) + #expect(r.outcome == "timeout") + #expect(r.id != nil) + } + + // ── atomic write shape ──────────────────────────────────────────────── + + @Test("highlight_selection: the written request file matches the ecosystem#43/OCCTSwiftInteraction#17 schema") + func highlightRequestSchemaShape() async throws { + let store = try scene([]) + let dir = dirOf(store) + defer { try? FileManager.default.removeItem(atPath: dir) } + let lock = try #require(HeldLock(path: "\(dir)/host.lock")) + defer { lock.release() } + + let result = await SelectionBridgeTools.highlightSelection( + bodyId: "box", kind: "vertex", index: 2, scheme: "xor", question: "is this the right vertex?", + store: store, timeoutSeconds: 0.2, pollIntervalSeconds: 0.02) + let r = try JSONDecoder().decode(HighlightResultMirror.self, from: Data(result.text.utf8)) + let id = try #require(r.id) + + let requestPath = "\(dir)/highlight_requests/\(id).json" + let data = try Data(contentsOf: URL(fileURLWithPath: requestPath)) + let written = try JSONDecoder().decode(SelectionBridgeTools.HighlightRequest.self, from: data) + #expect(written.id == id) + #expect(written.bodyId == "box") + #expect(written.kind == "vertex") + #expect(written.index == 2) + #expect(written.scheme == "xor") + #expect(written.question == "is this the right vertex?") + } +} diff --git a/docs/_config.yml b/docs/_config.yml index eeb9113..2a8f5e5 100644 --- a/docs/_config.yml +++ b/docs/_config.yml @@ -6,7 +6,7 @@ # `nav_exclude: true` hides internal pages. GitHub Pages' default plugins still apply # (optional-front-matter, readme-index, relative-links). title: OCCTMCP -description: MCP server that lets LLMs author, inspect, measure and iterate on 3D CAD models with OpenCASCADE via the OCCTSwift family: 77 typed tools + execute_script. +description: MCP server that lets LLMs author, inspect, measure and iterate on 3D CAD models with OpenCASCADE via the OCCTSwift family: 79 typed tools + execute_script. remote_theme: just-the-docs/just-the-docs@v0.3.3 # just-the-docs options diff --git a/docs/guides/architecture.md b/docs/guides/architecture.md index d4fa975..a2da716 100644 --- a/docs/guides/architecture.md +++ b/docs/guides/architecture.md @@ -14,11 +14,11 @@ OCCTMCP is an MCP server that lets an LLM author, inspect, and iterate on 3D CAD | | Swift (`occtmcp-server`) | Node (`dist/index.js`) | |---|---|---| | **Runtime** | macOS 15+, Swift in-process | Node 18+, any OS | -| **Tools** | 77 | 37 | +| **Tools** | 79 | 37 | | **OCCTSwift access** | Direct, no subprocess per call | Via `occtkit` CLI (shells out) | | **Build** | `swift build -c release` | `npm run build` | -The Swift server is the primary implementation. Because it calls OCCTSwift, OCCTSwiftTools, OCCTSwiftAIS, OCCTSwiftMesh, and DrawingComposer in-process, it can expose higher-level operations (selection, remap, annotations, reconstruction, history wiring) without serialising through a JSONL subprocess boundary. The Node server wraps `occtkit` verbs, so it only covers the 37 tools that map directly onto CLI commands. The 40 Swift-only tools are: the entire `select_*` / `remap_selection` / `find_correspondences` group, all annotation tools, `graph_select`, `pick_surface_point`, `ping`, the `reconstruct_*` group, and the mesh-zone analysis, mesh inspection, alignment, curvature, and mesh-feature tool groups (`segment_mesh_zones`, `zone_continuity_sweep`, `fit_primitives`, `mesh_diagnose`, `mesh_thickness`, `detect_symmetry`, `align_bodies`, `mesh_curvature`, `detect_mesh_features`, `fit_edge_chain`, `symmetric_difference_volume`, `measure_vertex_fit`, and their registry tools `list_zones`/`clear_zones`). +The Swift server is the primary implementation. Because it calls OCCTSwift, OCCTSwiftTools, OCCTSwiftAIS, OCCTSwiftMesh, and DrawingComposer in-process, it can expose higher-level operations (selection, remap, annotations, reconstruction, history wiring) without serialising through a JSONL subprocess boundary. The Node server wraps `occtkit` verbs, so it only covers the 37 tools that map directly onto CLI commands. The 42 Swift-only tools are: the entire `select_*` / `remap_selection` / `find_correspondences` group, `get_selection`/`highlight_selection` (the agent-to-viewport-host selection bridge, #189/#190), all annotation tools, `graph_select`, `pick_surface_point`, `ping`, the `reconstruct_*` group, and the mesh-zone analysis, mesh inspection, alignment, curvature, and mesh-feature tool groups (`segment_mesh_zones`, `zone_continuity_sweep`, `fit_primitives`, `mesh_diagnose`, `mesh_thickness`, `detect_symmetry`, `align_bodies`, `mesh_curvature`, `detect_mesh_features`, `fit_edge_chain`, `symmetric_difference_volume`, `measure_vertex_fit`, and their registry tools `list_zones`/`clear_zones`). See the [Tool Reference](../reference/) for the full per-tool listing; each entry notes which server(s) expose it. diff --git a/docs/guides/getting-started.md b/docs/guides/getting-started.md index 0e25f61..bc64f24 100644 --- a/docs/guides/getting-started.md +++ b/docs/guides/getting-started.md @@ -21,9 +21,10 @@ This page covers installing OCCTMCP, wiring it into an MCP client, and making yo [OCCTSwiftScripts](https://github.com/SecondMouseAU/OCCTSwiftScripts), or keep a sibling clone at `~/Projects/OCCTSwiftScripts` so OCCTMCP can fall back to `swift run -c release occtkit` automatically -The Node server exposes a 37-tool subset; the Swift server exposes all 77 tools (selection, remap, -annotations, reconstruction, mesh-zone analysis, mesh inspection, alignment, and more are -Swift-only). See the [Tool Reference](../reference/) for per-tool server availability. +The Node server exposes a 37-tool subset; the Swift server exposes all 79 tools (selection, remap, +annotations, reconstruction, mesh-zone analysis, mesh inspection, alignment, the viewport selection +bridge, and more are Swift-only). See the [Tool Reference](../reference/) for per-tool server +availability. --- diff --git a/docs/index.md b/docs/index.md index 4eb4cef..644bdc9 100644 --- a/docs/index.md +++ b/docs/index.md @@ -17,7 +17,7 @@ PNG + a `manifest.json` scene the viewer auto-reloads. Two interchangeable servers read/write the same scene: -- **Swift** (`occtmcp-server`) — the **primary**, in-process server. **77 typed tools.** macOS 15+. +- **Swift** (`occtmcp-server`) — the **primary**, in-process server. **79 typed tools.** macOS 15+. - **Node** (`dist/index.js`) — shells out to the `occtkit` CLI. **37-tool subset** (selection / remap / annotations / reconstruction are Swift-only). Runs anywhere Node 18+ does. diff --git a/docs/reference/README.md b/docs/reference/README.md index fd9ff02..2cd3ba9 100644 --- a/docs/reference/README.md +++ b/docs/reference/README.md @@ -12,7 +12,7 @@ with an example response, the underlying OCCTSwift / occtkit it drives, and gotc OCCTMCP is an **MCP server**, not a library: clients call these tools over stdio MCP, each with a single JSON-object argument, and get JSON text back. The **Swift** server (`occtmcp-server`) is the -canonical 75-tool surface documented here; the **Node** server exposes a 37-tool subset, and each tool +canonical 79-tool surface documented here; the **Node** server exposes a 37-tool subset, and each tool notes its Node availability. This complements the other docs: @@ -95,7 +95,7 @@ nav_order: | [Introspection & measurement](introspection.md) | validate_geometry, compute_metrics, query_topology, measure_distance, measure_deviation, recognize_features, inspect_assembly | | [Construction](construction.md) | apply_feature, transform_body, boolean_op, mirror_or_pattern | | [Engineering analysis](engineering.md) | check_thickness, analyze_clearance, heal_shape | -| [Selection & remap](selection.md) | select_topology, remap_selection, find_correspondences, select_by_feature, list_selections, clear_selections | +| [Selection & remap](selection.md) | select_topology, remap_selection, find_correspondences, select_by_feature, list_selections, clear_selections, get_selection, highlight_selection | | [Annotations & overlays](annotations.md) | add_dimension, add_scene_primitive, auto_dimension, show_bounding_box, diff_overlay, remove_scene_annotation, list_annotations | | [I/O](io.md) | read_brep, import_file, export_scene, set_assembly_metadata | | [Mesh & visualization](mesh-visualization.md) | generate_mesh, simplify_mesh, render_preview, pick_surface_point, generate_drawing | diff --git a/docs/reference/selection.md b/docs/reference/selection.md index 5672270..6cf36af 100644 --- a/docs/reference/selection.md +++ b/docs/reference/selection.md @@ -7,12 +7,20 @@ nav_order: 6 # Selection & remap These tools let an LLM pick faces, edges, or vertices on scene bodies and carry those picks forward -across mutations, transforms, and pattern instances. All six tools are **Swift only**: the Node +across mutations, transforms, and pattern instances. All eight tools are **Swift only**: the Node server does not expose them. +`get_selection`/`highlight_selection` (#189/#190) are a distinct pair within this family: instead of +picking topology in this server's own scene, they bridge to a *live viewport host* process (e.g. +ACADStudio) reading/writing sidecar files in the resolved output directory, per the wire format in +[`SecondMouseAU/OCCTSwiftInteraction#17`](https://github.com/SecondMouseAU/OCCTSwiftInteraction/issues/17). +No host implements the writer/watcher side yet as of this writing; both tools are host-agnostic by +construction and degrade to an explicit `noHost`/`timeout` result rather than hanging when nothing is +listening. + ## Tools -- [`select_topology`](#select_topology) · [`remap_selection`](#remap_selection) · [`find_correspondences`](#find_correspondences) · [`select_by_feature`](#select_by_feature) · [`list_selections`](#list_selections) · [`clear_selections`](#clear_selections) +- [`select_topology`](#select_topology) · [`remap_selection`](#remap_selection) · [`find_correspondences`](#find_correspondences) · [`select_by_feature`](#select_by_feature) · [`list_selections`](#list_selections) · [`clear_selections`](#clear_selections) · [`get_selection`](#get_selection) · [`highlight_selection`](#highlight_selection) --- @@ -298,3 +306,99 @@ discards, not just the anchor-scoped subset `list_selections` enumerates ([#150] // example result { "cleared": 3 } ``` + +--- + +## `get_selection` + +Read a live viewport host's current selection from `/selection.json` + `host.lock`, per +the wire format in `SecondMouseAU/OCCTSwiftInteraction#17`. + +**Server:** Swift only + +No parameters. + +**Returns:** A three-state result: `state: "noHost"` (`selections: null` — no viewport host is +running at all) vs `state: "hostRunning"` with `selections: []` (host live, nothing picked) or +`selections: [...]` (host live, N items picked). Never collapses those into one boolean/empty-array +reading. Each resolved selection carries `selectionId`, `bodyId`, `kind`, `index`, the host's own +`uid` (passed through as-is), an `anchor` snapshot (area/bounds/centroid for a face, length/curveType/ +endpoints for an edge, position for a vertex — resolved the same way `select_topology` resolves a +match), or an `error` string when that one entry couldn't be resolved (bad `bodyId`, out-of-range +`index`) without failing the rest of the response. A host that's running but whose `selection.json` is +missing or malformed (a torn read) is reported as an explicit tool error, never swallowed into an +empty selection list. + +**Example** + +```json +// tool call arguments +{} +``` +```json +// example result +{ + "state": "hostRunning", + "revision": 4, + "updatedAt": "2026-08-21T00:00:00Z", + "selections": [ + { + "selectionId": "sel:part#face[2]", + "bodyId": "part", + "kind": "face", + "index": 2, + "uid": "host-graphuid-string", + "anchor": { "center": [0.0, 0.0, 10.0], "area": 314.15, "surfaceType": "plane" }, + "error": null + } + ] +} +``` + +**Notes:** The resulting `selectionId`s compose with `remap_selection`/`find_correspondences`/ +`add_dimension`/`measure_distance` exactly like ones `select_topology` minted itself. The host's +`uid` is an opaque string from its own process-local `BRepGraph.GraphUID` and is never treated as a +uid this server's own registry could resolve. + +--- + +## `highlight_selection` + +Ask a live viewport host to highlight one sub-shape, by writing +`/highlight_requests/.json` and polling +`/highlight_requests/handled/.json` for the host's real outcome. + +**Server:** Swift only + +**Parameters** + +| name | type | required | description | +|------|------|:--------:|-------------| +| `bodyId` | string | yes | Body the highlighted sub-shape belongs to. | +| `kind` | string (`"body"` \| `"face"` \| `"edge"` \| `"vertex"`) | yes | Topological entity type. | +| `index` | integer | yes | Entity index (host-scoped; not validated against this server's own scene). | +| `scheme` | string (`"replace"` \| `"add"` \| `"remove"` \| `"xor"`) | yes | Mirrors `OCCTSwiftAIS.SelectionScheme` exactly. | +| `question` | string | no | Optional natural-language context for the host to show alongside the highlight. | +| `timeoutSeconds` | number | no | How long to poll `handled/.json` before returning `outcome: "timeout"`. Default `5.0`. | + +**Returns:** `{ "id": , "outcome": , "reason": }`. `outcome` +is the host's own `"applied"`/`"rejected"`/`"superseded"` (read from its `handled/.json`), or +`"timeout"` if nothing answers within the deadline, or `"noHost"` (with `id: null`, no request +written) if no viewport host is running at all. + +**Example** + +```json +// tool call arguments +{ "bodyId": "part", "kind": "face", "index": 2, "scheme": "replace" } +``` +```json +// example result +{ "id": "3f9b1c2a-...", "outcome": "applied", "reason": null } +``` + +**Notes:** `kind`/`scheme` are validated against their closed wire-format enums before anything is +written; a malformed request is never written. `bodyId`/`index` are written through UNVALIDATED +against the live scene (this tool has no other access to check them) — a bad reference still writes +the request and comes back as the host's own `rejected` outcome through the same poll, not a +client-side pre-check. `id` is generated by this tool, never supplied by the caller. diff --git a/okf/components/index.md b/okf/components/index.md index 5919d53..e4d9e06 100644 --- a/okf/components/index.md +++ b/okf/components/index.md @@ -3,7 +3,7 @@ type: component title: Components index resource: https://github.com/SecondMouseAU/OCCTMCP tags: [index, api, mcp-tools] -description: OCCTMCP products — OCCTMCPCore library, occtmcp-server executable, and the 77-tool catalogue. +description: OCCTMCP products — OCCTMCPCore library, occtmcp-server executable, and the 79-tool catalogue. timestamp: 2026-06-22 --- @@ -21,12 +21,13 @@ timestamp: 2026-06-22 A second, original **Node / TypeScript** implementation (`src/`, `package.json`) ships in the same repo (37 tools, shells out to the `occtkit` CLI) but is not a Swift product. -## MCP tool catalogue (77 typed tools) +## MCP tool catalogue (79 typed tools) The categorized table lives in the repo's own `README.md`, not duplicated here: keeping one copy is what stops the two from drifting apart as tools are added (the same duplication-drift problem #125/#134 fixed elsewhere in this codebase). See [README.md](https://github.com/SecondMouseAU/OCCTMCP#tools) for the current grouping (authoring, scene reads/mutation, introspection, construction, engineering analysis, -mesh analysis (zones, alignment, curvature, mesh features), selection & remap, annotations & -overlays, I/O, visualisation, topology graph, and the reconstruction graph tool group). +mesh analysis (zones, alignment, curvature, mesh features), selection & remap (including the +agent-to-viewport-host selection bridge, #189/#190), annotations & overlays, I/O, visualisation, +topology graph, and the reconstruction graph tool group). diff --git a/okf/index.md b/okf/index.md index 7889f40..65f7013 100644 --- a/okf/index.md +++ b/okf/index.md @@ -12,11 +12,12 @@ timestamp: 2026-06-22 An **MCP (Model Context Protocol) server** that gives LLMs the ability to author, inspect, and iterate on 3D CAD models with [OpenCASCADE](https://www.opencascade.com/) via the [OCCTSwift](https://github.com/SecondMouseAU/OCCTSwift) family. The primary Swift implementation -calls OCCT directly in-process (no subprocess, no JSONL marshalling) and exposes **77 typed MCP +calls OCCT directly in-process (no subprocess, no JSONL marshalling) and exposes **79 typed MCP tools** spanning authoring, scene reads/mutation, introspection, construction, analysis, I/O, -mesh, drawing, selection/remap, dimension overlays, and an attributed reconstruction graph. +mesh, drawing, selection/remap, dimension overlays, the agent-to-viewport-host selection bridge, +and an attributed reconstruction graph. -The repo ships two implementations side-by-side: the **Swift** server (primary, 77 tools, +The repo ships two implementations side-by-side: the **Swift** server (primary, 79 tools, macOS 15+) and the original **Node / TypeScript** server (37 tools, shells out to `occtkit`). ## Role in the ecosystem From 3d1969226e96898370951e51852a477795370392 Mon Sep 17 00:00:00 2001 From: gsdali <51393997+gsdali@users.noreply.github.com> Date: Fri, 21 Aug 2026 15:13:27 +1000 Subject: [PATCH 2/2] Address kilo-code-bot review on #191: flock errno, malformed handled/, cancellation Three review findings on SelectionBridgeTools.swift, all applied as real correctness/robustness fixes rather than dismissed: - HostLock.checkLiveness treated ANY flock failure as hostRunning, not just EWOULDBLOCK (the actual "something holds the exclusive lock" signal). A failure for another reason (locking unsupported on the filesystem, an interrupted call) now falls open to noHost, matching the file's own already-stated design philosophy for the open() failure case just above it. - readHandledOutcome silently treated a handled/.json that exists but fails to decode the same as "not there yet", so a malformed host response read as a plain timeout instead of surfacing that the host actually responded. Replaced with pollHandledOutcome returning a three-case HandledPoll (pending/decoded/malformed) so the poll loop reports outcome "error" immediately instead of waiting out the whole deadline. - The poll loop's try? await Task.sleep(...) swallowed CancellationError, so a cancelled call kept polling until the timeout instead of exiting early. Added an explicit Task.isCancelled check after each sleep, returning outcome "cancelled". Two new tests cover the malformed-handled-file and cancellation cases. The flock errno branch has no practical way to force a non-EWOULDBLOCK failure through the public API in a portable test, so it's covered by review/inspection rather than a new test; the existing HostLock coverage (held lock -> hostRunning, no lock -> noHost) is unaffected and still passes. swift build + swift test: 234/234 passing (232 previous + 2 new). Co-Authored-By: Claude Sonnet 5 --- .../Tools/SelectionBridgeTools.swift | 88 +++++++++++++++---- .../SelectionBridgeToolsTests.swift | 72 +++++++++++++++ 2 files changed, 144 insertions(+), 16 deletions(-) diff --git a/Sources/OCCTMCPCore/Tools/SelectionBridgeTools.swift b/Sources/OCCTMCPCore/Tools/SelectionBridgeTools.swift index 671400a..750b26f 100644 --- a/Sources/OCCTMCPCore/Tools/SelectionBridgeTools.swift +++ b/Sources/OCCTMCPCore/Tools/SelectionBridgeTools.swift @@ -48,13 +48,16 @@ enum HostLiveness: Sendable, Equatable { /// /// Trying a non-blocking SHARED /// lock and seeing whether that succeeds is the documented check: success -/// means nothing holds the exclusive lock (no host), failure means -/// something does (a host is running). A missing lock file is the same as -/// no host, trivially: nothing can be holding a lock on a file that was -/// never created. An `open` failure for any other reason (permissions, a -/// TOCTOU race with a deletion) fails open to `.noHost` rather than +/// means nothing holds the exclusive lock (no host), failure with +/// `EWOULDBLOCK` means something does (a host is running). A missing lock +/// file is the same as no host, trivially: nothing can be holding a lock on +/// a file that was never created. An `open` failure for any other reason +/// (permissions, a TOCTOU race with a deletion), and a `flock` failure for +/// any reason OTHER than `EWOULDBLOCK` (locking unsupported on the +/// filesystem, an interrupted call), fails open to `.noHost` rather than /// claiming a positive "host running" signal this probe never actually -/// observed. +/// observed: only `EWOULDBLOCK` is evidence of a live exclusive holder, and +/// nothing else is treated as if it were. enum HostLock { static func checkLiveness(outputDir: String) -> HostLiveness { let path = "\(outputDir)/host.lock" @@ -70,7 +73,7 @@ enum HostLock { flock(fd, LOCK_UN) return .noHost } - return .hostRunning + return (errno == EWOULDBLOCK) ? .hostRunning : .noHost } } @@ -383,7 +386,9 @@ public enum SelectionBridgeTools { public let id: String? /// "applied" | "rejected" | "superseded" (from the host's handled/ /// file) | "timeout" (no handled/ file within the deadline) | - /// "noHost" (no viewport host is running at all). + /// "noHost" (no viewport host is running at all) | "error" (a + /// handled/ file exists but did not decode) | "cancelled" (the + /// request was cancelled while waiting for a response). public let outcome: String public let reason: String? } @@ -466,14 +471,40 @@ public enum SelectionBridgeTools { let handledPath = "\(handledDir)/\(id).json" let deadline = Date().addingTimeInterval(timeoutSeconds) while Date() < deadline { - if let outcome = readHandledOutcome(path: handledPath) { + switch pollHandledOutcome(path: handledPath) { + case .decoded(let outcome): return IntrospectionTools.encode( HighlightSelectionResult( id: id, outcome: outcome.outcome, reason: outcome.reason) ) + case .malformed(let reason): + // The host DID respond, just not readably: report that + // immediately rather than waiting out the rest of the + // timeout for a response that has already arrived. + return IntrospectionTools.encode( + HighlightSelectionResult( + id: id, outcome: "error", + reason: + "highlight_requests/handled/\(id).json exists but did not decode: \(reason)" + )) + case .pending: + break } + // `Task.sleep` throws `CancellationError` when the ambient task + // is cancelled (e.g. the MCP client disconnected); `try?` + // swallows that so cancellation doesn't propagate as an error, + // but `Task.isCancelled` still reports it, so check explicitly + // rather than sleeping out the rest of the deadline for a caller + // that has already gone away. try? await Task.sleep( nanoseconds: UInt64(max(pollIntervalSeconds, 0.001) * 1_000_000_000)) + if Task.isCancelled { + return IntrospectionTools.encode( + HighlightSelectionResult( + id: id, outcome: "cancelled", + reason: "The request was cancelled before a response arrived." + )) + } } return IntrospectionTools.encode( @@ -484,13 +515,38 @@ public enum SelectionBridgeTools { )) } - static func readHandledOutcome(path: String) -> HandledOutcome? { - guard FileManager.default.fileExists(atPath: path), - let data = try? Data(contentsOf: URL(fileURLWithPath: path)), - let decoded = try? JSONDecoder().decode(HandledOutcome.self, from: data) - else { - return nil + /// One poll of `highlight_requests/handled/.json`. + /// + /// Distinguishes "nothing there yet" (`.pending`, keep polling) from "the + /// host wrote something that doesn't decode" (`.malformed`, worth + /// surfacing immediately): the ADR's atomic-write rule (temp name, then + /// `rename(2)`) means a fully-renamed file should always read cleanly, so + /// a decode failure here is a genuine anomaly (a host bug, or a + /// filesystem where rename isn't truly atomic), not a timing window. + /// Reporting it as `.pending` and waiting out the whole timeout would + /// hide that the host DID respond, just not readably. + enum HandledPoll { + case pending + case decoded(HandledOutcome) + case malformed(String) + } + + static func pollHandledOutcome(path: String) -> HandledPoll { + guard FileManager.default.fileExists(atPath: path) else { + return .pending + } + guard let data = try? Data(contentsOf: URL(fileURLWithPath: path)) else { + // A transient read failure (e.g. racing a rename that hasn't + // landed yet on some filesystem) is treated the same as "not + // there yet", not as malformed: the file existing but being + // unreadable for a moment is a timing window, not evidence the + // host wrote bad content. + return .pending + } + do { + return .decoded(try JSONDecoder().decode(HandledOutcome.self, from: data)) + } catch { + return .malformed(error.localizedDescription) } - return decoded } } diff --git a/SwiftTests/OCCTMCPCoreTests/SelectionBridgeToolsTests.swift b/SwiftTests/OCCTMCPCoreTests/SelectionBridgeToolsTests.swift index 638c93c..50e6fdf 100644 --- a/SwiftTests/OCCTMCPCoreTests/SelectionBridgeToolsTests.swift +++ b/SwiftTests/OCCTMCPCoreTests/SelectionBridgeToolsTests.swift @@ -417,4 +417,76 @@ struct SelectionBridgeToolsTests { #expect(written.scheme == "xor") #expect(written.question == "is this the right vertex?") } + + // ── review follow-ups: malformed handled/, cancellation ──────────────── + + @Test( + "highlight_selection: a handled/.json that exists but doesn't decode reports outcome=error immediately, not timeout" + ) + func highlightMalformedHandledFileReportsErrorNotTimeout() async throws { + let store = try scene([]) + let dir = dirOf(store) + defer { try? FileManager.default.removeItem(atPath: dir) } + let lock = try #require(HeldLock(path: "\(dir)/host.lock")) + defer { lock.release() } + + // A long timeout: if the malformed file were mistaken for "nothing + // there yet" this test would have to wait the whole thing out to + // observe the wrong "timeout" outcome. Asserting `outcome == "error"` + // (rather than the wall-clock the call took) is what actually proves + // it didn't fall through to the timeout branch. + async let resultTask = SelectionBridgeTools.highlightSelection( + bodyId: "box", kind: "face", index: 0, scheme: "replace", store: store, + timeoutSeconds: 30.0, pollIntervalSeconds: 0.02) + + let requestsDir = "\(dir)/highlight_requests" + var requestId: String? + for _ in 0..<200 { + if let files = try? FileManager.default.contentsOfDirectory(atPath: requestsDir), + let match = files.first(where: { $0.hasSuffix(".json") }) + { + requestId = String(match.dropLast(".json".count)) + break + } + try await Task.sleep(nanoseconds: 15_000_000) + } + let id = try #require(requestId) + + let handledDir = "\(requestsDir)/handled" + try FileManager.default.createDirectory(atPath: handledDir, withIntermediateDirectories: true) + try Data("{ not valid json".utf8).write( + to: URL(fileURLWithPath: "\(handledDir)/\(id).json"), options: .atomic) + + let result = await resultTask + #expect(!result.isError) + let r = try JSONDecoder().decode(HighlightResultMirror.self, from: Data(result.text.utf8)) + #expect(r.outcome == "error") + #expect(r.reason?.contains(id) == true) + } + + @Test("highlight_selection: cancelling the ambient task exits the poll loop instead of running to the deadline") + func highlightCancellationExitsPollLoopEarly() async throws { + let store = try scene([]) + let dir = dirOf(store) + defer { try? FileManager.default.removeItem(atPath: dir) } + let lock = try #require(HeldLock(path: "\(dir)/host.lock")) + defer { lock.release() } + + // A long timeout and a fast poll interval: nothing ever writes + // handled/.json, so the only way this returns before the 30s + // deadline is the cancellation check firing. + let task = Task { + await SelectionBridgeTools.highlightSelection( + bodyId: "box", kind: "face", index: 0, scheme: "replace", store: store, + timeoutSeconds: 30.0, pollIntervalSeconds: 0.02) + } + // Give it time to write the request and enter the poll loop at least + // once before cancelling. + try await Task.sleep(nanoseconds: 100_000_000) + task.cancel() + + let result = await task.value + let r = try JSONDecoder().decode(HighlightResultMirror.self, from: Data(result.text.utf8)) + #expect(r.outcome == "cancelled") + } }