Skip to content
Merged
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
29 changes: 15 additions & 14 deletions apps/server/src/git/GitManager.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import * as LookupResultCache from "./LookupResultCache.ts";
import * as Arr from "effect/Array";
import * as Cache from "effect/Cache";
import * as Context from "effect/Context";
Expand Down Expand Up @@ -66,6 +65,7 @@ import {
import * as ProjectSetupScriptRunner from "../project/ProjectSetupScriptRunner.ts";
import * as ProviderRegistry from "../provider/Services/ProviderRegistry.ts";
import { extractBranchNameFromRemoteRef } from "./remoteRefs.ts";
import { detachStackFrame } from "./detachStackFrame.ts";
import * as ServerSettings from "../serverSettings.ts";
import type { GitManagerServiceError } from "@t3tools/contracts";
import * as GitVcsDriver from "../vcs/GitVcsDriver.ts";
Expand Down Expand Up @@ -1093,7 +1093,7 @@ export const make = Effect.gen(function* () {
prLookupFailureStreakByKey.set(key, streak);
return prLookupFailureTtl(streak);
};
const prLookupCache = yield* LookupResultCache.make(
const prLookupCache = yield* Cache.makeWith(
(key: string) => {
const [
cwd = "",
Expand Down Expand Up @@ -1141,6 +1141,7 @@ export const make = Effect.gen(function* () {
},
},
);
const getPrLookup = (key: string) => detachStackFrame(Cache.get(prLookupCache, key));
// A transient lookup failure (rate limit, network blip) must not clear an
// already-known PR badge, so the last successful answer per branch sticks
// around as the fallback. Keep the resolved head context with it so a
Expand Down Expand Up @@ -1214,14 +1215,14 @@ export const make = Effect.gen(function* () {
const branchKey = `${cwd}\u0000${details.branch}`;
const cacheKey = prLookupCacheKey(cwd, details);
if (refreshMissingPullRequest) {
const cached = yield* prLookupCache
.getOption(cacheKey)
.pipe(Effect.orElseSucceed(() => Option.none()));
const cached = yield* Cache.getOption(prLookupCache, cacheKey).pipe(
Effect.orElseSucceed(() => Option.none()),
);
if (Option.isSome(cached) && cached.value.latest === null) {
yield* prLookupCache.invalidate(cacheKey);
yield* Cache.invalidate(prLookupCache, cacheKey);
}
}
return yield* prLookupCache.get(cacheKey).pipe(
return yield* getPrLookup(cacheKey).pipe(
Effect.map(({ latest, headContext }) => {
if (!latest) return { pr: null, headContext };
// On the default branch, only surface open PRs.
Expand Down Expand Up @@ -2226,12 +2227,12 @@ export const make = Effect.gen(function* () {
if (options?.refresh) {
// A completed turn can create a PR or reuse a merged PR's branch.
// Refresh successful answers, but keep failed lookups' retry backoff.
const cached = yield* prLookupCache
.getOption(cacheKey)
.pipe(Effect.orElseSucceed(() => Option.none()));
if (Option.isSome(cached)) yield* prLookupCache.invalidate(cacheKey);
const cached = yield* Cache.getOption(prLookupCache, cacheKey).pipe(
Effect.orElseSucceed(() => Option.none()),
);
if (Option.isSome(cached)) yield* Cache.invalidate(prLookupCache, cacheKey);
}
let cached = yield* prLookupCache.get(cacheKey);
let cached = yield* getPrLookup(cacheKey);
// The cached head context may have resolved on a different remote than
// the saved upstream: a branch tracking origin/main but pushed to a fork
// is looked up on the fork. Verify against the remote the lookup used.
Expand All @@ -2258,8 +2259,8 @@ export const make = Effect.gen(function* () {
});
}
if (!hasSameIdentity(cached.headContext, currentIdentity)) {
yield* prLookupCache.invalidate(cacheKey);
cached = yield* prLookupCache.get(cacheKey);
yield* Cache.invalidate(prLookupCache, cacheKey);
cached = yield* getPrLookup(cacheKey);
const refreshedIdentity = yield* resolvePrLookupRepositoryIdentity(
cacheCwd,
branch,
Expand Down
112 changes: 0 additions & 112 deletions apps/server/src/git/LookupResultCache.test.ts

This file was deleted.

90 changes: 0 additions & 90 deletions apps/server/src/git/LookupResultCache.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ import * as NodeUtil from "node:util";
import { expect, it } from "vite-plus/test";
const execFile = NodeUtil.promisify(NodeChildProcess.execFile);
it.each([false, true])(
"releases the traced caller snapshot while caching a result (failure=%s)",
"a detached cache lookup releases the traced caller snapshot (failure=%s)",
async (failure) => {
const fixture = NodeURL.fileURLToPath(
new URL("./testing/LookupRetention.fixture.mjs", import.meta.url),
new URL("./testing/StackRetention.fixture.mjs", import.meta.url),
);
const { stdout } = await execFile(process.execPath, [
"--expose-gc",
Expand Down
20 changes: 20 additions & 0 deletions apps/server/src/git/detachStackFrame.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import * as Effect from "effect/Effect";
import * as References from "effect/References";

const snapshotStack = (
frame: References.StackFrame | undefined,
): References.StackFrame | undefined => {
if (frame === undefined) return undefined;
const stack = frame.stack();
return { name: frame.name, stack: () => stack, parent: snapshotStack(frame.parent) };
};

/**
* Wrap a `Cache.get` so its lookup fiber inherits a materialized stack. The lazy
* caller frame would otherwise retain the caller's request snapshot for the TTL.
*/
export const detachStackFrame = <A, E, R>(effect: Effect.Effect<A, E, R>) =>
Effect.gen(function* () {
const frame = snapshotStack(yield* References.CurrentStackFrame);
return yield* Effect.provideService(effect, References.CurrentStackFrame, frame);
});
41 changes: 0 additions & 41 deletions apps/server/src/git/testing/LookupRetention.fixture.mjs

This file was deleted.

31 changes: 31 additions & 0 deletions apps/server/src/git/testing/StackRetention.fixture.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { Cache, Effect } from "effect";
import { detachStackFrame } from "../detachStackFrame.ts";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Medium testing/StackRetention.fixture.mjs:2

On Node 22.16/22.17, the spawned process exits while loading ../detachStackFrame.ts because it is invoked only with --expose-gc; those Node versions still require --experimental-strip-types for .ts imports, so the fixture never reaches its assertions. Add that flag to the child-process invocation or use a JavaScript fixture/built output.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/git/testing/StackRetention.fixture.mjs around line 2:

On Node 22.16/22.17, the spawned process exits while loading `../detachStackFrame.ts` because it is invoked only with `--expose-gc`; those Node versions still require `--experimental-strip-types` for `.ts` imports, so the fixture never reaches its assertions. Add that flag to the child-process invocation or use a JavaScript fixture/built output.

// `original` runs the bare Effect Cache to show the leak this helper prevents.
const original = process.argv.includes("original");
const failure = process.argv.includes("failure");
const lookup = () => (failure ? Effect.fail("unavailable") : Effect.succeed(1));
const cache = await Effect.runPromise(
Cache.makeWith(lookup, { capacity: 8, timeToLive: () => "1 hour" }),
);
const get = () => (original ? Cache.get(cache, "key") : detachStackFrame(Cache.get(cache, "key")));
let reference;
await Effect.runPromise(
Effect.gen(function* () {
const snapshot = { values: Array.from({ length: 250000 }, (_, i) => i) };
reference = new WeakRef(snapshot);
const request = Effect.fn("StackRetention.request")(function* () {
yield* Effect.exit(get());
return snapshot.values.length;
});
yield* request();
}),
);
for (let i = 0; i < 12; i++) {
await new Promise((resolve) => setImmediate(resolve));
global.gc();
}
const retained = reference.deref() !== undefined;
const cachedResult = await Effect.runPromise(
get().pipe(Effect.catch(() => Effect.succeed("unavailable"))),
);
process.stdout.write(JSON.stringify({ retained, cachedResult }));
Loading