feat(mcp): add project management tools#8677

mcp/ProjectMcpService.ts:51

The service interface should be declared inline in the Context.Service call rather than kept as a standalone ProjectMcpServiceShape type (see the sibling WorktreeMcpService.ts). Consider inlining it and referring to the inferred interface as ProjectMcpService["Service"][...] at the five method annotations (lines 147, 160, 166, 239, 278).

No longer relevant as of 2cb24f5ca3d8df9579cf13b2be9d4a79c0883211

🟠 High mcp/ProjectMcpService.ts:197

A failed or overlapping projects.create leaves a successful clone unregistered, so retrying the same clientRequestId attempts cloneRepository again and fails because its destination is no longer empty. This breaks idempotent retries and prevents registering the already-cloned directory; reserve the project durably before cloning or make retries recover and register the existing destination.

No longer relevant as of 101793703084e93aaf8ee4312feaa22627dc20d9

🟡 Medium mcp/ProjectMcpService.ts:297

Retrying a cascaded delete with the same clientRequestId reports deletedThreadCount: 0 after the original response is lost, even though the original operation deleted threads. This already-deleted branch hardcodes the count instead of recovering the prior operation result; persist or otherwise replay the original affected count for idempotent retries.

🟠 High mcp/ProjectMcpService.ts:254

Repeated clientRequestId values across different projects make valid updates or deletes fail as operation_failed: the second command reuses the first project's command ID, and the orchestration engine rejects it for a different aggregate. Include input.projectId in the stable command ID for both update and delete operations.

Fixed in db8d3a6b6f. Stable update and delete command IDs now include the target project ID, with focused coverage for reusing the same clientRequestId across different projects.

🟠 High mcp/ProjectMcpService.ts:235

create returns operation_failed after the project has already been committed when projectView fails at line 235 because serverSettings.getSettings fails; update has the same issue at line 274. Clients may treat these failed mutations as no-ops and issue conflicting follow-up actions. Return the committed project or distinguish response-enrichment failures from mutation failures.

🟠 High mcp/ProjectMcpService.ts:107

createSemaphores retains a semaphore for every distinct projectId, so repeated create calls with unique client request IDs permanently grow this long-lived map—even when projects.create fails. Remove entries after the permit completes using race-safe cleanup so failed and completed requests do not accumulate unbounded state.

Fixed in 56cf046791. Both project-create and repository-clone serialization now use the existing makeKeyedSerialExecutor, whose reference-counted acquire/release removes idle keys safely after completed, failed, waiting, or interrupted users. This replaces both retained semaphore maps.

🟠 High sourceControl/SourceControlRepositoryService.ts:226

Concurrent clones addressed through a symlink and its real path acquire different semaphores, so both can observe the destination as absent and race git clone into the same directory; one request then fails. path.resolve at line 226 only normalizes syntax and does not canonicalize symlinks. Key the lock by a canonical destination path (including the canonicalized existing parent) before acquiring it.

Fixed in 56cf046791. Clone lock keys now canonicalize an existing destination or the nearest existing parent before appending missing segments, so symlink and physical aliases share one lock. The overlapping-clone test now races symlink and real-path requests and proves only one clone runs.

🟠 High mcp/ProjectMcpService.ts:436

project.delete can delete a project while a newly created live thread still references it, causing later thread operations to fail and violating the cascadeThreads/nonempty guarantee. The thread snapshot is taken before this delete command, so a concurrent ThreadLaunchService.launch can persist thread.create after the snapshot without rechecking the project; serialize deletion with thread creation or enforce the invariant transactionally.

🟠 High mcp/ProjectMcpService.ts:67

A clientRequestId containing a lone surrogate such as "\ud800" makes stablePart throw URIError, so project create/update/delete fails without returning a ProjectMcpFailure. The request schema accepts this value because it only trims and checks length; normalize the string to well-formed Unicode before calling encodeURIComponent.

🟡 Medium project/tools.ts:75

t3_project_update is advertised as non-destructive even though it can clear model, workspace-mode, and icon overrides or replace the scripts configuration, allowing MCP clients to present configuration removal as a safe update. Mark Tool.Destructive as true so the safety annotation reflects these destructive operations.

Create retry fails after deletion

Medium Severity

Create idempotency uses getById without includeDeleted. After a project is removed, a retry with the same clientRequestId treats the id as unused and dispatches project.create again. The V1 aggregate still occupies that id, so the retry surfaces as operation_failed instead of a clear replay or deleted-id result.

^{Reviewed by Cursor Bugbot for commit 56cf04679110c37bfd0f262e6a067dd352fb7abf. Configure here.}

Fixed in 01f3cf4051. Create preflight now reads the stable project ID with includeDeleted before settings, clone, or directory work. A deleted match returns typed project_deleted guidance to use a new clientRequestId and cannot resurrect the record. Focused create -> cascade delete -> same-key clone retry coverage proves clone runs once and the deleted projection remains deleted. I also pinned the underlying V1 behavior: replaying the accepted create command returns the current soft-deleted projection via its receipt and emits no new create event, so the filesystem guard belongs before ProjectService.create.

mcp/ProjectMcpService.test.ts:39

./ProjectMcpService.ts is imported twice: once named for the tag and once as a namespace. Consider dropping the named import and using the module's public shape consistently (ProjectMcp.ProjectMcpService at the tag use sites, ProjectMcp.layer for the layer).

No longer relevant as of 101793703084e93aaf8ee4312feaa22627dc20d9

Fixed in 1017937030. The test now keeps only the ProjectMcp namespace import and resolves every service tag through ProjectMcp.ProjectMcpService. The focused Project MCP suite passes (8/8), formatting and diff checks are clean.

At a service boundary the module should be imported as a namespace rather than aliasing its layer export, which erases the module namespace. It also collides here: line 424 declares a local const threadManagementLayer = Layer.mock(ThreadManagementService)({...}), so the same identifier means the real layer in one test and a mock in another.

Consider import * as ThreadManagement from "../orchestration-v2/ThreadManagementService.ts"; and using ThreadManagement.layer / ThreadManagement.ThreadManagementService at the use sites.

^{Posted via Macroscope — Effect Service Conventions}

Fixed in 6da97d32c2 (included in latest head 945b99f818). The test now imports ThreadManagementService only as the ThreadManagement namespace, uses ThreadManagement.layer and ThreadManagement.ThreadManagementService at every service boundary, and renames the local mock to blockedThreadManagementLayer. The focused Project MCP suite passes 8/8.

ProjectMcpFailure carries only code plus a free-form message, and every wrapper builds that message by stringifying the underlying failure (errorMessage(error)), so the message is derived from the cause while the cause itself is discarded. That also lets clone/dispatch failures leak remote URLs or git output into a caller-visible message.

Consider giving ProjectMcpFailure stable structural attributes (e.g. code, operation, projectId/workspaceRoot) plus a required cause, deriving message from those attributes only, and constructing the error at each failure boundary — which also removes the need for the failure(...) factory, whose only behavior is new ProjectMcpFailure({ ... }).

^{Posted via Macroscope — Effect Service Conventions}

🟡 Medium project/tools.ts:61

Repeating ProjectCreateTool with identical arguments creates additional project records (and can repeat a clone) when clientRequestId is omitted, so advertising .annotate(Tool.Idempotent, true) falsely claims that retries have no additional effect. Mark this tool non-idempotent unless creation deduplicates requests without that field.

Fixed in 725e31db1944d1a106c8331cda6252e08fa43cd7. ProjectCreateTool no longer advertises idempotency when clientRequestId is omitted; keyed retries retain the existing durable behavior.

🟡 Medium sourceControl/SourceControlRepositoryService.ts:323

Retries reject a completed clone as unrelated when Git is configured with a non-default clone.defaultRemoteName, because recovery always checks remote.origin.url and refs/remotes/origin. Force the clone remote name to origin so these checks match the repository that was just created.

Fixed in 725e31db1944d1a106c8331cda6252e08fa43cd7. Clone now explicitly uses --origin origin, matching the recovery checks. Focused clone recovery coverage runs with a non-default clone.defaultRemoteName and proves recovery still uses the created origin remote.

Verified the implementation now explicitly uses --origin origin, which resolves the reported issue. I could not find the claimed non-default clone.defaultRemoteName recovery test, though; the existing coverage only asserts the clone arguments.

🟡 Medium src/projectMcp.ts:94

ProjectMcpCreateInput accepts and persists non-image values such as .env for faviconPath, but the icon asset endpoint rejects them, leaving the created project with a broken favicon configuration. Use ProjectFaviconPath here (and in the update schema) instead of TrimmedNonEmptyString so the extension constraint is enforced.

🟠 High project/http.ts:92

Requests with createWorkspaceRootIfMissing: true are rejected when the workspace root does not exist, instead of creating the directory. projectMutationOperation drops this field before calling ProjectService.create, so the service receives its default false; forward the field in the project.create mapping.

Fixed in fe9b7c3790d08deecfc11faad3aa31013faf5247. The HTTP create mapping now forwards createWorkspaceRootIfMissing only when supplied, preserving omission, and its handler test covers true, false, and omitted. The focused lower suite passes 14/14 and the scoped server typecheck passes.

🟠 High project/http.ts:92

The HTTP mutate endpoint can delete a nonempty project while leaving its existing threads pointing at the deleted project. projectMutationOperation calls ProjectService.delete directly, bypassing withProjectMutationLock, active/archived thread checks, and force cleanup; route deletes through the same guarded mutation path as WebSocket mutations.

Verified against the rebased V2 base c01ff207: the HTTP V1 mutation route already called ProjectService.delete directly before this PR. This PR extracted that unchanged mapping only to test the newly forwarded project fields; its guarded cascade/admission contract is intentionally the new MCP/V2 project-delete path. Migrating the older HTTP auth/result contract is a separate inherited limitation and is explicitly documented in the handoff, so it is out of scope here.

Project list sorts timestamps as text

Medium Severity

t3_project_list orders projects with localeCompare on updatedAt. IsoDateTime is an unconstrained string, so mixed timezone offsets invert recency and shift cursor pages. Nearby thread listing already orders with DateTime.toEpochMillis.

^{Triggered by learned rule: Use parseTimestampDate for ISO string parsing, return null for invalid}

^{Reviewed by Cursor Bugbot for commit 4f8d2ea2119a51a8bcfc696e635f89804451599a. Configure here.}

ProjectFaviconPath now lives in project.ts, and this line is a compatibility re-export from its old home. The only remaining consumer of the old path is packages/contracts/src/assets.ts (import { ProjectFaviconPath } from "./orchestration.ts"); everything else reaches it through the barrel, which already re-exports project.ts.

Consider deleting this re-export and pointing assets.ts at the canonical module (import { ProjectFaviconPath } from "./project.ts";) so the moved declaration has a single import path.

^{Posted via Macroscope — Effect Service Conventions}

Keeping this re-export is intentional compatibility. ProjectFaviconPath was public from orchestration.ts on the V2 base, and the project contract move must not break existing direct consumers that are not visible to an in-repo grep. The canonical declaration is in project.ts and orchestration.ts only re-exports that same schema, so there is no duplicate implementation or divergent service shape. Removing the old import path would violate the rollout's old-client/source compatibility requirement.

🟠 High orchestration-v2/Orchestrator.ts:7281

When a thread.create command reaches ClientCommandDispatch with the orchestration runtime unavailable, withProjectCreationAdmission calls getCommandReceipt and the fiber terminates with an untyped defect instead of returning the structured OrchestratorDispatchError used by this layer's other unavailable operations. Implement this stub with the appropriate structured OrchestratorDispatchError failure rather than Effect.die.

✅ Resolved in 4e9c9a50cd3789059e364ff93f948b580525e0ac

Fixed in 4e9c9a50cd3789059e364ff93f948b580525e0ac. The unavailable orchestrator now returns CommandReceiptStoreReadError, and the receipt-aware thread.create admission boundary maps that failure to OrchestratorDispatchError with the original command ID and type. The actual ClientCommandDispatch test proves the WebSocket creation path returns the structured error instead of a defect.

🟠 High mcp/ProjectMcpService.ts:65

redactOperationFailure writes the original error to warning logs before mapping it to the public ProjectMcpFailure. On the clone path, SourceControlRepositoryError.detail can contain credential-bearing remote URLs, so OAuth tokens are exposed in server logs even though the returned MCP error is redacted. Log only non-sensitive operation metadata here, or sanitize the error before logging.

🟡 Medium project/ProjectService.ts:416

A concurrent duplicate delete returns ProjectNotFoundError instead of the required alreadyDeleted result when the first delete completes while the second call waits for withProjectLock. This guard rejects the now-deleted projection before the stable commandId can replay the accepted project.delete command; handle an already-deleted project as an idempotent replay (while still returning ProjectNotFoundError for a genuinely missing project).

Fixed in 87026840af41cd93d7e836d1204a284ca37eeac3. If deleteDetailed loses an overlapping deletion and returns typed ProjectNotFoundError, the MCP boundary reloads includeDeleted. Only an explicit deleted projection becomes alreadyDeleted:true with a per-attempt count of zero; genuine absence remains project_not_found and reload failures remain finite operation errors. The gated MCP regression proves one fresh count and one already-deleted result.

🟡 Medium sourceControl/SourceControlRepositoryService.ts:236

maxOutputBytes: 64 * 1024 truncates for-each-ref output for repositories with roughly 1,600 or more origin branches, so remoteCommits.has(headCommit) returns false when the matching ref falls beyond the retained prefix and a completed clone is rejected on retry. Remove this fixed truncation or verify the commit without relying on a capped list of refs.

Fixed in 87026840af41cd93d7e836d1204a284ca37eeac3. Recovery now asks Git for at most one origin ref pointing at the checked-out HEAD and requires the returned OID to match exactly, using a 1 KiB cap. The real-Git regression creates 1,700 unrelated origin refs and a lexically last matching ref, proves the former 64 KiB inventory overflows, then verifies conservative recovery succeeds without fetch or reachability expansion.

🟡 Medium mcp/OrchestratorMcpService.ts:1454

A thread created after waiting for the project lock can be recorded against the completed parentRun.id and parentNodeId, so the active run's timeline does not show the new thread. The admission recheck validates a fresh active run, but thread.created.record later uses the stale values captured before the lock; use the run validated at admission (or revalidate immediately before recording) for the parent linkage.

🟡 Medium sourceControl/SourceControlRepositoryService.ts:225

isMatchingClone returns false for a successfully cloned empty repository, so retrying an empty remote reports the occupied destination as unrelated. The early headCommit === null return rejects unborn HEAD before the clone's clean checkout can be validated; defer commit/ref checks and allow a clean repository with no remote refs.

Delete retry can miss alreadyDeleted

Low Severity

The alreadyDeleted short-circuit reads the project before the mutation lock, while a concurrent deleteDetailed that then sees deletedAt is mapped to project_not_found. Overlapping deletes or a retry that races the first commit can return project_not_found instead of alreadyDeleted: true.

^{Reviewed by Cursor Bugbot for commit 001fdc890fc460492bed5aec2eae18f5b8d72d8f. Configure here.}

Fixed in 87026840af41cd93d7e836d1204a284ca37eeac3 via the same typed overlap recovery. The losing delete confirms an explicit deleted projection and returns alreadyDeleted:true/deletedThreadCount:0; it does not blanket-convert ProjectNotFoundError, so a genuinely absent project still returns project_not_found.