refactor(server): commit project commands without the V1 engine - #13896
juliusmarminge wants to merge 1 commit into
Conversation
ProjectService was the last caller of the V1 OrchestrationEngine, which hydrated the full V1 read model at boot and kept it for the process lifetime. Project commands now validate, append the same project.* events, write their receipt and project the projection_projects row in one transaction, then publish to shell subscribers. Readers that went through the V1 ProjectionSnapshotQuery (project shells, workspace lookups, replay stats, thread search) now read the project repository or a standalone thread-search query, and the runtime no longer builds the V1 engine, projection pipeline or snapshot query. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
vouch:trusted
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| ).pipe(Effect.orElseSucceed(() => Option.none<ProjectId>())); | ||
| ? Option.isNone(threadProjections) | ||
| ? none() | ||
| : yield* threadProjections.value.getThreadShell(input.threadId).pipe( |
There was a problem hiding this comment.
🟡 Medium git/GitManager.ts:726
Actions for existing V1 threads use global settings when the thread has not yet been lazily imported, so project-specific source-control writer settings are ignored. getThreadShell returns no thread in this case, and this lookup never calls ensureLegacyTranscript; resolve the legacy thread before reading its projectId (or retain the previous V1-aware lookup).
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/git/GitManager.ts around line 726:
Actions for existing V1 threads use global settings when the thread has not yet been lazily imported, so project-specific source-control writer settings are ignored. `getThreadShell` returns no thread in this case, and this lookup never calls `ensureLegacyTranscript`; resolve the legacy thread before reading its `projectId` (or retain the previous V1-aware lookup).
| @@ -469,13 +558,8 @@ export const make = Effect.gen(function* () { | |||
| { concurrency: 1, discard: true }, | |||
| ); | |||
| return yield* dispatch( | |||
There was a problem hiding this comment.
🟠 High project/ProjectService.ts:560
The project is soft-deleted while an active V2 thread can still reference it, leaving an orphaned runnable thread. ThreadLaunchService.launch can read the project before the snapshot and commit the thread before project.delete is committed, but projectCommandRejection accepts the delete without a final active-thread check; recheck active threads in the delete transaction and reject unless force is set.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/project/ProjectService.ts around line 560:
The project is soft-deleted while an active V2 thread can still reference it, leaving an orphaned runnable thread. `ThreadLaunchService.launch` can read the project before the snapshot and commit the thread before `project.delete` is committed, but `projectCommandRejection` accepts the delete without a final active-thread check; recheck active threads in the delete transaction and reject unless `force` is set.
| defaultModelSelection: input.defaultModelSelection ?? null, | ||
| scripts: [...(input.scripts ?? [])], | ||
| createdAt: now, | ||
| scripts: input.scripts ?? [], |
There was a problem hiding this comment.
🟠 High project/ProjectService.ts:394
Creating a project with input.defaultModelSelection silently stores defaultModelSelection: null, so the requested default model is lost. The project.create command omits this field and planProjectEvent applies the null default; include input.defaultModelSelection in the command payload.
| scripts: input.scripts ?? [], | |
| defaultModelSelection: input.defaultModelSelection ?? null, | |
| scripts: input.scripts ?? [], |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/project/ProjectService.ts around line 394:
Creating a project with `input.defaultModelSelection` silently stores `defaultModelSelection: null`, so the requested default model is lost. The `project.create` command omits this field and `planProjectEvent` applies the null default; include `input.defaultModelSelection` in the command payload.
| } from "../orchestration-v2/ThreadCommandExecutor.ts"; | ||
| import { planThreadDeletion } from "../orchestration-v2/ThreadDeletion.ts"; | ||
| import { OrchestrationCommandReceiptRepository } from "../persistence/Services/OrchestrationCommandReceipts.ts"; | ||
| import { OrchestrationEventStore } from "../persistence/Services/OrchestrationEventStore.ts"; |
There was a problem hiding this comment.
These imports consume Effect services at a service boundary but import their tags by name. Could you import each local service module as a namespace and access its tag through that namespace (for example, OrchestrationEventStore.OrchestrationEventStore)?
Posted via Macroscope — Effect Service Conventions
| Effect.map((rows) => | ||
| rows.flatMap((row) => (row.deletedAt === null ? [toProjectShell(row)] : [])), | ||
| ), | ||
| ); |
There was a problem hiding this comment.
These production helpers accept ProjectionProjectRepository["Service"] (and, below, ProjectEnrichmentService["Service"]) as arguments, so their Effect dependencies disappear from the environment. Could you acquire the services with yield* ProjectionProjectRepository.ProjectionProjectRepository and yield* ProjectEnrichmentService.ProjectEnrichmentService inside the Effects, and let callers provide them through layers? The same applies to the other new project-shell helpers in this file.
Posted via Macroscope — Effect Service Conventions
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR replaces the project-command engine and rewires many production read, persistence, worker, and WebSocket paths, including a changed project default-model behavior. Unresolved findings also identify incorrect V1 settings resolution and a project-deletion race affecting active threads. Not approved because:
Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: unavailable · PR result: Scenario and decoded snapshot size10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
This branch has not been deployed
1 participant
The V2 runtime still built V1's
OrchestrationLayerLivefor one caller:ProjectServicedispatchedproject.create/project.meta.update/project.deletethrough the V1OrchestrationEngine. That engine hydrated the whole V1 in-memory read model at boot (getCommandReadModel) and kept it for the process lifetime. The V1ProjectionSnapshotQuerywas also still used for project shells and a few reads.First layer of removing the V1 engine: port the remaining callers so nothing at runtime builds the engine, projection pipeline or snapshot query. The next layer deletes the V1 decider, projector, pipeline and snapshot query.
What changed
project/ProjectService.ts, newproject/ProjectEvents.ts). In one SQL transaction the service checks the command receipt, re-checks the workspace-root conflict, validates the command, appends the event, writes the receipt and projects theprojection_projectsrow with a pureprojectProjectEvent. After commit it publishes the event to shell subscribers. Event types and payloads are the existingproject.created/project.meta-updated/project.deleted(ApplicationProjectEvent), so rows written by V1, the 055 baseline and this code decode the same way. The validation the V1 decider did (existence, active workspace-root collision, script-ID shortcut rule with legacy IDs still editable, monogram ≤ 2 graphemes) moved with it, and so did idempotent retries by command ID, including durable rejections.project/ProjectShells.ts). They readProjectionProjectRepositorydirectly, which V2 already uses, instead of a new read model. Covered: ws/http shell snapshots and project deltas, startup auto-pull, storage cleanup, auto-pull policy, per-project git settings, PR list/link/sync, thread settlement, the agent-session scanner and the MCP PR tools. Enrichment reads only immediately available identity, as before.orchestration-v2/ThreadSearch.ts. It still covers V1 threads whose transcripts have not been imported, so the V1 message tables stay read-only import input.ws.ts.OrchestrationEventInfrastructureLayerLive(event store + receipts) moved toorchestration-v2/runtimeLayer.ts.server.tsprovidesProjectionProjectRepositoryLivewhere it used to provideOrchestrationInfrastructureLayerLive.orchestration/Layers/OrchestrationEngine.ts,orchestration/Services/OrchestrationEngine.ts,orchestration/runtimeLayer.ts. Also deleted: the threet3_orchestration_command*metrics, which only the V1 engine recorded, and their section indocs/operations/observability.md.The wire contract is unchanged: no contract edits, and web/mobile untouched.
One behavior difference: V1 project deletion also wrote
deleted_aton the legacyprojection_threadsrow. That row is now import input only. The imported V2 thread is deleted through the V2 command as before, and the importer skips it, soProjectService.deletion.test.tsnow asserts on the V2 row.Verification
vp exec tsc --noEmit -p .in apps/server, apps/web, apps/mobile, packages/client-runtime, packages/contracts: 0error TS/warning TS.vp run knip:check: clean.vp linton touched files: no new warnings.vp test runin small groups, all passing:ProjectService.test.ts(3 new cases: idempotent retry and no rollback from a late retry, script-ID rules with durable rejection, monogram limit; mutation-checked by disabling the receipt check and the monogram rule, and both fail), newProjectEvents.test.ts(ports the V1 decider's project-default, env-mode/autoPull clear and delete cases),ProjectService.deletion.test.ts,http.test.ts,ProjectMutation.test.ts,AgentSessionScanner.test.tslinkCreatedPullRequest.test.ts, MCPpullRequests/handlers.test.ts,McpHttpServer.test.ts,worktree/registration.test.ts,ThreadPullRequestService.test.ts,ThreadSettlementService.test.ts,PullRequestService.test.ts,PullRequestSyncReactor.test.tsruntimeLayer.test.ts,DelegatedCompletionDelivery.test.ts,serverRuntimeStartup.test.ts,ws.test.ts,http.test.ts,cli/project.test.tsLegacyV1Cutover.integration.test.ts,LegacyV1ThreadImporter.test.ts,055_*(3 files),056_*OrchestratorReplayFixtures.integration.test.ts+.contract.test.ts(98 tests)--expose-gcon aVACUUM INTOcopy of a real V2 DB (29 threads, 5.7k events, 3 projects) and sampled 40 s after listening, after forced GC. Two runs each: base 175.4 / 175.4 MiB heapUsed; this layer 172.7 / 172.7 MiB. That DB has already been imported, so its V1 read model was small. The saving grows with the size of the V1 tables in a user's DB. The layer-2 PR will carry final numbers.Model: Claude Opus 5.5 (Claude Code)
🤖 Generated with Claude Code