From ce171b1c989fbbda289d93cff6c4012242c8f36b Mon Sep 17 00:00:00 2001 From: Badcuban <108198679+badcuban@users.noreply.github.com> Date: Wed, 12 Aug 2026 01:44:29 -0400 Subject: [PATCH] Stabilize sidebar terminal and PR indicators --- apps/server/src/git/GitManager.test.ts | 47 +++++++- apps/server/src/git/GitManager.ts | 48 +++++++- .../src/terminal/Layers/Manager.test.ts | 58 ++++++++++ apps/server/src/terminal/Layers/Manager.ts | 105 +++++++++++++----- 4 files changed, 220 insertions(+), 38 deletions(-) diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index 1354de7ad..212e4578a 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -44,7 +44,7 @@ import { } from "../project/Services/ProjectSetupScriptRunner.ts"; interface FakeGhScenario { - prListSequence?: string[]; + prListSequence?: Array; prListByHeadSelector?: Record; prListSequenceByHeadSelector?: Record; createdPrUrl?: string; @@ -423,7 +423,11 @@ function createGitHubCliWithFakeGh(scenario: FakeGhScenario = {}): { typeof headSelector === "string" ? scenario.prListByHeadSelector?.[headSelector] : undefined; - const stdout = (mappedQueue ?? mappedStdout ?? prListQueue.shift() ?? "[]") + "\n"; + const queued = mappedQueue ?? mappedStdout ?? prListQueue.shift() ?? "[]"; + if (queued instanceof GitHubCliError) { + return Effect.fail(queued); + } + const stdout = queued + "\n"; return Effect.succeed(fakeGhOutput(stdout)); } @@ -1423,6 +1427,45 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => { }), ); + it.effect("keeps the last-known PR through a lookup failure and clears it on success", () => + Effect.gen(function* () { + const repoDir = yield* makeTempDir("threadlines-git-manager-"); + yield* initRepo(repoDir); + yield* runGit(repoDir, ["checkout", "-b", "feature/status-stable-pr"]); + const remoteDir = yield* createBareRemote(); + yield* runGit(repoDir, ["remote", "add", "origin", remoteDir]); + yield* runGit(repoDir, ["push", "-u", "origin", "feature/status-stable-pr"]); + + const existingPr = { + number: 47, + title: "Stable PR icon", + url: "https://github.com/pingdotgg/codething-mvp/pull/47", + baseRefName: "main", + headRefName: "feature/status-stable-pr", + }; + const { manager } = yield* makeManager({ + ghScenario: { + prListSequence: [ + JSON.stringify([existingPr]), + new GitHubCliError({ + operation: "execute", + detail: "Temporary GitHub lookup failure.", + }), + "[]", + ], + }, + }); + + const initial = yield* manager.status({ cwd: repoDir }); + const retained = yield* manager.remoteStatus({ cwd: repoDir }, { forceRefresh: true }); + const cleared = yield* manager.remoteStatus({ cwd: repoDir }, { forceRefresh: true }); + + expect(initial.pr?.number).toBe(47); + expect(retained?.pr?.number).toBe(47); + expect(cleared?.pr).toBeNull(); + }), + ); + it.effect("creates a commit when working tree is dirty", () => Effect.gen(function* () { const repoDir = yield* makeTempDir("threadlines-git-manager-"); diff --git a/apps/server/src/git/GitManager.ts b/apps/server/src/git/GitManager.ts index 4dd2096a9..11acd77ae 100644 --- a/apps/server/src/git/GitManager.ts +++ b/apps/server/src/git/GitManager.ts @@ -899,6 +899,15 @@ export const makeGitManager = Effect.fn("makeGitManager")(function* () { normalizeStatusCacheKey(cwd).pipe( Effect.flatMap((cacheKey) => Cache.invalidate(localStatusResultCache, cacheKey)), ); + const lastKnownPrByCwdRef = yield* Ref.make( + new Map< + string, + { + readonly branch: string; + readonly pr: VcsStatusRemoteResult["pr"]; + } + >(), + ); const readRemoteStatus = Effect.fn("readRemoteStatus")(function* ( cwd: string, options?: GitRemoteStatusOptions, @@ -910,12 +919,20 @@ export const makeGitManager = Effect.fn("makeGitManager")(function* () { return null; } + const branch = details.branch; const pr = - details.branch !== null - ? yield* findLatestPr(cwd, { - branch: details.branch, - upstreamRef: details.upstreamRef, - }).pipe( + branch !== null + ? yield* sourceControlProvider(cwd).pipe( + // An unknown host has no PR integration. That is a successful + // absence, unlike a configured provider that temporarily fails. + Effect.flatMap((provider) => + provider.kind === "unknown" + ? Effect.succeed(null) + : findLatestPr(cwd, { + branch, + upstreamRef: details.upstreamRef, + }), + ), Effect.map((latest) => { if (!latest) return null; // On the default branch, only surface open PRs. @@ -923,7 +940,26 @@ export const makeGitManager = Effect.fn("makeGitManager")(function* () { if (details.isDefaultBranch && latest.state !== "open") return null; return toStatusPr(latest); }), - Effect.catch(() => Effect.succeed(null)), + Effect.tap((nextPr) => + Ref.update(lastKnownPrByCwdRef, (lastKnownByCwd) => { + const next = new Map(lastKnownByCwd); + next.delete(cwd); + next.set(cwd, { branch, pr: nextPr }); + if (next.size > STATUS_RESULT_CACHE_CAPACITY) { + const oldestCwd = next.keys().next().value; + if (oldestCwd !== undefined) next.delete(oldestCwd); + } + return next; + }), + ), + Effect.catch(() => + Ref.get(lastKnownPrByCwdRef).pipe( + Effect.map((lastKnownByCwd) => { + const lastKnown = lastKnownByCwd.get(cwd); + return lastKnown?.branch === branch ? lastKnown.pr : null; + }), + ), + ), ) : null; diff --git a/apps/server/src/terminal/Layers/Manager.test.ts b/apps/server/src/terminal/Layers/Manager.test.ts index a01be037f..81342754d 100644 --- a/apps/server/src/terminal/Layers/Manager.test.ts +++ b/apps/server/src/terminal/Layers/Manager.test.ts @@ -712,6 +712,64 @@ it.layer(NodeServices.layer, { excludeTestServices: true })("TerminalManager", ( }), ); + it.effect("ignores a PowerShell completion marker queued before command submission", () => + Effect.gen(function* () { + const { manager, getEvents, ptyAdapter } = yield* createManager(5, { + platform: "win32", + shellResolver: () => "powershell.exe", + subprocessChecker: () => Effect.succeed(false), + subprocessPollIntervalMs: 20, + }); + + yield* manager.open(openInput()); + const process = ptyAdapter.processes[0]; + assert.isDefined(process); + + const drainBlocked = yield* Deferred.make(); + const releaseDrain = yield* Deferred.make(); + const unsubscribe = yield* manager.subscribe((event) => + event.type === "output" && event.data === "prompt output" + ? Deferred.succeed(drainBlocked, undefined).pipe( + Effect.andThen(Deferred.await(releaseDrain)), + ) + : Effect.void, + ); + yield* Effect.addFinalizer(() => Effect.sync(unsubscribe)); + + process.emitData("prompt output"); + yield* Deferred.await(drainBlocked); + process.emitData("\u001b]633;D\u0007PS C:\\repo> "); + + yield* manager.write({ + threadId: "thread-1", + terminalId: DEFAULT_TERMINAL_ID, + data: "Start-Sleep -Seconds 30\r", + }); + yield* waitFor( + Effect.map(getEvents, (events) => + events.some((event) => event.type === "activity" && event.hasRunningSubprocess), + ), + ); + + yield* Deferred.succeed(releaseDrain, undefined); + yield* Effect.sleep("60 millis"); + expect((yield* getEvents).filter((event) => event.type === "activity")).toEqual([ + expect.objectContaining({ + type: "activity", + hasRunningSubprocess: true, + command: "Start-Sleep -Seconds 30", + }), + ]); + + process.emitData("\u001b]633;D\u0007PS C:\\repo> "); + yield* waitFor( + Effect.map(getEvents, (events) => + events.some((event) => event.type === "activity" && !event.hasRunningSubprocess), + ), + ); + }), + ); + it.effect("ignores an in-flight subprocess result after PowerShell reports completion", () => Effect.gen(function* () { const checkStarted = yield* Deferred.make(); diff --git a/apps/server/src/terminal/Layers/Manager.ts b/apps/server/src/terminal/Layers/Manager.ts index 78c102bbc..86914546f 100644 --- a/apps/server/src/terminal/Layers/Manager.ts +++ b/apps/server/src/terminal/Layers/Manager.ts @@ -132,7 +132,7 @@ interface TerminalSessionState { status: TerminalSessionStatus; pid: number | null; history: string; - pendingHistoryControlSequence: string; + pendingHistoryControlSequence: PendingHistoryControlSequence | null; pendingProcessEvents: Array; pendingProcessEventIndex: number; processEventDrainRunning: boolean; @@ -147,6 +147,7 @@ interface TerminalSessionState { hasRunningSubprocess: boolean; runningSubprocessCommand: string | null; submittedCommand: string | null; + commandGeneration: number; subprocessPollingArmed: boolean; consecutiveSubprocessIdlePolls: number; commandCompletionMarkersEnabled: boolean; @@ -154,12 +155,19 @@ interface TerminalSessionState { runtimeEnv: Record | null; } +interface PendingHistoryControlSequence { + data: string; + commandGeneration: number; +} + interface PersistHistoryRequest { history: string; immediate: boolean; } -type PendingProcessEvent = { type: "output"; data: string } | { type: "exit"; event: PtyExitEvent }; +type PendingProcessEvent = + | { type: "output"; data: string; commandGeneration: number } + | { type: "exit"; event: PtyExitEvent }; type DrainProcessEventAction = | { type: "idle" } @@ -523,19 +531,39 @@ function findEscapeSequenceEndIndex(input: string, start: number): number | null } function sanitizeTerminalHistoryChunk( - pendingControlSequence: string, + pendingControlSequence: PendingHistoryControlSequence | null, data: string, -): { visibleText: string; pendingControlSequence: string; commandFinished: boolean } { - const input = `${pendingControlSequence}${data}`; + commandGeneration: number, +): { + visibleText: string; + pendingControlSequence: PendingHistoryControlSequence | null; + commandFinishedGenerations: number[]; +} { + const pendingData = pendingControlSequence?.data ?? ""; + const input = `${pendingData}${data}`; let visibleText = ""; - let commandFinished = false; + const commandFinishedGenerations: number[] = []; let index = 0; - const result = (nextPendingControlSequence: string) => ({ - visibleText, - pendingControlSequence: nextPendingControlSequence, - commandFinished, - }); + const generationAt = (offset: number) => + pendingControlSequence !== null && offset < pendingData.length + ? pendingControlSequence.commandGeneration + : commandGeneration; + + const result = (pendingStartIndex: number | null) => { + const nextPendingData = pendingStartIndex === null ? "" : input.slice(pendingStartIndex); + return { + visibleText, + pendingControlSequence: + nextPendingData.length > 0 + ? { + data: nextPendingData, + commandGeneration: generationAt(pendingStartIndex ?? input.length), + } + : null, + commandFinishedGenerations, + }; + }; const append = (value: string) => { visibleText += value; @@ -547,7 +575,7 @@ function sanitizeTerminalHistoryChunk( if (codePoint === 0x1b) { const nextCodePoint = input.charCodeAt(index + 1); if (Number.isNaN(nextCodePoint)) { - return result(input.slice(index)); + return result(index); } if (nextCodePoint === 0x5b) { @@ -565,7 +593,7 @@ function sanitizeTerminalHistoryChunk( cursor += 1; } if (cursor >= input.length) { - return result(input.slice(index)); + return result(index); } continue; } @@ -578,12 +606,12 @@ function sanitizeTerminalHistoryChunk( ) { const terminatorIndex = findStringTerminatorIndex(input, index + 2); if (terminatorIndex === null) { - return result(input.slice(index)); + return result(index); } const sequence = input.slice(index, terminatorIndex); const content = stripStringTerminator(input.slice(index + 2, terminatorIndex)); if (nextCodePoint === 0x5d && content === COMMAND_FINISHED_OSC_CONTENT) { - commandFinished = true; + commandFinishedGenerations.push(generationAt(index)); } if (nextCodePoint !== 0x5d || !shouldStripOscSequence(content)) { append(sequence); @@ -594,7 +622,7 @@ function sanitizeTerminalHistoryChunk( const escapeSequenceEndIndex = findEscapeSequenceEndIndex(input, index + 1); if (escapeSequenceEndIndex === null) { - return result(input.slice(index)); + return result(index); } append(input.slice(index, escapeSequenceEndIndex)); index = escapeSequenceEndIndex; @@ -616,7 +644,7 @@ function sanitizeTerminalHistoryChunk( cursor += 1; } if (cursor >= input.length) { - return result(input.slice(index)); + return result(index); } continue; } @@ -624,12 +652,12 @@ function sanitizeTerminalHistoryChunk( if (codePoint === 0x9d || codePoint === 0x90 || codePoint === 0x9e || codePoint === 0x9f) { const terminatorIndex = findStringTerminatorIndex(input, index + 1); if (terminatorIndex === null) { - return result(input.slice(index)); + return result(index); } const sequence = input.slice(index, terminatorIndex); const content = stripStringTerminator(input.slice(index + 1, terminatorIndex)); if (codePoint === 0x9d && content === COMMAND_FINISHED_OSC_CONTENT) { - commandFinished = true; + commandFinishedGenerations.push(generationAt(index)); } if (codePoint !== 0x9d || !shouldStripOscSequence(content)) { append(sequence); @@ -642,7 +670,7 @@ function sanitizeTerminalHistoryChunk( index += 1; } - return result(""); + return result(null); } function legacySafeThreadId(threadId: string): string { @@ -1425,6 +1453,7 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith const sanitized = sanitizeTerminalHistoryChunk( session.pendingHistoryControlSequence, nextEvent.data, + nextEvent.commandGeneration, ); session.pendingHistoryControlSequence = sanitized.pendingControlSequence; if (sanitized.visibleText.length > 0) { @@ -1435,7 +1464,7 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith } const commandFinished = session.commandCompletionMarkersEnabled && - sanitized.commandFinished && + sanitized.commandFinishedGenerations.includes(session.commandGeneration) && session.hasRunningSubprocess; if (commandFinished) { session.hasRunningSubprocess = false; @@ -1469,7 +1498,7 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith session.commandCompletionMarkersEnabled = false; session.terminalCommandInputState = createTerminalCommandInputState(); session.status = "exited"; - session.pendingHistoryControlSequence = ""; + session.pendingHistoryControlSequence = null; session.pendingProcessEvents = []; session.pendingProcessEventIndex = 0; session.processEventDrainRunning = false; @@ -1555,7 +1584,7 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith session.commandCompletionMarkersEnabled = false; session.terminalCommandInputState = createTerminalCommandInputState(); session.status = "exited"; - session.pendingHistoryControlSequence = ""; + session.pendingHistoryControlSequence = null; session.pendingProcessEvents = []; session.pendingProcessEventIndex = 0; session.processEventDrainRunning = false; @@ -1658,10 +1687,12 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith session.hasRunningSubprocess = false; session.runningSubprocessCommand = null; session.submittedCommand = null; + session.commandGeneration = 0; session.subprocessPollingArmed = false; session.consecutiveSubprocessIdlePolls = 0; session.commandCompletionMarkersEnabled = false; session.terminalCommandInputState = createTerminalCommandInputState(); + session.pendingHistoryControlSequence = null; session.pendingProcessEvents = []; session.pendingProcessEventIndex = 0; session.processEventDrainRunning = false; @@ -1688,7 +1719,16 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith const processPid = ptyProcess.pid; const unsubscribeData = ptyProcess.onData((data) => { - if (!enqueueProcessEvent(session, processPid, { type: "output", data })) { + // PTY callbacks can outpace the event drain. Preserve the + // command generation from receipt time so an initial or prior + // prompt marker cannot finish a command submitted afterward. + if ( + !enqueueProcessEvent(session, processPid, { + type: "output", + data, + commandGeneration: session.commandGeneration, + }) + ) { return; } runFork(drainProcessEvents(session, processPid)); @@ -1746,10 +1786,12 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith session.hasRunningSubprocess = false; session.runningSubprocessCommand = null; session.submittedCommand = null; + session.commandGeneration = 0; session.subprocessPollingArmed = false; session.consecutiveSubprocessIdlePolls = 0; session.commandCompletionMarkersEnabled = false; session.terminalCommandInputState = createTerminalCommandInputState(); + session.pendingHistoryControlSequence = null; session.pendingProcessEvents = []; session.pendingProcessEventIndex = 0; session.processEventDrainRunning = false; @@ -2060,7 +2102,7 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith status: "starting", pid: null, history, - pendingHistoryControlSequence: "", + pendingHistoryControlSequence: null, pendingProcessEvents: [], pendingProcessEventIndex: 0, processEventDrainRunning: false, @@ -2075,6 +2117,7 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith hasRunningSubprocess: false, runningSubprocessCommand: null, submittedCommand: null, + commandGeneration: 0, subprocessPollingArmed: false, consecutiveSubprocessIdlePolls: 0, commandCompletionMarkersEnabled: false, @@ -2119,7 +2162,7 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith liveSession.worktreePath = input.worktreePath ?? null; liveSession.runtimeEnv = nextRuntimeEnv; liveSession.history = ""; - liveSession.pendingHistoryControlSequence = ""; + liveSession.pendingHistoryControlSequence = null; liveSession.pendingProcessEvents = []; liveSession.pendingProcessEventIndex = 0; liveSession.processEventDrainRunning = false; @@ -2135,7 +2178,7 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith liveSession.runtimeEnv = nextRuntimeEnv; liveSession.worktreePath = input.worktreePath ?? null; liveSession.history = ""; - liveSession.pendingHistoryControlSequence = ""; + liveSession.pendingHistoryControlSequence = null; liveSession.pendingProcessEvents = []; liveSession.pendingProcessEventIndex = 0; liveSession.processEventDrainRunning = false; @@ -2197,6 +2240,7 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith // Keep checking this PTY after a submitted command even if an early // process snapshot misses its child. This also covers commands recalled // through shell history, whose text cannot be reconstructed here. + session.commandGeneration += 1; session.subprocessPollingArmed = true; session.consecutiveSubprocessIdlePolls = 0; if (session.commandCompletionMarkersEnabled && session.pid !== null) { @@ -2239,7 +2283,7 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith const session = yield* requireSession(input.threadId, terminalId); const updatedAt = yield* nowIso; session.history = ""; - session.pendingHistoryControlSequence = ""; + session.pendingHistoryControlSequence = null; session.pendingProcessEvents = []; session.pendingProcessEventIndex = 0; session.processEventDrainRunning = false; @@ -2278,7 +2322,7 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith status: "starting", pid: null, history: "", - pendingHistoryControlSequence: "", + pendingHistoryControlSequence: null, pendingProcessEvents: [], pendingProcessEventIndex: 0, processEventDrainRunning: false, @@ -2293,6 +2337,7 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith hasRunningSubprocess: false, runningSubprocessCommand: null, submittedCommand: null, + commandGeneration: 0, subprocessPollingArmed: false, consecutiveSubprocessIdlePolls: 0, commandCompletionMarkersEnabled: false, @@ -2318,7 +2363,7 @@ export const makeTerminalManagerWithOptions = Effect.fn("makeTerminalManagerWith const rows = input.rows ?? session.rows; session.history = ""; - session.pendingHistoryControlSequence = ""; + session.pendingHistoryControlSequence = null; session.pendingProcessEvents = []; session.pendingProcessEventIndex = 0; session.processEventDrainRunning = false;