From e720c63433ca578daeb9ebc1d0a2744a0e5df6ef Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Fri, 6 Nov 2026 00:38:04 -0800 Subject: [PATCH 1/2] fix: repair recently merged thread and cleanup regressions --- apps/mobile/src/state/entities.ts | 20 ++-- .../ThreadSplitSuggestions.test.ts | 103 +++++++++++++++++- .../ThreadSplitSuggestions.ts | 11 ++ apps/server/src/storageCleanup.test.ts | 67 +++++++++--- apps/server/src/storageCleanup.ts | 4 + .../components/chat/MarkdownThreadLink.tsx | 22 ++-- .../src/state/threadFamilies.test.ts | 91 +++++++++++++++- .../src/state/threadFamilies.ts | 57 +++++++++- .../src/server/modelEvaluation.test.ts | 63 +++++++++++ .../src/server/modelEvaluation.ts | 29 +++-- packages/shared/src/testing/providerEnv.ts | 2 + 11 files changed, 416 insertions(+), 53 deletions(-) diff --git a/apps/mobile/src/state/entities.ts b/apps/mobile/src/state/entities.ts index 41237b95c8a7c..e01611b5c596f 100644 --- a/apps/mobile/src/state/entities.ts +++ b/apps/mobile/src/state/entities.ts @@ -210,26 +210,22 @@ export function useSideChats(ref: ScopedThreadRef) { export function useLiveThreadLinkLabels(markdown: string, environmentId: EnvironmentId): string { const refs = hasThreadLinks(markdown) ? threadLinkRefs(markdown, environmentId) : []; + const refsKey = JSON.stringify(refs.map(threadKey)); const titles = useAtomValue( - refs.length === 0 - ? EMPTY_THREAD_LINK_TITLES_ATOM - : threadLinkTitlesAtom(JSON.stringify(refs.map(threadKey))), + refs.length === 0 ? EMPTY_THREAD_LINK_TITLES_ATOM : threadLinkTitlesAtom(refsKey), ); - // Shells leave out idle subagent children and older settled threads, so a cited thread the - // shell lacks is read with its family until it lands; only a settled empty read proves it gone. - const missingKey = JSON.stringify( - refs.filter((ref) => !titles.has(threadKey(ref))).map(threadKey), - ); - // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- mounts family reads for cited threads the shell omits while this message shows them + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- keeps cited rows mounted for the message's lifetime, independently of the family data it loads useEffect(() => { - const releases = (JSON.parse(missingKey) as ReadonlyArray).map((key) => { + const releases = decodeThreadKeys(JSON.parse(refsKey)).map((key) => { const ref = scopeThreadRef(...decodeThreadKey(JSON.parse(key))); - return threadFamilies.mount(appAtomRegistry, ref.environmentId, ref.threadId); + return threadFamilies.mount(appAtomRegistry, ref.environmentId, ref.threadId, { + onlyWhenMissing: true, + }); }); return () => { for (const release of releases) release(); }; - }, [missingKey]); + }, [refsKey]); return relabelThreadLinks(markdown, environmentId, (ref) => titles.get(threadKey(ref))); } diff --git a/apps/server/src/orchestration-v2/ThreadSplitSuggestions.test.ts b/apps/server/src/orchestration-v2/ThreadSplitSuggestions.test.ts index 567175badae28..5d943488782b0 100644 --- a/apps/server/src/orchestration-v2/ThreadSplitSuggestions.test.ts +++ b/apps/server/src/orchestration-v2/ThreadSplitSuggestions.test.ts @@ -4,15 +4,18 @@ import { ProjectId, ProviderInstanceId, MessageId, + RunId, DEFAULT_SERVER_SETTINGS, type OrchestrationV2AppThread, type OrchestrationV2ConversationMessage, + type OrchestrationV2Run, } from "@t3tools/contracts"; import * as DateTime from "effect/DateTime"; import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; import * as Deferred from "effect/Deferred"; import * as Fiber from "effect/Fiber"; +import * as Schema from "effect/Schema"; import * as TestClock from "effect/testing/TestClock"; import * as HttpClient from "effect/http/HttpClient"; import * as HttpClientResponse from "effect/http/HttpClientResponse"; @@ -21,9 +24,16 @@ import * as ServerSettings from "../serverSettings.ts"; import * as Suggestions from "./ThreadSplitSuggestions.ts"; import type { ProjectionRecords, ProjectionRecordField } from "./ProjectionStore.ts"; +const decodeRequest = Schema.decodeUnknownSync( + Schema.fromJsonString(Schema.Struct({ state: Schema.Unknown })), +); const threadId = ThreadId.make("split-test"); const input = { threadId, message: "What is the capital of France?" }; -function source() { +function source(): { + thread: OrchestrationV2AppThread; + messages: OrchestrationV2ConversationMessage[]; + runs: OrchestrationV2Run[]; +} { const now = DateTime.makeUnsafe("2026-11-03T00:00:00Z"); const thread: OrchestrationV2AppThread = { id: threadId, @@ -193,3 +203,94 @@ it.effect("does not queue a second model request while one is in flight", () => }).pipe(Effect.provide(layer)); }), ); + +it.effect("classifies only history from visible turns", () => + Effect.gen(function* () { + for (const scenario of [ + "rolled-back", + "cancelled-before-start", + "cancelled-after-start", + ] as const) { + const records = source(); + const runId = RunId.make("discarded-run"); + const user = { + ...records.messages[0]!, + id: MessageId.make("discarded-request"), + text: "An abandoned topic about cooking.", + }; + records.runs.push({ + id: runId, + threadId, + ordinal: 1, + providerInstanceId: records.thread.providerInstanceId, + modelSelection: records.thread.modelSelection, + providerThreadId: null, + userMessageId: user.id, + rootNodeId: null, + activeAttemptId: null, + status: scenario === "rolled-back" ? "rolled_back" : "cancelled", + requestedAt: records.thread.createdAt, + startedAt: scenario === "cancelled-before-start" ? null : records.thread.createdAt, + completedAt: records.thread.createdAt, + checkpointId: null, + contextHandoffId: null, + }); + records.messages.push(user); + if (scenario !== "cancelled-before-start") + records.messages.push({ + ...records.messages[1]!, + id: MessageId.make("discarded-reply"), + runId, + text: "Here is a recipe.", + }); + let state: unknown; + const client = HttpClient.make((request) => + Effect.sync(() => { + assert.equal(request.body._tag, "Uint8Array"); + if (request.body._tag === "Uint8Array") + state = decodeRequest(new TextDecoder().decode(request.body.body)).state; + return HttpClientResponse.fromWeb( + request, + Response.json({ + model: "pplx-decider-flash-v1.2", + answers: { q: { type: "noul", noul: 0.98 } }, + }), + ); + }), + ); + const layer = Suggestions.layer.pipe( + Layer.provide( + Layer.mergeAll( + Layer.mock(Orchestrator.OrchestratorV2)({ + getMessageCount: () => Effect.succeed(records.messages.length), + getThreadRecords: () => + Effect.succeed(records as unknown as ProjectionRecords), + }), + ServerSettings.layerTest({ + threadSplitSuggestionsEnabled: true, + threadSplitModelEndpoint: "http://127.0.0.1:8000/v1/systemone", + }), + Layer.succeed(HttpClient.HttpClient, client), + ), + ), + ); + const result = yield* Effect.gen(function* () { + return yield* (yield* Suggestions.ThreadSplitSuggestions).suggest(input); + }).pipe(Effect.provide(layer)); + assert.deepEqual(result, { suggest: true }, scenario); + assert.deepEqual( + state, + { + prior_thread: [ + { user: "Run the tests.", assistant_final: "The parser tests passed." }, + ...(scenario === "cancelled-after-start" + ? [{ user: user.text, assistant_final: "Here is a recipe." }] + : []), + ], + new_message: input.message, + }, + scenario, + ); + } + }), +); diff --git a/apps/server/src/orchestration-v2/ThreadSplitSuggestions.ts b/apps/server/src/orchestration-v2/ThreadSplitSuggestions.ts index 90227e1222757..7fcd52413a983 100644 --- a/apps/server/src/orchestration-v2/ThreadSplitSuggestions.ts +++ b/apps/server/src/orchestration-v2/ThreadSplitSuggestions.ts @@ -81,8 +81,19 @@ const make = Effect.gen(function* () { source.thread.deletedAt ) return noSuggestion; + const hiddenRuns = source.runs.filter( + (run) => + run.status === "rolled_back" || (run.status === "cancelled" && run.startedAt === null), + ); + const hiddenRunIds = new Set(hiddenRuns.map((run) => run.id)); + const hiddenMessageIds = new Set(hiddenRuns.map((run) => run.userMessageId)); const prior: Array<{ user: string; assistant_final: string }> = []; for (const message of source.messages) { + if ( + (message.runId !== null && hiddenRunIds.has(message.runId)) || + hiddenMessageIds.has(message.id) + ) + continue; if ( message.streaming || message.attachments.length || diff --git a/apps/server/src/storageCleanup.test.ts b/apps/server/src/storageCleanup.test.ts index ad4f7b74e9b15..c0e3837df6c3a 100644 --- a/apps/server/src/storageCleanup.test.ts +++ b/apps/server/src/storageCleanup.test.ts @@ -8,7 +8,7 @@ import * as Fiber from "effect/Fiber"; import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; -import type * as Path from "effect/Path"; +import * as Path from "effect/Path"; import type * as Scope from "effect/Scope"; import * as Stream from "effect/Stream"; import { SCAN_CACHE_FILE_NAME } from "./usage/usageScanCache.ts"; @@ -321,6 +321,7 @@ const cleanupFixture = ( ) => Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; const config = yield* ServerConfig.ServerConfig; const git = yield* GitVcsDriver.GitVcsDriver; const repo = yield* fs.realPath(yield* fs.makeTempDirectoryScoped({ prefix: "cleanup-repo-" })); @@ -334,7 +335,7 @@ const cleanupFixture = ( yield* command(repo, ["add", "."]); yield* command(repo, ["commit", "-m", "initial"]); yield* fs.makeDirectory(config.worktreesDir, { recursive: true }); - const worktree = `${config.worktreesDir}/feature`; + const worktree = path.join(config.worktreesDir, "feature"); yield* command(repo, ["worktree", "add", "-b", "feature", worktree]); let settings: import("@t3tools/contracts").ServerSettings = { ...DEFAULT_SERVER_SETTINGS, @@ -522,6 +523,7 @@ const cleanupFixture = ( editBeforeRemoval = true; }, fs, + path, worktree, config, command, @@ -1272,6 +1274,35 @@ describe("deleted thread worktrees", () => { ), ); + it.live.each([ + ["reflog", "Worktree history exceeds the inspection limit"], + ["worktree", "Worktree list exceeds the inspection limit"], + ] as const)("keeps a worktree when its %s inspection is truncated", ([command, reason]) => + runCleanupTest( + Effect.gen(function* () { + const git = yield* GitVcsDriver.GitVcsDriver; + const { service, fs, worktree } = yield* cleanupFixture().pipe( + Effect.provideService(GitVcsDriver.GitVcsDriver, { + ...git, + execute: (input) => + git + .execute(input) + .pipe( + Effect.map((result) => + input.operation === "StorageCleanup.checkoutBlocker" && + input.args[0] === command + ? { ...result, stdoutTruncated: true } + : result, + ), + ), + }), + ); + expect((yield* service.runNow).entries[0]).toMatchObject({ outcome: "kept", reason }); + expect(yield* fs.exists(worktree)).toBe(true); + }), + ), + ); + it.live("keeps a deleted owner's worktree while only its reflog holds a commit", () => runCleanupTest( Effect.gen(function* () { @@ -1850,8 +1881,8 @@ describe("orphan worktree reports", () => { for (let index = 0; index < 205; index++) { yield* fixture.fs.makeDirectory(`${fixture.config.worktreesDir}/busy-${index}`); } - const orphans = ["one", "two"].map( - (name) => `${fixture.config.worktreesDir}/repository/${name}`, + const orphans = ["one", "two"].map((name) => + fixture.path.join(fixture.config.worktreesDir, "repository", name), ); for (const orphan of orphans) { yield* fixture.fs.makeDirectory(orphan, { recursive: true }); @@ -1907,7 +1938,7 @@ describe("orphan worktree reports", () => { const loop = `${fixture.repo}/loop`; yield* fixture.fs.symlink(loop, loop); fixture.setProjectRoots([fixture.repo, loop]); - const orphan = `${fixture.config.worktreesDir}/repository/orphan`; + const orphan = fixture.path.join(fixture.config.worktreesDir, "repository", "orphan"); yield* fixture.fs.makeDirectory(orphan, { recursive: true }); yield* fixture.fs.utimes(orphan, 1, 1); expect((yield* fixture.service.runNow).entries).toEqual([ @@ -1924,7 +1955,11 @@ describe("orphan worktree reports", () => { Effect.gen(function* () { const fixture = yield* cleanupFixture(); fixture.setThreads([]); - const orphan = `${fixture.config.worktreesDir}/${location === "nested" ? "repository/" : ""}orphan`; + const orphan = fixture.path.join( + fixture.config.worktreesDir, + location === "nested" ? "repository" : "", + "orphan", + ); yield* fixture.command(fixture.repo, ["worktree", "add", "-b", "orphan", orphan]); const sizes = yield* Effect.forEach(yield* fixture.fs.readDirectory(orphan), (name) => allocatedBytes(fixture.fs, `${orphan}/${name}`), @@ -2044,13 +2079,13 @@ describe("orphan worktree reports", () => { fixture.setThreads([]); const outside = yield* fixture.fs.makeTempDirectoryScoped(); yield* fixture.fs.makeDirectory(`${outside}/checkout`); - const projectDirectory = `${fixture.config.worktreesDir}/repository`; + const projectDirectory = fixture.path.join(fixture.config.worktreesDir, "repository"); yield* fixture.fs.makeDirectory(projectDirectory); yield* fixture.fs.symlink(outside, `${fixture.config.worktreesDir}/linked-project`); yield* fixture.fs.symlink(`${outside}/checkout`, `${projectDirectory}/linked-checkout`); yield* fixture.fs.symlink(`${outside}/missing`, `${projectDirectory}/dangling-link`); expect((yield* fixture.service.runNow).entries).toEqual([]); - const orphan = `${projectDirectory}/orphan`; + const orphan = fixture.path.join(projectDirectory, "orphan"); yield* fixture.fs.makeDirectory(orphan); fixture.disable(); expect((yield* fixture.service.runNow).entries).toEqual([]); @@ -2064,7 +2099,7 @@ describe("orphan worktree reports", () => { Effect.gen(function* () { const fixture = yield* cleanupFixture(); fixture.setThreads([]); - const folder = `${fixture.config.worktreesDir}/repository/leftover`; + const folder = fixture.path.join(fixture.config.worktreesDir, "repository", "leftover"); yield* fixture.fs.makeDirectory(folder, { recursive: true }); yield* fixture.fs.writeFileString(`${folder}/payload`, "leftover data"); yield* fixture.fs.utimes(folder, 1, 1); @@ -2091,7 +2126,7 @@ describe("orphan worktree reports", () => { const fixture = yield* cleanupFixture(); fixture.setThreads([]); const blocked = `${fixture.config.worktreesDir}/a-inaccessible`; - const orphan = `${fixture.config.worktreesDir}/z-repository/orphan`; + const orphan = fixture.path.join(fixture.config.worktreesDir, "z-repository", "orphan"); yield* fixture.fs.makeDirectory(blocked); yield* fixture.fs.makeDirectory(orphan, { recursive: true }); yield* fixture.fs.writeFileString(`${orphan}/file`, "x"); @@ -2121,10 +2156,10 @@ describe("orphan worktree reports", () => { Effect.provideService(HostProcess.Platform, platform), ); fixture.setThreads([]); - const parent = `${fixture.config.worktreesDir}/repository`; - const referenced = `${parent}/Referenced`; - const project = `${parent}/Project`; - const nested = `${parent}/Nested`; + const parent = fixture.path.join(fixture.config.worktreesDir, "repository"); + const referenced = fixture.path.join(parent, "Referenced"); + const project = fixture.path.join(parent, "Project"); + const nested = fixture.path.join(parent, "Nested"); for (const folder of [referenced, project, nested]) { yield* fixture.fs.makeDirectory(`${folder}/child`, { recursive: true }); yield* fixture.fs.utimes(folder, 1, 1); @@ -2133,7 +2168,9 @@ describe("orphan worktree reports", () => { fixture.setProjectRoots([fixture.repo, project, `${nested}/child`]); const report = yield* fixture.service.runNow; expect(report.entries.map((entry) => entry.path).sort()).toEqual( - platform === "win32" ? [] : [referenced], + platform === "win32" || (yield* fixture.fs.exists(referenced.toUpperCase())) + ? [] + : [referenced], ); }), ), diff --git a/apps/server/src/storageCleanup.ts b/apps/server/src/storageCleanup.ts index 07ddde641a4cc..d424c87a08bad 100644 --- a/apps/server/src/storageCleanup.ts +++ b/apps/server/src/storageCleanup.ts @@ -623,6 +623,8 @@ const make = Effect.gen(function* () { args: ["reflog", "show", "--format=%H", "HEAD"], maxOutputBytes: 16 * 1024 * 1024, }); + if (reflog.stdoutTruncated || reflog.stderrTruncated) + return "worktree history exceeds the inspection limit"; // Only refs that stay count. A user cannot recover from refs/t3, and a routine // `git fetch --prune` can drop any remote branch, even one a stale local HEAD calls the default. const unheld = yield* git.execute({ @@ -637,6 +639,8 @@ const make = Effect.gen(function* () { cwd, args: ["worktree", "list", "--porcelain"], }); + if (worktrees.stdoutTruncated || worktrees.stderrTruncated) + return "worktree list exceeds the inspection limit"; const held = new Set( [...worktrees.stdout.matchAll(/^branch refs\/heads\/(.+?)\r?$/gm)].map((match) => match[1]), ); diff --git a/apps/web/src/components/chat/MarkdownThreadLink.tsx b/apps/web/src/components/chat/MarkdownThreadLink.tsx index b5d3bbb5ac782..cc90b5f995ecc 100644 --- a/apps/web/src/components/chat/MarkdownThreadLink.tsx +++ b/apps/web/src/components/chat/MarkdownThreadLink.tsx @@ -29,17 +29,17 @@ export function MarkdownThreadLink(props: { written === null && decodedId !== null ? scopeThreadRef(environmentId, decodedId) : null, ); const local = written ?? decoded; - // Shells leave out idle subagent children and older settled threads, so a cited thread the - // shell lacks is read with its family until it lands; only a settled empty read proves it gone. - const missingThreadId = local === null ? (decodedId ?? props.link.threadId) : null; - // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- mounts the family read for a cited thread the shell omits while this link shows it - useEffect( - () => - missingThreadId === null - ? undefined - : threadFamilies.mount(appAtomRegistry, environmentId, missingThreadId), - [environmentId, missingThreadId], - ); + const threadId = props.link.threadId; + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- keeps cited rows mounted for the link's lifetime, independently of the family data it loads + useEffect(() => { + const ids = decodedId === null ? [threadId] : [threadId, decodedId]; + const releases = ids.map((id) => + threadFamilies.mount(appAtomRegistry, environmentId, id, { onlyWhenMissing: true }), + ); + return () => { + for (const release of releases) release(); + }; + }, [environmentId, threadId, decodedId]); const guess = local === null && props.link.environmentId === null; const refs = useThreadRefs(guess); const remote = useThreadShell(guess ? guessThreadLinkRef(props.link, refs) : null); diff --git a/packages/client-runtime/src/state/threadFamilies.test.ts b/packages/client-runtime/src/state/threadFamilies.test.ts index b08656975dbd9..df30aef139b2e 100644 --- a/packages/client-runtime/src/state/threadFamilies.test.ts +++ b/packages/client-runtime/src/state/threadFamilies.test.ts @@ -125,8 +125,12 @@ describe("createThreadFamilyAtoms", () => { yield* Queue.unbounded< Effect.Effect >(); + const requests: ThreadId[] = []; const client = { - [ORCHESTRATION_V2_WS_METHODS.getThreadFamily]: () => Effect.flatten(Queue.take(replies)), + [ORCHESTRATION_V2_WS_METHODS.getThreadFamily]: ({ threadId }: { threadId: ThreadId }) => + Effect.sync(() => requests.push(threadId)).pipe( + Effect.andThen(Effect.flatten(Queue.take(replies))), + ), } as unknown as WsRpcProtocolClient; const session = { client, @@ -231,6 +235,91 @@ describe("createThreadFamilyAtoms", () => { yield* settle; expect(threadFamilyRead(registry.get(family))).toBe("settled"); expect(mergedIds()).toEqual(["parent", "idle-child"]); + + const visible = row("visible-citation"); + const omitted = row("omitted-citation"); + registry.set( + shellState, + AsyncResult.success({ + ...shellAt(2), + snapshot: Option.some({ ...shellSnapshot, threads: [parent, visible] }), + }), + ); + const releaseVisible = families.mount(registry, TARGET.environmentId, visible.id, { + onlyWhenMissing: true, + }); + const releaseOmitted = families.mount(registry, TARGET.environmentId, omitted.id, { + onlyWhenMissing: true, + }); + yield* Queue.offerAll(replies, [ + Effect.succeed({ snapshotSequence: 5, threads: [omitted] }), + Effect.succeed({ snapshotSequence: 5, threads: [visible] }), + ]); + yield* AtomRegistry.getResult( + registry, + families.familyAtom(TARGET.environmentId, omitted.id), + { suspendOnWaiting: true }, + ); + expect(requests).toEqual([parent.id, parent.id, parent.id, omitted.id]); + expect(mergedIds()).toEqual([ + "parent", + "visible-citation", + "idle-child", + "omitted-citation", + ]); + + registry.set(shellState, AsyncResult.success(shellAt(2))); + yield* AtomRegistry.getResult( + registry, + families.familyAtom(TARGET.environmentId, visible.id), + { suspendOnWaiting: true }, + ); + expect(requests).toEqual([parent.id, parent.id, parent.id, omitted.id, visible.id]); + expect(mergedIds()).toEqual([ + "parent", + "idle-child", + "visible-citation", + "omitted-citation", + ]); + releaseVisible(); + releaseOmitted(); + expect(mergedIds()).toEqual(["parent", "idle-child"]); + + const unavailable = row("cached-failure"); + const unavailableFamily = families.familyAtom(TARGET.environmentId, unavailable.id); + const releaseFailed = families.mount(registry, TARGET.environmentId, unavailable.id); + yield* Queue.offer( + replies, + Effect.fail(new OrchestrationV2GetShellSnapshotError({ message: "read failed" })), + ); + yield* AtomRegistry.getResult(registry, unavailableFamily, { + suspendOnWaiting: true, + }).pipe(Effect.ignore); + expect(threadFamilyRead(registry.get(unavailableFamily))).toBe("failed"); + releaseFailed(); + registry.set( + shellState, + AsyncResult.success({ + ...shellAt(2), + snapshot: Option.some({ ...shellSnapshot, threads: [parent, unavailable] }), + }), + ); + const releaseRetry = families.mount(registry, TARGET.environmentId, unavailable.id, { + onlyWhenMissing: true, + }); + expect(requests.filter((id) => id === unavailable.id)).toHaveLength(1); + registry.set(shellState, AsyncResult.success(shellAt(2))); + yield* settle; + expect(threadFamilyRead(registry.get(unavailableFamily))).toBe("pending"); + yield* Queue.offer( + replies, + Effect.succeed({ snapshotSequence: 5, threads: [unavailable] }), + ); + yield* AtomRegistry.getResult(registry, unavailableFamily, { suspendOnWaiting: true }); + expect(requests.filter((id) => id === unavailable.id)).toHaveLength(2); + expect(mergedIds()).toEqual(["parent", "idle-child", "cached-failure"]); + releaseRetry(); + expect(mergedIds()).toEqual(["parent", "idle-child"]); }), ), ); diff --git a/packages/client-runtime/src/state/threadFamilies.ts b/packages/client-runtime/src/state/threadFamilies.ts index 0f1bb8d9cd274..7a0f62f75a6de 100644 --- a/packages/client-runtime/src/state/threadFamilies.ts +++ b/packages/client-runtime/src/state/threadFamilies.ts @@ -151,6 +151,26 @@ export function createThreadFamilyAtoms( }).pipe(Atom.withLabel(`environment-thread-family-rows:${key}`)), ); + const shellThreadIdsAtom = Atom.family((environmentId: EnvironmentId) => { + let previous: ReadonlyArray | undefined; + let ids: ReadonlySet = new Set(); + return Atom.make((get) => { + const state = shellStateValue(get(shellStateAtom(environmentId))); + const threads = state === null ? undefined : Option.getOrNull(state.snapshot)?.threads; + if (threads !== previous) { + previous = threads; + ids = new Set(threads?.map(({ id }) => id)); + } + return ids; + }).pipe(Atom.withLabel(`environment-shell-thread-ids:${environmentId}`)); + }); + const missingFamilyRowsAtom = Atom.family((key: string) => { + const [environmentId, threadId] = JSON.parse(key) as [EnvironmentId, ThreadId]; + return Atom.make((get) => + get(shellThreadIdsAtom(environmentId)).has(threadId) ? EMPTY_READS : get(familyRowsAtom(key)), + ).pipe(Atom.withLabel(`environment-missing-thread-family-rows:${key}`)); + }); + /** * Merges `rows` behind the shell rows until the returned release runs. Reads read under `key` * share one entry; each read's `snapshotSequence` decides which removals drop its rows. @@ -186,14 +206,41 @@ export function createThreadFamilyAtoms( registry: AtomRegistry.AtomRegistry, environmentId: EnvironmentId, threadId: ThreadId, + options: { readonly onlyWhenMissing?: boolean } = {}, ): (() => void) => { const key = familyKey(environmentId, threadId); - const release = mountRows(registry, environmentId, `family:${key}`, familyRowsAtom(key)); - // Mounting again after a failure retries instead of keeping the failure until reconnect. - if (threadFamilyRead(registry.get(familyAtom(key))) === "failed") { - refresh(registry, environmentId, threadId); + const onlyWhenMissing = options.onlyWhenMissing === true; + const release = mountRows( + registry, + environmentId, + `${onlyWhenMissing ? "missing-family" : "family"}:${key}`, + onlyWhenMissing ? missingFamilyRowsAtom(key) : familyRowsAtom(key), + ); + const retryFailed = () => { + if (threadFamilyRead(registry.get(familyAtom(key))) === "failed") { + refresh(registry, environmentId, threadId); + } + }; + if (!onlyWhenMissing) { + retryFailed(); + return release; } - return release; + let wasMissing = false; + // A cached failure must retry when the shell first omits a still-mounted citation. + const unsubscribe = registry.subscribe( + shellThreadIdsAtom(environmentId), + (ids) => { + const missing = !ids.has(threadId); + const becameMissing = missing && !wasMissing; + wasMissing = missing; + if (becameMissing) retryFailed(); + }, + { immediate: true }, + ); + return () => { + unsubscribe(); + release(); + }; }; const snapshotAtom = Atom.family((environmentId: EnvironmentId) => { diff --git a/packages/provider-core/src/server/modelEvaluation.test.ts b/packages/provider-core/src/server/modelEvaluation.test.ts index cb440d346eb2d..2972cd64ebb0a 100644 --- a/packages/provider-core/src/server/modelEvaluation.test.ts +++ b/packages/provider-core/src/server/modelEvaluation.test.ts @@ -1,5 +1,6 @@ import { expect, it } from "vite-plus/test"; import { + ComposerContextId, MessageId, type ModelEvaluationVerdict, NodeId, @@ -8,6 +9,7 @@ import { RunId, ThreadId, TurnItemId, + type OrchestrationMessageContext, type OrchestrationV2ConversationMessage, type OrchestrationV2TurnItem, type BlindModelEvaluationPacket, @@ -361,3 +363,64 @@ it("abstains when there is no finalized outcome and rejects fabricated citations ), ).toBe(true); }); + +it.each([ + "request", + "request-item", + "earlier-context", + "interleaved-user", + "empty-context", +] as const)("marks omitted composer artifacts from %s", (scenario) => { + const source = fixture(); + const context: OrchestrationMessageContext = { + version: 1, + records: + scenario === "empty-context" + ? [] + : [ + { + version: 1, + contextId: ComposerContextId.make("terminal-log"), + label: "Test output", + kind: "terminal", + terminalId: "terminal", + terminalLabel: "Tests", + lineStart: 1, + lineEnd: 2, + text: "Error: expected success but got failure", + }, + ], + }; + const request = source.messages[0]!; + const requestItem = source.turnItems[0]!; + if (requestItem.type !== "user_message") throw new Error("Expected request item"); + if (scenario === "request-item") { + source.messages.shift(); + const { textOmitted: _textOmitted, ...inlineItem } = requestItem; + source.turnItems[0] = { ...inlineItem, text: request.text, context }; + } else if (scenario === "earlier-context") { + source.messages.unshift({ + ...request, + id: MessageId.make("earlier-request"), + runId: null, + createdAt: DateTime.makeUnsafe("2026-01-01T23:59:59Z"), + context, + }); + } else if (scenario === "interleaved-user") { + const followup = { ...request, id: MessageId.make("followup"), context }; + source.messages.push(followup); + source.turnItems[3] = { + ...requestItem, + id: TurnItemId.make("followup-item"), + messageId: followup.id, + ordinal: 4, + }; + } else { + source.messages[0] = { ...request, context }; + } + const packet = buildModelEvaluationEvidence(source)!.packet; + expect(packet.missingArtifacts).toBe(scenario !== "empty-context"); + expect(validateModelEvaluationVerdict({ ...verdict(), confidence: "high" }, packet)).toBe( + scenario === "empty-context", + ); +}); diff --git a/packages/provider-core/src/server/modelEvaluation.ts b/packages/provider-core/src/server/modelEvaluation.ts index 692b026435419..542fb4dc83d60 100644 --- a/packages/provider-core/src/server/modelEvaluation.ts +++ b/packages/provider-core/src/server/modelEvaluation.ts @@ -50,6 +50,17 @@ const IDENTITY_NAMES = [ ]; const escapePattern = (value: string) => value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +function hasOmittedMessageArtifacts( + message: + | { + readonly attachments?: ReadonlyArray | undefined; + readonly context?: { readonly records: ReadonlyArray } | undefined; + } + | undefined, +): boolean { + return (message?.attachments?.length ?? 0) > 0 || (message?.context?.records.length ?? 0) > 0; +} + function familyTokens(model: string) { return model.split("/").flatMap((segment) => segment.match(/^\p{L}{3,}/u) ?? []); } @@ -234,11 +245,12 @@ export function buildModelEvaluationEvidence(input: { let truncated = request.truncated || final.truncated; let omittedItems = 0; let missingArtifacts = - (requestMessage?.attachments.length ?? - (requestItem?.type === "user_message" ? requestItem.attachments.length : 0)) > 0 || - (finalMessage?.attachments.length ?? - (finalItem?.type === "assistant_message" ? finalItem.attachments?.length : 0) ?? - 0) > 0; + hasOmittedMessageArtifacts( + requestMessage ?? (requestItem?.type === "user_message" ? requestItem : undefined), + ) || + hasOmittedMessageArtifacts( + finalMessage ?? (finalItem?.type === "assistant_message" ? finalItem : undefined), + ); const sourceMessageIds = new Set([input.run.userMessageId]); if (finalMessage !== undefined) sourceMessageIds.add(finalMessage.id); const context: Array = []; @@ -257,7 +269,7 @@ export function buildModelEvaluationEvidence(input: { for (const message of earlier.slice(-4)) { const excerpt = clip(redact(message.text), 1_000); truncated ||= excerpt.truncated; - missingArtifacts ||= message.attachments.length > 0; + missingArtifacts ||= hasOmittedMessageArtifacts(message); remaining -= excerpt.text.length; context.push({ ref: `C${context.length + 1}`, @@ -280,11 +292,12 @@ export function buildModelEvaluationEvidence(input: { continue; let entry; if (item.type === "user_message") { - const text = messageById.get(item.messageId)?.text ?? (!item.textOmitted ? item.text : ""); + const message = messageById.get(item.messageId); + const text = message?.text ?? (!item.textOmitted ? item.text : ""); entry = { kind: "user" as const, text, - missing: text.length === 0 || item.attachments.length > 0, + missing: text.length === 0 || hasOmittedMessageArtifacts(message ?? item), }; } else { entry = evidenceText(item, redact); diff --git a/packages/shared/src/testing/providerEnv.ts b/packages/shared/src/testing/providerEnv.ts index ac07867c20473..50db357a45f9e 100644 --- a/packages/shared/src/testing/providerEnv.ts +++ b/packages/shared/src/testing/providerEnv.ts @@ -10,6 +10,8 @@ const KEPT_ENV = new Set([ "T3CODE_TRANSFER_BUDGET_RESULT_PATH", // Opt-in: points the HtmlRender tests at a headless browser, which they skip without. "T3CODE_TEST_HEADLESS_SHELL", + // The mobile test script sets this; tests must see the platform their code was compiled for. + "T3_MOBILE_TEST_PLATFORM", ]); if (process.env.T3CODE_TEST_KEEP_PROVIDER_ENV !== "1") { From 7ad853bdf55e57be9436f4158fd4f724837a8644 Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Fri, 6 Nov 2026 01:00:19 -0800 Subject: [PATCH 2/2] test: make cleanup allocation and git fixtures portable to Windows Compare report totals with actual filesystem allocation and preserve Git's hidden worktree metadata file while testing invalid pointers. Model: GPT-6 Astra Pro. Harness: ChatGPT. --- apps/server/src/storageCleanup.test.ts | 29 ++++++++++++++++++-------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/apps/server/src/storageCleanup.test.ts b/apps/server/src/storageCleanup.test.ts index c0e3837df6c3a..d4e4fc60e92ad 100644 --- a/apps/server/src/storageCleanup.test.ts +++ b/apps/server/src/storageCleanup.test.ts @@ -2487,6 +2487,10 @@ describe("storage cleanup reports and local file policies", () => { }), ); const root = yield* fs.realPath(fixture.worktree); + const sizes = yield* Effect.forEach(yield* fs.readDirectory(root), (name) => + allocatedBytes(fs, fixture.path.join(root, name)), + ); + const expectedBytes = sizes.reduce((sum, size) => sum + size, 0); fixture.setOpenSessionCwd(fixture.worktree); const kept = yield* fixture.service.runNow; expect(kept.entries[0]).toMatchObject({ @@ -2497,8 +2501,8 @@ describe("storage cleanup reports and local file policies", () => { expect(walked.filter((directory) => directory.startsWith(root))).toEqual([]); fixture.setSessionLive(false); const removed = yield* fixture.service.runNow; - expect(removed.entries[0]).toMatchObject({ outcome: "removed" }); - expect(removed.entries[0]?.bytes).toBeGreaterThan(0); + expect(removed.entries[0]).toMatchObject({ outcome: "removed", bytes: expectedBytes }); + expect(removed.bytesFreed).toBe(expectedBytes); expect(walked).toContain(root); }), ), @@ -2541,10 +2545,14 @@ describe("storage cleanup reports and local file policies", () => { expect(locked?.reason).toContain("Git index is locked"); expect(yield* fs.readFileString(`${worktree}/tracked.txt`)).toBe("original\n"); yield* fs.remove(`${index.trim()}.lock`); - const gitFile = yield* fs.readFileString(`${worktree}/.git`); - yield* fs.writeFileString(`${worktree}/.git`, "gitdir: /missing-git-directory\n"); + const gitFile = `${worktree}/.git`; + const savedGitFile = `${worktree}/.git-original`; + // Git marks this file hidden on Windows, where overwriting it fails with EPERM. + yield* fs.rename(gitFile, savedGitFile); + yield* fs.writeFileString(gitFile, "gitdir: /missing-git-directory\n"); expect((yield* service.runNow).entries[0]).toMatchObject(changed); - yield* fs.writeFileString(`${worktree}/.git`, gitFile); + yield* fs.remove(gitFile); + yield* fs.rename(savedGitFile, gitFile); expect((yield* service.runNow).entries[0]).toMatchObject({ outcome: "removed" }); expect(fullStatusCalls()).toBe(0); }), @@ -2908,7 +2916,11 @@ describe("storage cleanup reports and local file policies", () => { it.live("caps entries while preserving totals and prioritizing removals", () => runCleanupTest( Effect.gen(function* () { - const { service, fs, worktree, setThreads } = yield* cleanupFixture(); + const { service, fs, path, worktree, setThreads } = yield* cleanupFixture(); + const sizes = yield* Effect.forEach(yield* fs.readDirectory(worktree), (name) => + allocatedBytes(fs, path.join(worktree, name)), + ); + const expectedBytes = sizes.reduce((sum, size) => sum + size, 0); const threads = []; for (let i = 0; i < 205; i++) { const worktreePath = `${worktree}-${i}`; @@ -2928,9 +2940,8 @@ describe("storage cleanup reports and local file policies", () => { expect(report.entries).toHaveLength(200); expect(report.omittedCount).toBe(6); expect(report.counts).toEqual({ removed: 1, kept: 205, failed: 0 }); - expect(report.entries[0]?.outcome).toBe("removed"); - expect(report.entries[0]?.bytes).toBeGreaterThan(0); - expect(report.bytesFreed).toBe(report.entries[0]?.bytes); + expect(report.entries[0]).toMatchObject({ outcome: "removed", bytes: expectedBytes }); + expect(report.bytesFreed).toBe(expectedBytes); }), ), );