From 71ea1c59e64fb767299b4c088729b575f170cfc2 Mon Sep 17 00:00:00 2001 From: Val Alexander Date: Mon, 13 Apr 2026 09:36:49 -0500 Subject: [PATCH 1/2] Extract PR review history schema - Share recent review entry type in contracts - Trim PR review query results and update blocked-step messaging - Remove unused Copilot setup config and adjust snapshot sync test --- apps/server/src/prReview/Layers/PrReview.ts | 2 +- .../src/components/chat/ProviderSetupCard.tsx | 5 ----- .../src/components/pr-review/PrReviewShell.tsx | 4 +++- apps/web/src/lib/snapshotSyncManager.test.ts | 14 ++++++-------- packages/contracts/src/prReview.ts | 17 +++++++++-------- 5 files changed, 19 insertions(+), 23 deletions(-) diff --git a/apps/server/src/prReview/Layers/PrReview.ts b/apps/server/src/prReview/Layers/PrReview.ts index 12c0d052f..177fda8f9 100644 --- a/apps/server/src/prReview/Layers/PrReview.ts +++ b/apps/server/src/prReview/Layers/PrReview.ts @@ -61,7 +61,7 @@ query PullRequestReviewDashboard($owner: String!, $name: String!, $number: Int!) headRefName baseRefOid headRefOid - reviews(last: 100) { + reviews(last: 10) { nodes { state body diff --git a/apps/web/src/components/chat/ProviderSetupCard.tsx b/apps/web/src/components/chat/ProviderSetupCard.tsx index 62b4b19aa..811a0fa1e 100644 --- a/apps/web/src/components/chat/ProviderSetupCard.tsx +++ b/apps/web/src/components/chat/ProviderSetupCard.tsx @@ -37,11 +37,6 @@ const PROVIDER_CONFIG = { verifyCmd: "gh auth status", note: undefined, }, - copilot: { - installCmd: "npm install -g @github/copilot", - authCmd: "copilot login", - verifyCmd: "gh auth status", - }, } as const; function StatusIcon({ status }: { status: ServerProviderStatus["status"] }) { diff --git a/apps/web/src/components/pr-review/PrReviewShell.tsx b/apps/web/src/components/pr-review/PrReviewShell.tsx index 3c67d2969..6d6c726f5 100644 --- a/apps/web/src/components/pr-review/PrReviewShell.tsx +++ b/apps/web/src/components/pr-review/PrReviewShell.tsx @@ -410,7 +410,9 @@ export function PrReviewShell({ ...(conflictQuery.data?.status === "conflicted" ? ["Merge conflicts must be resolved"] : []), ...checksSummary.failing.map((name) => `Failing check: ${name}`), ...checksSummary.pending.map((name) => `Pending check: ${name}`), - ...blockingWorkflowStepsComputed.map((step) => `Workflow blocked: ${step.title}`), + ...blockingWorkflowStepsComputed.map( + (step) => `Workflow blocked: ${step.detail ?? step.stepId}`, + ), ]; const approveDisabled = submitReviewMutation.isPending || diff --git a/apps/web/src/lib/snapshotSyncManager.test.ts b/apps/web/src/lib/snapshotSyncManager.test.ts index d2ed06e9e..a7c06b48b 100644 --- a/apps/web/src/lib/snapshotSyncManager.test.ts +++ b/apps/web/src/lib/snapshotSyncManager.test.ts @@ -47,15 +47,13 @@ describe("createSnapshotSyncManager", () => { }); it("coalesces overlapping sync requests and reruns once after success", async () => { - let resolveFetch: ((snapshot: OrchestrationReadModel) => void) | null = null; + let releaseFirstFetch!: (snapshot: OrchestrationReadModel) => void; + const firstFetchPromise = new Promise((resolve) => { + releaseFirstFetch = resolve; + }); const fetchSnapshot = vi .fn<() => Promise>() - .mockImplementation( - () => - new Promise((resolve) => { - resolveFetch = resolve; - }), - ) + .mockImplementationOnce(() => firstFetchPromise) .mockResolvedValueOnce(makeSnapshot(2)); const applySnapshot = vi.fn(); const manager = createSnapshotSyncManager({ @@ -69,7 +67,7 @@ describe("createSnapshotSyncManager", () => { expect(fetchSnapshot).toHaveBeenCalledTimes(1); expect(firstSync).toBe(secondSync); - resolveFetch?.(makeSnapshot(1)); + releaseFirstFetch(makeSnapshot(1)); await firstSync; await Promise.resolve(); diff --git a/packages/contracts/src/prReview.ts b/packages/contracts/src/prReview.ts index aaf1e4b36..c169dba92 100644 --- a/packages/contracts/src/prReview.ts +++ b/packages/contracts/src/prReview.ts @@ -68,6 +68,14 @@ export const PrReviewStatusCheck = Schema.Struct({ }); export type PrReviewStatusCheck = typeof PrReviewStatusCheck.Type; +export const PrReviewHistoryEntry = Schema.Struct({ + authorLogin: TrimmedNonEmptyString, + state: TrimmedNonEmptyString, + body: Schema.String, + submittedAt: Schema.String, +}); +export type PrReviewHistoryEntry = typeof PrReviewHistoryEntry.Type; + export const PrReviewComment = Schema.Struct({ id: TrimmedNonEmptyString, databaseId: Schema.NullOr(PositiveInt), @@ -227,14 +235,7 @@ export const PrReviewSummary = Schema.Struct({ statusChecks: Schema.Array(PrReviewStatusCheck), participants: Schema.Array(PrReviewParticipant), reviewRequests: Schema.Array(PrReviewParticipant), - recentReviews: Schema.Array( - Schema.Struct({ - authorLogin: TrimmedNonEmptyString, - state: TrimmedNonEmptyString, - body: Schema.String, - submittedAt: Schema.String, - }), - ), + recentReviews: Schema.Array(PrReviewHistoryEntry), totalThreadCount: NonNegativeInt, unresolvedThreadCount: NonNegativeInt, headSha: Schema.NullOr(TrimmedNonEmptyString), From d8e761e604e0224931ae02e99b84988da3d717e5 Mon Sep 17 00:00:00 2001 From: Val Alexander Date: Mon, 13 Apr 2026 09:38:53 -0500 Subject: [PATCH 2/2] Fix provider runtime event feed typecheck coverage --- .../OrchestrationEngineHarness.integration.ts | 2 ++ .../integration/providerService.integration.test.ts | 2 ++ .../src/orchestration/Layers/CheckpointReactor.test.ts | 10 ++++++---- .../Layers/ProviderRuntimeIngestion.test.ts | 2 +- .../provider/Layers/ProviderRuntimeEventFeed.test.ts | 9 +++++---- .../src/provider/Layers/ProviderRuntimeEventFeed.ts | 6 ++++-- 6 files changed, 20 insertions(+), 11 deletions(-) diff --git a/apps/server/integration/OrchestrationEngineHarness.integration.ts b/apps/server/integration/OrchestrationEngineHarness.integration.ts index b59bfa6de..b8f94d22c 100644 --- a/apps/server/integration/OrchestrationEngineHarness.integration.ts +++ b/apps/server/integration/OrchestrationEngineHarness.integration.ts @@ -38,6 +38,7 @@ import { ProjectionPendingApprovalRepository } from "../src/persistence/Services import { EnvironmentVariablesLive } from "../src/persistence/Services/EnvironmentVariables.ts"; import { ProviderUnsupportedError } from "../src/provider/Errors.ts"; import { ProviderAdapterRegistry } from "../src/provider/Services/ProviderAdapterRegistry.ts"; +import { ProviderRuntimeEventFeedLive } from "../src/provider/Layers/ProviderRuntimeEventFeed.ts"; import { ProviderSessionDirectoryLive } from "../src/provider/Layers/ProviderSessionDirectory.ts"; import { makeProviderServiceLive } from "../src/provider/Layers/ProviderService.ts"; import { makeCodexAdapterLive } from "../src/provider/Layers/CodexAdapter.ts"; @@ -298,6 +299,7 @@ export const makeOrchestrationIntegrationHarness = ( Layer.provideMerge(ProjectionPendingApprovalRepositoryLive), Layer.provideMerge(checkpointStoreLayer), Layer.provideMerge(providerLayer), + Layer.provideMerge(ProviderRuntimeEventFeedLive), Layer.provideMerge(RuntimeReceiptBusLive), ); const runtimeIngestionLayer = ProviderRuntimeIngestionLive.pipe( diff --git a/apps/server/integration/providerService.integration.test.ts b/apps/server/integration/providerService.integration.test.ts index 7576896ba..73ae2f629 100644 --- a/apps/server/integration/providerService.integration.test.ts +++ b/apps/server/integration/providerService.integration.test.ts @@ -6,6 +6,7 @@ import { Effect, FileSystem, Layer, Path, Queue, Stream } from "effect"; import { ProviderUnsupportedError } from "../src/provider/Errors.ts"; import { ProviderAdapterRegistry } from "../src/provider/Services/ProviderAdapterRegistry.ts"; +import { ProviderRuntimeEventFeedLive } from "../src/provider/Layers/ProviderRuntimeEventFeed.ts"; import { ProviderSessionDirectoryLive } from "../src/provider/Layers/ProviderSessionDirectory.ts"; import { makeProviderServiceLive } from "../src/provider/Layers/ProviderService.ts"; import { @@ -59,6 +60,7 @@ const makeIntegrationFixture = Effect.gen(function* () { const shared = Layer.mergeAll( directoryLayer, Layer.succeed(ProviderAdapterRegistry, registry), + ProviderRuntimeEventFeedLive, ).pipe(Layer.provide(SqlitePersistenceMemory)); const layer = makeProviderServiceLive().pipe(Layer.provide(shared)); diff --git a/apps/server/src/orchestration/Layers/CheckpointReactor.test.ts b/apps/server/src/orchestration/Layers/CheckpointReactor.test.ts index c7fad3350..3a1cfa6e8 100644 --- a/apps/server/src/orchestration/Layers/CheckpointReactor.test.ts +++ b/apps/server/src/orchestration/Layers/CheckpointReactor.test.ts @@ -209,7 +209,7 @@ async function waitForGitRefExists(cwd: string, ref: string, timeoutMs = 15_000) describe("CheckpointReactor", () => { let runtime: ManagedRuntime.ManagedRuntime< - OrchestrationEngineService | CheckpointReactor | CheckpointStore, + OrchestrationEngineService | CheckpointReactor | CheckpointStore | ProviderRuntimeEventFeed, unknown > | null = null; let scope: Scope.Closeable | null = null; @@ -331,10 +331,12 @@ describe("CheckpointReactor", () => { return { engine, - provider, + provider: { + ...provider, + emit: (event: LegacyProviderRuntimeEvent) => + Effect.runSync(eventFeed.publish(event as unknown as ProviderRuntimeEvent)), + }, cwd, - emit: (event: LegacyProviderRuntimeEvent) => - Effect.runSync(eventFeed.publish(event as unknown as ProviderRuntimeEvent)), drain, }; } diff --git a/apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.test.ts b/apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.test.ts index 3697399fc..e006881a2 100644 --- a/apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.test.ts +++ b/apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.test.ts @@ -125,7 +125,7 @@ type ProviderRuntimeTestCheckpoint = ProviderRuntimeTestThread["checkpoints"][nu describe("ProviderRuntimeIngestion", () => { let runtime: ManagedRuntime.ManagedRuntime< - OrchestrationEngineService | ProviderRuntimeIngestionService, + OrchestrationEngineService | ProviderRuntimeIngestionService | ProviderRuntimeEventFeed, unknown > | null = null; let scope: Scope.Closeable | null = null; diff --git a/apps/server/src/provider/Layers/ProviderRuntimeEventFeed.test.ts b/apps/server/src/provider/Layers/ProviderRuntimeEventFeed.test.ts index 207ee2288..b8c6f17ea 100644 --- a/apps/server/src/provider/Layers/ProviderRuntimeEventFeed.test.ts +++ b/apps/server/src/provider/Layers/ProviderRuntimeEventFeed.test.ts @@ -1,7 +1,7 @@ import { EventId, ThreadId, TurnId, type ProviderRuntimeEvent } from "@okcode/contracts"; import { it } from "@effect/vitest"; import { describe, expect } from "vitest"; -import { Effect, Layer, Stream } from "effect"; +import { Effect, Fiber, Stream } from "effect"; import { ProviderRuntimeEventFeedLive } from "./ProviderRuntimeEventFeed.ts"; import { ProviderRuntimeEventFeed } from "../Services/ProviderRuntimeEventFeed.ts"; @@ -11,6 +11,7 @@ function makeTurnStartedEvent(id: string): ProviderRuntimeEvent { type: "turn.started", eventId: EventId.makeUnsafe(id), provider: "codex", + payload: {}, threadId: ThreadId.makeUnsafe("thread-1"), turnId: TurnId.makeUnsafe(`turn-${id}`), createdAt: "2026-01-01T00:00:00.000Z", @@ -27,17 +28,17 @@ describe("ProviderRuntimeEventFeedLive", () => { const events = yield* Stream.take(feed.subscribeWithReplay(), 3).pipe( Stream.runCollect, - Effect.fork, + Effect.forkScoped, ); yield* feed.publish(makeTurnStartedEvent("evt-3")); - const collected = yield* Effect.fromFiber(events); + const collected = yield* Fiber.join(events); expect(Array.from(collected).map((event) => event.eventId)).toEqual([ "evt-1", "evt-2", "evt-3", ]); - }).pipe(Effect.provide(Layer.mergeAll(ProviderRuntimeEventFeedLive))), + }).pipe(Effect.provide(ProviderRuntimeEventFeedLive)), ); }); diff --git a/apps/server/src/provider/Layers/ProviderRuntimeEventFeed.ts b/apps/server/src/provider/Layers/ProviderRuntimeEventFeed.ts index 341c308a5..0a4c7a81a 100644 --- a/apps/server/src/provider/Layers/ProviderRuntimeEventFeed.ts +++ b/apps/server/src/provider/Layers/ProviderRuntimeEventFeed.ts @@ -48,8 +48,9 @@ const makeProviderRuntimeEventFeed = Effect.gen(function* () { ); const subscribeWithReplay: ProviderRuntimeEventFeedShape["subscribeWithReplay"] = () => - Stream.unwrapScoped( + Stream.unwrap( Effect.gen(function* () { + const scope = yield* Effect.scope; const subscriber = yield* Queue.unbounded(); const replay = yield* mutex.withPermits(1)( Ref.modify(stateRef, (state) => { @@ -69,7 +70,8 @@ const makeProviderRuntimeEventFeed = Effect.gen(function* () { discard: true, }); - yield* Scope.addFinalizer(() => + yield* Scope.addFinalizer( + scope, mutex.withPermits(1)( Ref.update(stateRef, (state) => { const subscribers = new Set(state.subscribers);