History

Fix CI regressions after branch sync

Julius Marminge committed 5 months ago

Commit 3ee6fbb1 parent 6b482889 files · +181 −26Browse files

Unified · Split

9 files
  1. apps/server/src/server.test.ts +18 −0
  2. apps/web/src/components/chat/MessagesTimeline.tsx +73 −0
  3. apps/web/src/environments/remote/api.test.ts +22 −1
  4. apps/web/src/environments/runtime/connection.test.ts +1 −0
  5. apps/web/src/environments/runtime/service.threadSubscriptions.test.ts +42 −2
  6. apps/web/src/rpc/wsTransport.test.ts +4 −9
  7. packages/client-runtime/src/wsTransport.test.ts +6 −6
  8. packages/client-runtime/src/wsTransport.ts +14 −8
  9. packages/contracts/src/terminal.test.ts +1 −0
apps/server/src/server.test.ts+18−0
@@ -2602,6 +2602,9 @@ it.layer(NodeServices.layer)("server router seam", (it) => { · 2601 unmodified lines
26022602 cwd: "/tmp/repo",
26032603 },
26042604 layers: {
2605+ vcsDriver: {
2606+ isInsideWorkTree: () => Effect.succeed(true),
2607+ },
26052608 gitManager: {
26062609 invalidateLocalStatus: () => Effect.void,
26072610 invalidateRemoteStatus: () => Effect.void,
@@ -2738,6 +2741,21 @@ it.layer(NodeServices.layer)("server router seam", (it) => { · 130 unmodified lines
27382741 createRef: (input) => Effect.succeed({ refName: input.refName }),
27392742 switchRef: (input) => Effect.succeed({ refName: input.refName }),
27402743 },
2744+ vcsStatusBroadcaster: {
2745+ refreshStatus: () =>
2746+ Effect.succeed({
2747+ isRepo: true,
2748+ hasPrimaryRemote: true,
2749+ isDefaultRef: true,
2750+ refName: "main",
2751+ hasWorkingTreeChanges: false,
2752+ workingTree: { files: [], insertions: 0, deletions: 0 },
2753+ hasUpstream: true,
2754+ aheadCount: 0,
2755+ behindCount: 0,
2756+ pr: null,
2757+ }),
2758+ },
27412759 reviewService: {
27422760 getDiffPreview: (input) =>
27432761 Effect.succeed({
apps/web/src/components/chat/MessagesTimeline.tsx+73−0
@@ -16,9 +16,15 @@ import { · 15 unmodified lines
1616 type ReactNode,
1717 } from "react";
1818 import { LegendList, type LegendListRef } from "@legendapp/list/react";
19+import { FileDiff } from "@pierre/diffs/react";
1920 import { deriveTimelineEntries, formatElapsed } from "../../session-logic";
2021 import { type TurnDiffSummary } from "../../types";
2122 import { summarizeTurnDiffStats } from "../../lib/turnDiffTree";
23+import {
24+ getRenderablePatch,
25+ resolveDiffThemeName,
26+ resolveFileDiffPath,
27+} from "../../lib/diffRendering";
2228 import ChatMarkdown from "../ChatMarkdown";
2329 import {
2430 BotIcon,
@@ -67,6 +73,11 @@ import { · 42 unmodified lines
6773 } from "./userMessageTerminalContexts";
6874 import { SkillInlineText } from "./SkillInlineText";
6975 import { formatWorkspaceRelativePath } from "../../filePathDisplay";
76+import {
77+ buildReviewCommentRenderablePatch,
78+ parseReviewCommentMessageSegments,
79+ type ReviewCommentContext,
80+} from "../../reviewCommentContext";
7081
7182 // ---------------------------------------------------------------------------
7283 // Context — shared state consumed by every row component via Context.
@@ -837,6 +848,25 @@ const UserMessageBody = memo(function UserMessageBody(props: { · 764 unmodified lines
837848 terminalContexts: ParsedTerminalContextEntry[];
838849 skills: ReadonlyArray<Pick<ServerProviderSkill, "name" | "displayName">>;
839850 }) {
851+ const reviewCommentSegments = parseReviewCommentMessageSegments(props.text);
852+ if (reviewCommentSegments.some((segment) => segment.kind === "review-comment")) {
853+ return (
854+ <div className="space-y-3 text-sm leading-relaxed text-foreground">
855+ {reviewCommentSegments.map((segment) =>
856+ segment.kind === "text" ? (
857+ segment.text.trim().length > 0 ? (
858+ <div key={segment.id} className="whitespace-pre-wrap wrap-break-word">
859+ <SkillInlineText text={segment.text.trim()} skills={props.skills} />
860+ </div>
861+ ) : null
862+ ) : (
863+ <UserMessageReviewCommentCard key={segment.comment.id} comment={segment.comment} />
864+ ),
865+ )}
866+ </div>
867+ );
868+ }
869+
840870 if (props.terminalContexts.length > 0) {
841871 const hasEmbeddedInlineLabels = textContainsInlineTerminalContextLabels(
842872 props.text,
@@ -930,6 +960,49 @@ const UserMessageBody = memo(function UserMessageBody(props: { · 87 unmodified lines
930960 );
931961 });
932962
963+function UserMessageReviewCommentCard({ comment }: { comment: ReviewCommentContext }) {
964+ const ctx = use(TimelineRowCtx);
965+ const renderablePatch = getRenderablePatch(
966+ buildReviewCommentRenderablePatch(comment),
967+ `review-comment:${comment.id}`,
968+ );
969+
970+ return (
971+ <div className="space-y-2 rounded-lg border border-border/70 bg-background/70 p-3">
972+ <div className="space-y-1">
973+ <div className="text-xs font-medium text-foreground">
974+ {formatWorkspaceRelativePath(comment.filePath, ctx.workspaceRoot)}
975+ </div>
976+ <div className="text-[11px] text-muted-foreground">
977+ {comment.sectionTitle} · {comment.rangeLabel}
978+ </div>
979+ </div>
980+ {comment.text.length > 0 && (
981+ <div className="whitespace-pre-wrap wrap-break-word text-sm">
982+ <SkillInlineText text={comment.text} skills={ctx.skills} />
983+ </div>
984+ )}
985+ {renderablePatch?.kind === "files" &&
986+ renderablePatch.files.map((fileDiff) => (
987+ <FileDiff
988+ key={resolveFileDiffPath(fileDiff)}
989+ fileDiff={fileDiff}
990+ options={{
991+ collapsed: false,
992+ diffStyle: "unified",
993+ theme: resolveDiffThemeName(ctx.resolvedTheme),
994+ }}
995+ />
996+ ))}
997+ {renderablePatch?.kind === "raw" && (
998+ <pre className="overflow-x-auto rounded-md bg-muted/40 p-2 text-xs">
999+ {renderablePatch.text}
1000+ </pre>
1001+ )}
1002+ </div>
1003+ );
1004+}
1005+
9331006 // ---------------------------------------------------------------------------
9341007 // Structural sharing — reuse old row references when data hasn't changed
9351008 // so LegendList (and React) can skip re-rendering unchanged items.
apps/web/src/environments/remote/api.test.ts+22−1
@@ -239,7 +239,28 @@ describe("remote environment api", () => { · 238 unmodified lines
239239 httpBaseUrl: "https://remote.example.com/",
240240 bearerToken: "bearer-token",
241241 }),
242− ).resolves.toBe("wss://remote.example.com/ws?wsToken=ws-token");
242+ ).resolves.toBe("wss://remote.example.com/?wsToken=ws-token");
243+ });
244+
245+ it("preserves the caller-provided websocket path when minting a token", async () => {
246+ const fetchMock = vi.fn().mockResolvedValue(
247+ new Response(
248+ JSON.stringify({
249+ token: "ws-token",
250+ expiresAt: "2026-05-01T12:05:00.000Z",
251+ }),
252+ { status: 200 },
253+ ),
254+ );
255+ globalThis.fetch = fetchMock as typeof fetch;
256+
257+ await expect(
258+ resolveRemoteWebSocketConnectionUrl({
259+ wsBaseUrl: "wss://remote.example.com/custom/socket?existing=1",
260+ httpBaseUrl: "https://remote.example.com/",
261+ bearerToken: "bearer-token",
262+ }),
263+ ).resolves.toBe("wss://remote.example.com/custom/socket?existing=1&wsToken=ws-token");
243264 });
244265 });
245266
apps/web/src/environments/runtime/connection.test.ts+1−0
@@ -72,6 +72,7 @@ function createTestClient() { · 71 unmodified lines
7272 clear: vi.fn(async () => undefined),
7373 restart: vi.fn(async () => undefined),
7474 close: vi.fn(async () => undefined),
75+ onEvent: vi.fn(() => () => undefined),
7576 onMetadata: vi.fn(() => () => undefined),
7677 },
7778 projects: {
apps/web/src/environments/runtime/service.threadSubscriptions.test.ts+42−2
@@ -269,6 +269,17 @@ describe("retainThreadDetailSubscription", () => { · 268 unmodified lines
269269 mockCreateEnvironmentConnection.mockImplementation((input) => {
270270 const reconnect = vi.fn(async () => undefined);
271271 mockConnectionReconnects.push(reconnect);
272+ queueMicrotask(() => {
273+ input.onConfigSnapshot?.({
274+ environment: {
275+ environmentId: input.knownEnvironment.environmentId,
276+ label: input.knownEnvironment.label,
277+ platform: { os: "darwin", arch: "arm64" },
278+ serverVersion: "0.0.0-test",
279+ capabilities: { repositoryIdentity: true },
280+ },
281+ });
282+ });
272283 return {
273284 kind: input.kind,
274285 environmentId: input.knownEnvironment.environmentId,
@@ -438,13 +449,13 @@ describe("retainThreadDetailSubscription", () => { · 163 unmodified lines
438449 const stop = startEnvironmentConnectionService(new QueryClient());
439450 savedEnvironmentRegistryListener?.();
440451 await vi.waitFor(() => {
441− expect(mockCreateEnvironmentConnection).toHaveBeenCalledTimes(2);
442452 expect(
443453 listEnvironmentConnections().some(
444454 (connection) => connection.environmentId === environmentId,
445455 ),
446456 ).toBe(true);
447457 });
458+ const createConnectionCallsBeforeReconnect = mockCreateEnvironmentConnection.mock.calls.length;
448459
449460 const release = retainThreadDetailSubscription(environmentId, threadId);
450461 expect(mockSubscribeThread).toHaveBeenCalledTimes(1);
@@ -459,7 +470,9 @@ describe("retainThreadDetailSubscription", () => { · 8 unmodified lines
459470 await vi.advanceTimersByTimeAsync(200);
460471 await reconnectPromise;
461472 await vi.waitFor(() => {
462− expect(mockCreateEnvironmentConnection).toHaveBeenCalledTimes(3);
473+ expect(mockCreateEnvironmentConnection).toHaveBeenCalledTimes(
474+ createConnectionCallsBeforeReconnect + 1,
475+ );
463476 expect(mockSubscribeThread).toHaveBeenCalledTimes(2);
464477 });
465478
@@ -486,6 +499,33 @@ describe("retainThreadDetailSubscription", () => { · 20 unmodified lines
486499
487500 const { resetEnvironmentServiceForTests, startEnvironmentConnectionService } =
488501 await import("./service");
502+ mockCreateEnvironmentConnection.mockImplementation((input) => {
503+ const reconnect = vi.fn(async () => undefined);
504+ mockConnectionReconnects.push(reconnect);
505+ queueMicrotask(() => {
506+ input.onConfigSnapshot?.({
507+ environment: {
508+ environmentId: input.knownEnvironment.environmentId,
509+ label: input.knownEnvironment.label,
510+ platform: { os: "darwin", arch: "arm64" },
511+ serverVersion: "0.0.0-test",
512+ capabilities: { repositoryIdentity: true },
513+ },
514+ });
515+ });
516+ return {
517+ kind: input.kind,
518+ environmentId: input.knownEnvironment.environmentId,
519+ knownEnvironment: input.knownEnvironment,
520+ client: {
521+ ...input.client,
522+ isHeartbeatFresh: vi.fn(() => true),
523+ },
524+ ensureBootstrapped: vi.fn(async () => undefined),
525+ reconnect,
526+ dispose: vi.fn(async () => undefined),
527+ };
528+ });
489529
490530 const stop = startEnvironmentConnectionService(new QueryClient());
491531 expect(mockConnectionReconnects).toHaveLength(1);
apps/web/src/rpc/wsTransport.test.ts+4−9
@@ -242,15 +242,10 @@ describe("WsTransport (web instrumentation)", () => { · 241 unmodified lines
242242 socket.close(1012, "service restart");
243243
244244 await waitFor(() => {
245− expect(onClose).toHaveBeenCalledWith(
246− {
247− code: 1012,
248− reason: "service restart",
249− },
250− {
251− intentional: false,
252− },
253− );
245+ expect(onClose).toHaveBeenCalledWith({
246+ code: 1012,
247+ reason: "service restart",
248+ });
254249 expect(getWsConnectionStatus()).toMatchObject({
255250 attemptCount: 2,
256251 closeReason: "service restart",
packages/client-runtime/src/wsTransport.test.ts+6−6
@@ -645,8 +645,8 @@ describe("WsTransport", () => { · 644 unmodified lines
645645 });
646646
647647 it("does not retry stream subscriptions after application-level failures", async () => {
648− const transport = createTransport("ws://localhost:3020");
649− const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => undefined);
648+ const warnSpy = vi.fn();
649+ const transport = createTransport("ws://localhost:3020", undefined, { logWarning: warnSpy });
650650 let attempts = 0;
651651
652652 const unsubscribe = transport.subscribe(
@@ -684,8 +684,8 @@ describe("WsTransport", () => { · 31 unmodified lines
684684 });
685685
686686 it("keeps retrying stream subscriptions after transport failures", async () => {
687− const transport = createTransport("ws://localhost:3020");
688− const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => undefined);
687+ const warnSpy = vi.fn();
688+ const transport = createTransport("ws://localhost:3020", undefined, { logWarning: warnSpy });
689689 let attempts = 0;
690690
691691 const unsubscribe = transport.subscribe(
@@ -717,8 +717,8 @@ describe("WsTransport", () => { · 25 unmodified lines
717717 });
718718
719719 it("logs a transport disconnect once even when multiple subscriptions fail together", async () => {
720− const transport = createTransport("ws://localhost:3020");
721− const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => undefined);
720+ const warnSpy = vi.fn();
721+ const transport = createTransport("ws://localhost:3020", undefined, { logWarning: warnSpy });
722722
723723 const unsubscribeA = transport.subscribe(
724724 () => Stream.fail(new Error("SocketCloseError: 1006")),
packages/client-runtime/src/wsTransport.ts+14−8
@@ -31,6 +31,7 @@ export interface WsTransportOptions { · 30 unmodified lines
3131 url: WsRpcProtocolSocketUrlProvider,
3232 lifecycleHandlers?: WsProtocolLifecycleHandlers,
3333 ) => Layer.Layer<RpcClient.Protocol, never, never>;
34+ readonly logWarning?: (message: string, metadata: { readonly error: string }) => void;
3435 /**
3536 * Invoked at the start of {@link WsTransport.reconnect} before the session is replaced.
3637 */
@@ -172,18 +173,14 @@ export class WsTransport { · 135 unmodified lines
172173
173174 const formattedError = formatErrorMessage(error);
174175 if (!isTransportConnectionErrorMessage(formattedError)) {
175− Effect.runSync(
176− Effect.logWarning("WebSocket RPC subscription failed", { error: formattedError }),
177− );
176+ this.logWarning("WebSocket RPC subscription failed", { error: formattedError });
178177 return;
179178 }
180179
181180 if (!this.hasReportedTransportDisconnect) {
182− Effect.runSync(
183− Effect.logWarning("WebSocket RPC subscription disconnected", {
184− error: formattedError,
185− }),
186− );
181+ this.logWarning("WebSocket RPC subscription disconnected", {
182+ error: formattedError,
183+ });
187184 }
188185 this.hasReportedTransportDisconnect = true;
189186 await sleep(retryDelayMs);
@@ -252,6 +249,15 @@ export class WsTransport { · 62 unmodified lines
252249 };
253250 }
254251
252+ private logWarning(message: string, metadata: { readonly error: string }) {
253+ const logWarning = this.options?.logWarning;
254+ if (logWarning) {
255+ logWarning(message, metadata);
256+ } else {
257+ Effect.runSync(Effect.logWarning(message, metadata));
258+ }
259+ }
260+
255261 private runStreamOnSession<TValue>(
256262 session: TransportSession,
257263 connect: (client: WsRpcProtocolClient) => Stream.Stream<TValue, Error, never>,
packages/contracts/src/terminal.test.ts+1−0
@@ -44,6 +44,7 @@ describe("TerminalOpenInput", () => { · 43 unmodified lines
4444 expect(
4545 decodes(TerminalOpenInput, {
4646 threadId: "thread-1",
47+ terminalId: DEFAULT_TERMINAL_ID,
4748 cwd: "/tmp/project",
4849 cols: 423,
4950 rows: 40,