Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions packages/client-runtime/src/state/review.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { EnvironmentId } from "@t3tools/contracts";
import { describe, expect, it } from "@effect/vitest";
import { Atom, AtomRegistry } from "effect/unstable/reactivity";

import { invalidateReviewDiffPreviews, reviewDiffPreviewRevisionAtom } from "./review.ts";

const TARGET = {
environmentId: EnvironmentId.make("environment-1"),
};

describe("review diff preview invalidation", () => {
it("scopes revision changes to the invalidated environment", () => {
const registry = AtomRegistry.make();
const otherEnvironment = {
environmentId: EnvironmentId.make("environment-2"),
};

expect(registry.get(reviewDiffPreviewRevisionAtom(TARGET))).toBe(0);
expect(registry.get(reviewDiffPreviewRevisionAtom(otherEnvironment))).toBe(0);

invalidateReviewDiffPreviews(registry, TARGET);

expect(registry.get(reviewDiffPreviewRevisionAtom(TARGET))).toBe(1);
expect(registry.get(reviewDiffPreviewRevisionAtom(otherEnvironment))).toBe(0);
registry.dispose();
});

it("notifies reactive dependents without replacing their atom identity", () => {
const registry = AtomRegistry.make();
const dependent = Atom.make((get) => get(reviewDiffPreviewRevisionAtom(TARGET)));
const unmount = registry.mount(dependent);

expect(registry.get(dependent)).toBe(0);
invalidateReviewDiffPreviews(registry, TARGET);
expect(registry.get(dependent)).toBe(1);

unmount();
registry.dispose();
});
});
27 changes: 25 additions & 2 deletions packages/client-runtime/src/state/review.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,31 @@
import { WS_METHODS } from "@t3tools/contracts";
import { Atom } from "effect/unstable/reactivity";
import { type EnvironmentId, WS_METHODS } from "@t3tools/contracts";
import { Atom, type AtomRegistry } from "effect/unstable/reactivity";

import { createEnvironmentRpcQueryAtomFamily } from "./runtime.ts";
import type { EnvironmentRegistry } from "../connection/registry.ts";

export interface ReviewDiffPreviewInvalidationTarget {
readonly environmentId: EnvironmentId;
}

const diffPreviewRevisionByEnvironment = Atom.family((environmentId: EnvironmentId) =>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Scope preview invalidation to the changed repository

When one environment contains multiple projects or worktrees, every post-snapshot status event increments this single environment-wide revision, so an event from any visible sidebar thread forces the active diff preview for an unrelated cwd to execute another potentially large Git diff and send it over the WebSocket. Web and mobile both mount additional status streams outside their review panels, making parallel agent activity in one environment a practical source of repeated unrelated preview requests; key the revision by normalized repository/CWD (and invalidate only matching preview inputs) instead.

AGENTS.md reference: AGENTS.md:L15-L17

Useful? React with 👍 / 👎.

Atom.make(0).pipe(
Atom.keepAlive,
Atom.withLabel(`environment-data:review:diff-preview-revision:${environmentId}`),
),
);

export function reviewDiffPreviewRevisionAtom(target: ReviewDiffPreviewInvalidationTarget) {
return diffPreviewRevisionByEnvironment(target.environmentId);
}

export function invalidateReviewDiffPreviews(
registry: AtomRegistry.AtomRegistry,
target: ReviewDiffPreviewInvalidationTarget,
): void {
registry.update(reviewDiffPreviewRevisionAtom(target), (revision) => revision + 1);
}

export function createReviewEnvironmentAtoms<R, E>(
runtime: Atom.AtomRuntime<EnvironmentRegistry | R, E>,
) {
Expand All @@ -12,6 +34,7 @@ export function createReviewEnvironmentAtoms<R, E>(
label: "environment-data:review:diff-preview",
tag: WS_METHODS.reviewGetDiffPreview,
staleTimeMs: 5_000,
refreshSignal: reviewDiffPreviewRevisionAtom,
}),
};
}
12 changes: 12 additions & 0 deletions packages/client-runtime/src/state/runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ interface EnvironmentQueryAtomOptions<Input, A, E, R> extends EnvironmentAtomOpt
readonly staleTimeMs?: number;
readonly idleTtlMs?: number;
readonly refreshIntervalMs?: number;
readonly refreshSignal?: (target: {
readonly environmentId: EnvironmentIdType;
readonly input: Input;
}) => Atom.Atom<unknown>;
}

interface EnvironmentSubscriptionAtomOptions<Input, A, E, R> {
Expand Down Expand Up @@ -488,6 +492,9 @@ function createEnvironmentQueryAtomFamily<R, ER, Input, A, E>(
const idleTtlMs = options.idleTtlMs ?? 5 * 60_000;
const queryAtom = runtime
.atom((get) => {
if (options.refreshSignal !== undefined) {
get(options.refreshSignal(target));
}
const generation = Option.getOrNull(
AsyncResult.value(get(rpcGenerationAtom(target.environmentId))),
);
Expand Down Expand Up @@ -576,6 +583,10 @@ export function createEnvironmentRpcQueryAtomFamily<R, ER, TTag extends Environm
readonly staleTimeMs?: number;
readonly idleTtlMs?: number;
readonly refreshIntervalMs?: number;
readonly refreshSignal?: (target: {
readonly environmentId: EnvironmentIdType;
readonly input: EnvironmentRpcInput<TTag>;
}) => Atom.Atom<unknown>;
},
) {
return createEnvironmentQueryAtomFamily(runtime, {
Expand All @@ -585,6 +596,7 @@ export function createEnvironmentRpcQueryAtomFamily<R, ER, TTag extends Environm
...(options.refreshIntervalMs === undefined
? {}
: { refreshIntervalMs: options.refreshIntervalMs }),
...(options.refreshSignal === undefined ? {} : { refreshSignal: options.refreshSignal }),
execute: (input: EnvironmentRpcInput<TTag>) => request(options.tag, input),
});
}
Expand Down
76 changes: 76 additions & 0 deletions packages/client-runtime/src/state/vcs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import {
WS_METHODS,
type VcsListRefsInput,
type VcsListRefsResult,
type VcsStatusLocalResult,
type VcsStatusRemoteResult,
} from "@t3tools/contracts";
import { describe, expect, it } from "@effect/vitest";
import * as Effect from "effect/Effect";
Expand Down Expand Up @@ -32,7 +34,9 @@ import {
commitVcsRefsRefresh,
createVcsEnvironmentAtoms,
makeCachedVcsRefsChanges,
projectVcsStatusChanges,
} from "./vcs.ts";
import { reviewDiffPreviewRevisionAtom } from "./review.ts";
import {
invalidateCachedVcsRefs,
invalidateVcsRefs,
Expand Down Expand Up @@ -82,6 +86,36 @@ const LIVE_REFS: VcsListRefsResult = {
],
};

const LOCAL_STATUS_WITH_FILE: VcsStatusLocalResult = {
isRepo: true,
hasPrimaryRemote: true,
isDefaultRef: false,
refName: "feature",
hasWorkingTreeChanges: true,
workingTree: {
files: [{ path: "temporary.txt", insertions: 1, deletions: 0 }],
insertions: 1,
deletions: 0,
},
};

const LOCAL_STATUS_AFTER_DELETION: VcsStatusLocalResult = {
...LOCAL_STATUS_WITH_FILE,
hasWorkingTreeChanges: false,
workingTree: {
files: [],
insertions: 0,
deletions: 0,
},
};

const REMOTE_STATUS: VcsStatusRemoteResult = {
hasUpstream: true,
aheadCount: 0,
behindCount: 0,
pr: null,
};

function session(client: WsRpcProtocolClient): RpcSession {
return {
client,
Expand Down Expand Up @@ -114,6 +148,45 @@ function cacheWithRefs(
}

describe("cached VCS refs", () => {
it.effect(
"invalidates diff previews after a changed status event, not the initial snapshot",
() =>
Effect.gen(function* () {
const registry = yield* Effect.acquireRelease(Effect.sync(AtomRegistry.make), (registry) =>
Effect.sync(() => registry.dispose()),
);
const revisionAtom = reviewDiffPreviewRevisionAtom(TARGET);
const revisions: number[] = [];

const statuses = yield* projectVcsStatusChanges(
TARGET.environmentId,
Stream.make(
{
_tag: "snapshot" as const,
local: LOCAL_STATUS_WITH_FILE,
remote: REMOTE_STATUS,
},
{
_tag: "localUpdated" as const,
local: LOCAL_STATUS_AFTER_DELETION,
},
),
).pipe(
Stream.tap(() =>
Effect.sync(() => {
revisions.push(registry.get(revisionAtom));
}),
),
Stream.provideService(AtomRegistry.AtomRegistry, registry),
Stream.runCollect,
);

expect(statuses).toHaveLength(2);
expect(statuses[1]?.workingTree.files).toEqual([]);
expect(revisions).toEqual([0, 1]);
}),
);

it("invalidates all ref streams in the mutated environment", () => {
const registry = AtomRegistry.make();
const environment = {
Expand Down Expand Up @@ -260,6 +333,7 @@ describe("cached VCS refs", () => {
revision: 1,
persistedCacheReadable: true,
});
expect(registry.get(reviewDiffPreviewRevisionAtom(TARGET))).toBe(1);

expect(
yield* commitVcsRefsRefresh(registry, cache, {
Expand Down Expand Up @@ -329,6 +403,7 @@ describe("cached VCS refs", () => {
expect(AsyncResult.isFailure(result)).toBe(true);
expect(yield* Ref.get(clears)).toBe(1);
expect(registry.get(vcsRefsCacheStateAtom(TARGET)).revision).toBe(1);
expect(registry.get(reviewDiffPreviewRevisionAtom(TARGET))).toBe(1);

const refreshResult = yield* Effect.promise(() =>
atoms.refreshStatus.run(registry, {
Expand All @@ -340,6 +415,7 @@ describe("cached VCS refs", () => {
expect(AsyncResult.isSuccess(refreshResult)).toBe(true);
expect(yield* Ref.get(clears)).toBe(2);
expect(registry.get(vcsRefsCacheStateAtom(TARGET)).revision).toBe(2);
expect(registry.get(reviewDiffPreviewRevisionAtom(TARGET))).toBe(2);
}),
),
);
Expand Down
63 changes: 49 additions & 14 deletions packages/client-runtime/src/state/vcs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
type VcsListRefsInput,
type VcsListRefsResult,
type VcsStatusResult,
type VcsStatusStreamEvent,
WS_METHODS,
} from "@t3tools/contracts";
import { applyGitStatusStreamEvent } from "@t3tools/shared/git";
Expand All @@ -15,14 +16,15 @@ import * as Stream from "effect/Stream";
import * as SubscriptionRef from "effect/SubscriptionRef";
import { Atom, AtomRegistry } from "effect/unstable/reactivity";

import { createEnvironmentRpcCommand, createEnvironmentSubscriptionAtomFamily } from "./runtime.ts";
import { createEnvironmentRpcCommand } from "./runtime.ts";
import type { EnvironmentRegistry } from "../connection/registry.ts";
import { EnvironmentSupervisor } from "../connection/supervisor.ts";
import { safeErrorLogAttributes } from "../errors/safeLog.ts";
import { EnvironmentCacheStore } from "../platform/persistence.ts";
import { request, subscribe, type EnvironmentRpcInput } from "../rpc/client.ts";
import { followStreamInEnvironment } from "./runtime.ts";
import { vcsCommandConcurrency, vcsCommandScheduler } from "./vcsCommandScheduler.ts";
import { invalidateReviewDiffPreviews } from "./review.ts";
import {
invalidateCachedVcsRefs,
vcsRefsCacheStateAtom,
Expand Down Expand Up @@ -233,6 +235,34 @@ export function cachedVcsRefsChanges(
);
}

export function projectVcsStatusChanges<E, R>(
environmentId: EnvironmentId,
events: Stream.Stream<VcsStatusStreamEvent, E, R>,
) {
return events.pipe(
Stream.mapAccumEffect(
() => null as VcsStatusResult | null,
(current, event) => {
const next = applyGitStatusStreamEvent(current, event);
return Effect.gen(function* () {
if (current !== null) {
const registry = yield* AtomRegistry.AtomRegistry;
invalidateReviewDiffPreviews(registry, { environmentId });
Comment on lines +248 to +250

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Refresh previews when local status summaries stay unchanged

When a later turn edits an already-dirty file without changing its path or insertion/deletion counts, this invalidation never runs: CheckpointReactor refreshes local status after the turn, but VcsStatusBroadcaster.updateCachedLocalStatus publishes localUpdated only when the JSON fingerprint of the summary changes. The patch returned by review.getDiffPreview can therefore change while the mounted query keeps its old result indefinitely. Fresh evidence in this revision is that preview freshness now depends exclusively on receiving these status events, so invalidate after every checkpoint refresh or propagate a content generation even when the summary is identical.

Useful? React with 👍 / 👎.

Comment on lines +249 to +250

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Track fetched remote refs when invalidating previews

When the automatic status fetch moves a selected remote base such as origin/release without changing the current branch's divergence or PR fields, no remoteUpdated event is published and this invalidation is skipped, although ${baseRef}...HEAD now produces a different preview. The server invokes git fetch ... <remote> without a refspec (git fetch -h documents the repository-only form as git fetch [<repository> [<refspec>...]]), while VcsStatusBroadcaster fingerprints only VcsStatusRemoteResult, which contains no remote-ref generation. Fresh evidence in this revision is that the new event-driven cache signal has no other path for automatic remote fetches; propagate a ref generation/hash or invalidate after the fetch.

Useful? React with 👍 / 👎.

}
return [next, [next]] as const;
});
},
),
);
}

export function vcsStatusChanges(
environmentId: EnvironmentId,
input: EnvironmentRpcInput<typeof WS_METHODS.subscribeVcsStatus>,
) {
return projectVcsStatusChanges(environmentId, subscribe(WS_METHODS.subscribeVcsStatus, input));
}

export function createVcsEnvironmentAtoms<R, E>(
runtime: Atom.AtomRuntime<EnvironmentRegistry | EnvironmentCacheStore | R, E>,
) {
Expand All @@ -259,6 +289,23 @@ export function createVcsEnvironmentAtoms<R, E>(
readonly environmentId: EnvironmentId;
readonly input: VcsListRefsInput;
}) => listRefsByEnvironment(target.environmentId)(JSON.stringify(target.input));
const statusByEnvironment = Atom.family((environmentId: EnvironmentId) =>
Atom.family((inputKey: string) => {
const input = JSON.parse(inputKey) as EnvironmentRpcInput<
typeof WS_METHODS.subscribeVcsStatus
>;
return runtime
.atom(followStreamInEnvironment(environmentId, vcsStatusChanges(environmentId, input)))
.pipe(
Atom.setIdleTTL(5 * 60_000),
Atom.withLabel(`environment-data:vcs:status:${environmentId}:${inputKey}`),
);
}),
);
const status = (target: {
readonly environmentId: EnvironmentId;
readonly input: EnvironmentRpcInput<typeof WS_METHODS.subscribeVcsStatus>;
}) => statusByEnvironment(target.environmentId)(JSON.stringify(target.input));
const invalidateRefs = (
target: { readonly environmentId: EnvironmentId; readonly input: { readonly cwd: string } },
registry: AtomRegistry.AtomRegistry,
Expand All @@ -270,19 +317,7 @@ export function createVcsEnvironmentAtoms<R, E>(

return {
listRefs,
status: createEnvironmentSubscriptionAtomFamily(runtime, {
label: "environment-data:vcs:status",
subscribe: (input: EnvironmentRpcInput<typeof WS_METHODS.subscribeVcsStatus>) =>
subscribe(WS_METHODS.subscribeVcsStatus, input).pipe(
Stream.mapAccum(
() => null as VcsStatusResult | null,
(current, event) => {
const next = applyGitStatusStreamEvent(current, event);
return [next, [next]] as const;
},
),
),
}),
status,
pull: createEnvironmentRpcCommand(runtime, {
label: "environment-data:vcs:pull",
tag: WS_METHODS.vcsPull,
Expand Down
2 changes: 2 additions & 0 deletions packages/client-runtime/src/state/vcsRefInvalidation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { Atom, type AtomRegistry } from "effect/unstable/reactivity";

import { safeErrorLogAttributes } from "../errors/safeLog.ts";
import { EnvironmentCacheStore } from "../platform/persistence.ts";
import { invalidateReviewDiffPreviews } from "./review.ts";

export interface VcsRefsInvalidationTarget {
readonly environmentId: EnvironmentId;
Expand Down Expand Up @@ -78,6 +79,7 @@ export const invalidateCachedVcsRefs = Effect.fn("VcsRefsState.invalidateCached"
),
);
invalidateVcsRefs(registry, target, persistedCacheReadable);
invalidateReviewDiffPreviews(registry, target);
}),
);
});
Loading