From fc97550ccabb007524c6025f2f0e6e6910ea7583 Mon Sep 17 00:00:00 2001 From: "omegent-app[bot]" <306514130+omegent-app[bot]@users.noreply.github.com> Date: Sat, 25 Jul 2026 12:46:20 +0000 Subject: [PATCH 1/5] fix(discord-bot): make Discord thread title sync durable and race-safe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stop VCS callbacks from re-applying ⏳ after settle using a stale running thread, keep a pending desired title for rate-limit/timeout retries, and retry desynced titles on a periodic tick so idle threads no longer stay wrong. --- .../src/features/ResponseBridge.test.ts | 103 +++ .../src/features/ResponseBridge.ts | 603 ++++++++++++------ 2 files changed, 515 insertions(+), 191 deletions(-) diff --git a/apps/discord-bot/src/features/ResponseBridge.test.ts b/apps/discord-bot/src/features/ResponseBridge.test.ts index 147830bcf26..ec7c61eb283 100644 --- a/apps/discord-bot/src/features/ResponseBridge.test.ts +++ b/apps/discord-bot/src/features/ResponseBridge.test.ts @@ -62,6 +62,8 @@ import { resolveTemporaryDiscordThreadTitleBadge, resolveThreadChangeRequestLookupCwds, mergeStickyTitlePr, + nextMirroredThreadTitleAfterApply, + planDiscordThreadTitleApply, shouldApplyDiscordThreadPrBadge, shouldApplyDiscordThreadTitleBadge, shouldConvertWorkingTipsToWakeUp, @@ -2035,6 +2037,107 @@ describe("resolveSettledDiscordThreadTitleUpgrade", () => { }); }); +describe("planDiscordThreadTitleApply", () => { + it("applies a freshly computed title that Discord does not have yet", () => { + expect( + planDiscordThreadTitleApply({ + mirroredThreadTitle: "πŸ”€ Empasa pickup carrier rollout", + pendingDesiredThreadTitle: null, + computedDesiredTitle: "πŸ”€ ⏳ Empasa pickup carrier rollout", + }), + ).toEqual({ + pendingDesiredThreadTitle: "πŸ”€ ⏳ Empasa pickup carrier rollout", + applyTitle: "πŸ”€ ⏳ Empasa pickup carrier rollout", + }); + }); + + it("clears pending when compute says Discord already matches", () => { + expect( + planDiscordThreadTitleApply({ + mirroredThreadTitle: "πŸ”€ Empasa pickup carrier rollout", + pendingDesiredThreadTitle: "πŸ”€ ⏳ Empasa pickup carrier rollout", + computedDesiredTitle: "πŸ”€ Empasa pickup carrier rollout", + }), + ).toEqual({ + pendingDesiredThreadTitle: null, + applyTitle: null, + }); + }); + + it("retries a prior failed rename when compute has nothing new", () => { + // Turn settled, secondary timed out mid-rename β€” heartbeat must re-apply settle. + expect( + planDiscordThreadTitleApply({ + mirroredThreadTitle: "πŸ”€ ⏳ Empasa pickup carrier rollout", + pendingDesiredThreadTitle: "πŸ”€ Empasa pickup carrier rollout", + computedDesiredTitle: null, + }), + ).toEqual({ + pendingDesiredThreadTitle: "πŸ”€ Empasa pickup carrier rollout", + applyTitle: "πŸ”€ Empasa pickup carrier rollout", + }); + }); + + it("does not re-apply when mirrored already matches pending", () => { + expect( + planDiscordThreadTitleApply({ + mirroredThreadTitle: "πŸ”€ Empasa pickup carrier rollout", + pendingDesiredThreadTitle: "πŸ”€ Empasa pickup carrier rollout", + computedDesiredTitle: null, + }), + ).toEqual({ + pendingDesiredThreadTitle: null, + applyTitle: null, + }); + }); + + it("prefers a newer computed settle over a stale pending busy title", () => { + // VCS raced with settle: pending still wants ⏳, compute now wants settled. + expect( + planDiscordThreadTitleApply({ + mirroredThreadTitle: "πŸ”€ ⏳ Empasa pickup carrier rollout", + pendingDesiredThreadTitle: "πŸ”€ ⏳ Empasa pickup carrier rollout", + computedDesiredTitle: "πŸ”€ Empasa pickup carrier rollout", + }), + ).toEqual({ + pendingDesiredThreadTitle: "πŸ”€ Empasa pickup carrier rollout", + applyTitle: "πŸ”€ Empasa pickup carrier rollout", + }); + }); +}); + +describe("nextMirroredThreadTitleAfterApply", () => { + it("keeps pending and leaves mirrored unchanged on REST failure", () => { + expect( + nextMirroredThreadTitleAfterApply({ + mirroredThreadTitle: "πŸ”€ Empasa pickup carrier rollout", + pendingDesiredThreadTitle: "πŸ”€ ⏳ Empasa pickup carrier rollout", + appliedTitle: "πŸ”€ ⏳ Empasa pickup carrier rollout", + success: false, + }), + ).toEqual({ + mirroredThreadTitle: "πŸ”€ Empasa pickup carrier rollout", + pendingDesiredThreadTitle: "πŸ”€ ⏳ Empasa pickup carrier rollout", + attemptedThreadTitle: null, + }); + }); + + it("updates mirrored and clears matching pending on success", () => { + expect( + nextMirroredThreadTitleAfterApply({ + mirroredThreadTitle: "πŸ”€ ⏳ Empasa pickup carrier rollout", + pendingDesiredThreadTitle: "πŸ”€ Empasa pickup carrier rollout", + appliedTitle: "πŸ”€ Empasa pickup carrier rollout", + success: true, + }), + ).toEqual({ + mirroredThreadTitle: "πŸ”€ Empasa pickup carrier rollout", + pendingDesiredThreadTitle: null, + attemptedThreadTitle: "πŸ”€ Empasa pickup carrier rollout", + }); + }); +}); + describe("resolveTemporaryDiscordThreadTitleBadge", () => { it("returns busy while a turn is Working", () => { expect( diff --git a/apps/discord-bot/src/features/ResponseBridge.ts b/apps/discord-bot/src/features/ResponseBridge.ts index 1f2a5a4abff..b482f2042d1 100644 --- a/apps/discord-bot/src/features/ResponseBridge.ts +++ b/apps/discord-bot/src/features/ResponseBridge.ts @@ -1634,6 +1634,78 @@ export function resolveTemporaryDiscordThreadTitleBadge(input: { return resolveDiscordThreadActivityBadgeState(input); } +/** + * Desired-state plan for Discord thread renames. + * + * Title apply must be durable and coalesced: + * - Never apply a previously captured thread snapshot (VCS races re-painted ⏳ + * after settle when the callback held a stale "running" thread). + * - On REST failure / rate-limit / interrupt, keep `pending` so a later tick retries. + * - Only clear pending when Discord already shows that exact name. + */ +export function planDiscordThreadTitleApply(input: { + readonly mirroredThreadTitle: string | null; + readonly pendingDesiredThreadTitle: string | null; + /** Freshly computed desired name, or null when compute has nothing new. */ + readonly computedDesiredTitle: string | null; +}): { + readonly pendingDesiredThreadTitle: string | null; + readonly applyTitle: string | null; +} { + if (input.computedDesiredTitle !== null) { + if (input.computedDesiredTitle === input.mirroredThreadTitle) { + return { pendingDesiredThreadTitle: null, applyTitle: null }; + } + return { + pendingDesiredThreadTitle: input.computedDesiredTitle, + applyTitle: input.computedDesiredTitle, + }; + } + // No fresh compute β€” retry a prior failed apply if Discord still lags. + if ( + input.pendingDesiredThreadTitle !== null && + input.pendingDesiredThreadTitle !== input.mirroredThreadTitle + ) { + return { + pendingDesiredThreadTitle: input.pendingDesiredThreadTitle, + applyTitle: input.pendingDesiredThreadTitle, + }; + } + return { pendingDesiredThreadTitle: null, applyTitle: null }; +} + +/** + * Commit or roll back after a Discord channel rename attempt. + * Success updates mirrored; failure keeps pending so settle/busy is retried. + */ +export function nextMirroredThreadTitleAfterApply(input: { + readonly mirroredThreadTitle: string | null; + readonly pendingDesiredThreadTitle: string | null; + readonly appliedTitle: string; + readonly success: boolean; +}): { + readonly mirroredThreadTitle: string | null; + readonly pendingDesiredThreadTitle: string | null; + readonly attemptedThreadTitle: string | null; +} { + if (!input.success) { + return { + mirroredThreadTitle: input.mirroredThreadTitle, + pendingDesiredThreadTitle: input.appliedTitle, + // Do not poison attempted with a name Discord never accepted. + attemptedThreadTitle: null, + }; + } + const pendingCleared = + input.pendingDesiredThreadTitle === null || + input.pendingDesiredThreadTitle === input.appliedTitle; + return { + mirroredThreadTitle: input.appliedTitle, + pendingDesiredThreadTitle: pendingCleared ? null : input.pendingDesiredThreadTitle, + attemptedThreadTitle: input.appliedTitle, + }; +} + export function resolveThreadTitleChangeRequestFromStatus( thread: Pick, status: VcsStatusResult | null, @@ -2139,6 +2211,12 @@ export const runBridge = ( const streamWriteLock = yield* Semaphore.make(1); const titleSyncLock = yield* Semaphore.make(1); + /** + * Latest desired Discord thread name that has not been confirmed on Discord yet. + * Survives REST failures / secondary timeouts so heartbeat can retry without + * waiting for another T3 snapshot (idle threads used to stay on ⏳ forever). + */ + const pendingDesiredThreadTitleRef = yield* Ref.make(null); // Seed title settle cache from Discord's live name so rehydrate demotion guards // work immediately (in-memory mirrored starts null after every bridge restart). @@ -3890,236 +3968,323 @@ export const runBridge = ( /** * Apply a Discord thread title only after a successful rename. - * Never mark `attemptedThreadTitle` before the REST call β€” a rate-limit / - * network failure used to poison retries so busy/wake badges never appeared. + * Never mark `attemptedThreadTitle` / `mirroredThreadTitle` before the REST call β€” + * a rate-limit / network failure used to poison retries so busy/wake badges never + * appeared. Failed applies keep `pendingDesiredThreadTitle` for heartbeat retry. + * + * Must run under `titleSyncLock`. */ const applyDiscordThreadTitle = (desiredTitle: string, reason: string) => Effect.gen(function* () { const latest = yield* Ref.get(stateRef); if (latest.mirroredThreadTitle === desiredTitle) { + yield* Ref.set(pendingDesiredThreadTitleRef, null); + return; + } + // Record desired *before* REST so interrupt/timeout/rate-limit still retries. + yield* Ref.set(pendingDesiredThreadTitleRef, desiredTitle); + const result = yield* rest + .updateChannel(input.discordChannelId, { name: desiredTitle }) + .pipe(Effect.result); + const pending = yield* Ref.get(pendingDesiredThreadTitleRef); + if (Result.isFailure(result)) { + const afterFail = nextMirroredThreadTitleAfterApply({ + mirroredThreadTitle: latest.mirroredThreadTitle, + pendingDesiredThreadTitle: pending, + appliedTitle: desiredTitle, + success: false, + }); + yield* Ref.set(pendingDesiredThreadTitleRef, afterFail.pendingDesiredThreadTitle); + yield* Effect.logWarning("Discord thread title rename failed; will retry", { + discordChannelId: input.discordChannelId, + t3ThreadId: input.t3ThreadId, + desiredTitle, + reason, + cause: formatAlertCause(result.failure, 300), + }); return; } - yield* rest.updateChannel(input.discordChannelId, { name: desiredTitle }); + const afterOk = nextMirroredThreadTitleAfterApply({ + mirroredThreadTitle: latest.mirroredThreadTitle, + pendingDesiredThreadTitle: pending, + appliedTitle: desiredTitle, + success: true, + }); yield* Effect.logInfo("Discord thread title mirrored to Discord", { discordChannelId: input.discordChannelId, t3ThreadId: input.t3ThreadId, desiredTitle, reason, }); + yield* Ref.set(pendingDesiredThreadTitleRef, afterOk.pendingDesiredThreadTitle); yield* Ref.update(stateRef, (current) => ({ ...current, - attemptedThreadTitle: desiredTitle, - mirroredThreadTitle: desiredTitle, + attemptedThreadTitle: afterOk.attemptedThreadTitle ?? desiredTitle, + mirroredThreadTitle: afterOk.mirroredThreadTitle, })); }); /** - * Idempotent title badge sync. + * Idempotent, desired-state title badge sync. * * Dual optional columns (like T3 client): `| PR | activity | Title`. * Activity (busy / wake-required) is independent of sticky PR evidence. * Unknown remote never paints ▫️. + * + * **Always re-reads `latestThreadRef` under the lock** β€” never apply a caller- + * captured thread snapshot for activity. VCS callbacks used to re-paint ⏳ after + * settle when they held a stale "running" thread across the lock queue. + * + * Callers must publish the latest orchestration thread to `latestThreadRef` + * (via `ensureVcsStatusSubscription` / snapshot path) *before* invoking this. */ - const syncDiscordThreadTitle = (thread: OrchestrationThread) => + const syncDiscordThreadTitle = () => titleSyncLock .withPermit( Effect.gen(function* () { - const state = yield* Ref.get(stateRef); - const titleBase = (value: string) => - decorateDiscordThreadTitle(value, null, 100, false); - const currentBase = titleBase(thread.title); - const alreadySettled = - (state.mirroredThreadTitle !== null && - titleBase(state.mirroredThreadTitle) === currentBase) || - (state.attemptedThreadTitle !== null && - titleBase(state.attemptedThreadTitle) === currentBase); - - const cachedStatus = yield* Ref.get(latestVcsStatusRef); - const remoteObserved = yield* Ref.get(vcsRemoteObservedRef); - const stickyBefore = yield* Ref.get(stickyTitlePrRef); - const statusPr = toStickyTitlePrEvidence( - resolveThreadTitleChangeRequestFromStatus(thread, cachedStatus), - ); + const commitComputedTitle = (desiredTitle: string | null, reason: string) => + Effect.gen(function* () { + const latest = yield* Ref.get(stateRef); + const pending = yield* Ref.get(pendingDesiredThreadTitleRef); + const plan = planDiscordThreadTitleApply({ + mirroredThreadTitle: latest.mirroredThreadTitle, + pendingDesiredThreadTitle: pending, + computedDesiredTitle: desiredTitle, + }); + yield* Ref.set(pendingDesiredThreadTitleRef, plan.pendingDesiredThreadTitle); + if (plan.applyTitle === null) return; + yield* applyDiscordThreadTitle(plan.applyTitle, reason); + }); - const activity = resolveDiscordThreadActivityBadgeState({ - sessionStatus: thread.session?.status ?? null, - activeTurnId: thread.session?.activeTurnId ?? null, - latestTurnState: thread.latestTurn?.state ?? null, - latestTurnCompletedAt: thread.latestTurn?.completedAt ?? null, - }); + // Coalesce: re-read the latest thread under the lock before every compute + // and again right before apply so a settle that landed while we held the + // lock (or during GH lookup) always wins over a stale busy state. + for (let attempt = 0; attempt < 3; attempt += 1) { + const thread = yield* Ref.get(latestThreadRef); + if (thread === null) { + const pendingOnly = yield* Ref.get(pendingDesiredThreadTitleRef); + const stateOnly = yield* Ref.get(stateRef); + const planOnly = planDiscordThreadTitleApply({ + mirroredThreadTitle: stateOnly.mirroredThreadTitle, + pendingDesiredThreadTitle: pendingOnly, + computedDesiredTitle: null, + }); + yield* Ref.set(pendingDesiredThreadTitleRef, planOnly.pendingDesiredThreadTitle); + if (planOnly.applyTitle !== null) { + yield* applyDiscordThreadTitle(planOnly.applyTitle, "pending-retry-no-thread"); + } + return; + } + + const state = yield* Ref.get(stateRef); + const titleBase = (value: string) => + decorateDiscordThreadTitle(value, null, 100, false); + const currentBase = titleBase(thread.title); + const alreadySettled = + (state.mirroredThreadTitle !== null && + titleBase(state.mirroredThreadTitle) === currentBase) || + (state.attemptedThreadTitle !== null && + titleBase(state.attemptedThreadTitle) === currentBase); + + const cachedStatus = yield* Ref.get(latestVcsStatusRef); + const remoteObserved = yield* Ref.get(vcsRemoteObservedRef); + const stickyBefore = yield* Ref.get(stickyTitlePrRef); + const statusPr = toStickyTitlePrEvidence( + resolveThreadTitleChangeRequestFromStatus(thread, cachedStatus), + ); + + // Fast path: settled base + no new PR evidence from warm VCS β€” compose + // PR (sticky) + activity without GH dual-lookup. Covers turn start/end + // busy toggles so settle never waits on branch lookup. + if (alreadySettled && statusPr === null) { + const evidence = resolveDiscordTitlePrEvidence({ + stickyPr: stickyBefore, + statusPr: null, + remoteStatusObserved: remoteObserved, + }); + yield* Ref.set(stickyTitlePrRef, evidence.stickyPr); + + // Re-read immediately before decorating activity so we do not paint ⏳ + // from a thread that settled while we waited for this lock. + const threadNow = (yield* Ref.get(latestThreadRef)) ?? thread; + if (threadNow !== thread && attempt < 2) { + continue; + } + const upgradeTitle = resolveSettledDiscordThreadTitleUpgrade({ + thread: threadNow, + mirroredThreadTitle: state.mirroredThreadTitle, + attemptedThreadTitle: state.attemptedThreadTitle, + cachedPr: evidence.effectivePr, + canApplyNoPrBadge: evidence.canApplyNoPrBadge, + }); + const activityNow = resolveDiscordThreadActivityBadgeState({ + sessionStatus: threadNow.session?.status ?? null, + activeTurnId: threadNow.session?.activeTurnId ?? null, + latestTurnState: threadNow.latestTurn?.state ?? null, + latestTurnCompletedAt: threadNow.latestTurn?.completedAt ?? null, + }); + yield* commitComputedTitle( + upgradeTitle, + activityNow !== null ? "dual-badge-settled" : "settled-title-badge-upgrade", + ); + return; + } + + const project = yield* t3.getProjectShell(thread.projectId); + const branch = thread.branch; + + // Prefer warm VCS stream. Only cold-start GH lookup when remote is unknown + // and sticky is empty β€” never dual-source every snapshot. + let branchLookupPr: StickyTitlePrEvidence | null = null; + let branchLookupCompleted = false; + let prSource: "vcs-status-stream" | "sticky" | "branch-lookup" | "none" = "none"; + let lookupCwds: ReadonlyArray = []; + + if (statusPr !== null) { + prSource = "vcs-status-stream"; + } else if (stickyBefore !== null) { + prSource = "sticky"; + } else if (!remoteObserved && branch !== null && project !== null) { + lookupCwds = resolveThreadChangeRequestLookupCwds(thread, project); + for (const cwd of lookupCwds) { + const resolved = yield* t3 + .resolveBranchChangeRequest({ + cwd, + refName: branch, + }) + .pipe( + Effect.catchCause((cause) => + Effect.logWarning("Discord thread title PR lookup failed", { + discordChannelId: input.discordChannelId, + t3ThreadId: input.t3ThreadId, + title: thread.title, + branch, + cwd, + cause: formatAlertCause(cause, 400), + }).pipe(Effect.as({ pr: null })), + ), + ); + yield* Effect.logInfo("Discord thread title PR lookup result", { + discordChannelId: input.discordChannelId, + t3ThreadId: input.t3ThreadId, + title: thread.title, + branch, + cwd, + prNumber: resolved.pr?.number ?? null, + prState: resolved.pr?.state ?? null, + prHeadRef: resolved.pr?.headRef ?? null, + prBaseRef: resolved.pr?.baseRef ?? null, + }); + if (resolved.pr !== null) { + branchLookupPr = toStickyTitlePrEvidence(resolved.pr); + prSource = "branch-lookup"; + break; + } + } + // Completed the cold-start lookup path (possibly with no PR). + branchLookupCompleted = true; + if (prSource === "none") prSource = "branch-lookup"; + } else if (branch === null || project === null) { + yield* Effect.logInfo("Discord thread title sync skipping PR lookup", { + discordChannelId: input.discordChannelId, + t3ThreadId: input.t3ThreadId, + title: thread.title, + branch, + hasProject: project !== null, + worktreePath: thread.worktreePath, + }); + } - // Fast path: settled base + no new PR evidence from warm VCS β€” compose - // PR (sticky) + activity without GH dual-lookup. - if (alreadySettled && statusPr === null) { const evidence = resolveDiscordTitlePrEvidence({ stickyPr: stickyBefore, - statusPr: null, + statusPr, remoteStatusObserved: remoteObserved, + branchLookupPr, + branchLookupCompleted, }); yield* Ref.set(stickyTitlePrRef, evidence.stickyPr); - const upgradeTitle = resolveSettledDiscordThreadTitleUpgrade({ - thread, - mirroredThreadTitle: state.mirroredThreadTitle, - attemptedThreadTitle: state.attemptedThreadTitle, - cachedPr: evidence.effectivePr, - canApplyNoPrBadge: evidence.canApplyNoPrBadge, + + // Re-read after slow GH lookup β€” settle may have landed mid-wait. + const threadAfterLookup = (yield* Ref.get(latestThreadRef)) ?? thread; + if (threadAfterLookup !== thread && attempt < 2) { + continue; + } + + const activityAfterLookup = resolveDiscordThreadActivityBadgeState({ + sessionStatus: threadAfterLookup.session?.status ?? null, + activeTurnId: threadAfterLookup.session?.activeTurnId ?? null, + latestTurnState: threadAfterLookup.latestTurn?.state ?? null, + latestTurnCompletedAt: threadAfterLookup.latestTurn?.completedAt ?? null, }); - if (upgradeTitle === null) return; - yield* applyDiscordThreadTitle( - upgradeTitle, - activity !== null ? "dual-badge-settled" : "settled-title-badge-upgrade", - ); - return; - } - const project = yield* t3.getProjectShell(thread.projectId); - const branch = thread.branch; - - // Prefer warm VCS stream. Only cold-start GH lookup when remote is unknown - // and sticky is empty β€” never dual-source every snapshot. - let branchLookupPr: StickyTitlePrEvidence | null = null; - let branchLookupCompleted = false; - let prSource: "vcs-status-stream" | "sticky" | "branch-lookup" | "none" = "none"; - let lookupCwds: ReadonlyArray = []; - - if (statusPr !== null) { - prSource = "vcs-status-stream"; - } else if (stickyBefore !== null) { - prSource = "sticky"; - } else if (!remoteObserved && branch !== null && project !== null) { - lookupCwds = resolveThreadChangeRequestLookupCwds(thread, project); - for (const cwd of lookupCwds) { - const resolved = yield* t3 - .resolveBranchChangeRequest({ - cwd, - refName: branch, - }) - .pipe( - Effect.catchCause((cause) => - Effect.logWarning("Discord thread title PR lookup failed", { - discordChannelId: input.discordChannelId, - t3ThreadId: input.t3ThreadId, - title: thread.title, - branch, - cwd, - cause: formatAlertCause(cause, 400), - }).pipe(Effect.as({ pr: null })), - ), - ); - yield* Effect.logInfo("Discord thread title PR lookup result", { + const prState = threadTitleChangeRequestState( + threadAfterLookup, + evidence.effectivePr, + ); + // Unknown evidence: do not paint ▫️ / no-PR from missing data. + const effectivePrState: DiscordThreadPrBadgeState = + prState === "initialized" && !evidence.canApplyNoPrBadge ? null : prState; + + const latest = yield* Ref.get(stateRef); + const mirroredBadges = parseDiscordThreadTitleBadges(latest.mirroredThreadTitle); + const currentBadges = + mirroredBadges.pr !== null || mirroredBadges.activity !== null + ? mirroredBadges + : parseDiscordThreadTitleBadges(latest.attemptedThreadTitle); + const prAllowed = shouldApplyDiscordThreadPrBadge(currentBadges.pr, effectivePrState); + const appliedPr = prAllowed ? effectivePrState : currentBadges.pr; + + // Still nothing actionable (no PR column, no activity, nothing mirrored). + if (appliedPr === null && activityAfterLookup === null && currentBadges.pr === null) { + yield* Effect.logInfo("Discord thread title sync deferred; PR evidence not ready", { discordChannelId: input.discordChannelId, t3ThreadId: input.t3ThreadId, - title: thread.title, - branch, - cwd, - prNumber: resolved.pr?.number ?? null, - prState: resolved.pr?.state ?? null, - prHeadRef: resolved.pr?.headRef ?? null, - prBaseRef: resolved.pr?.baseRef ?? null, + remoteObserved, + stickyPr: evidence.stickyPr?.number ?? null, + prSource, + activity: activityAfterLookup, }); - if (resolved.pr !== null) { - branchLookupPr = toStickyTitlePrEvidence(resolved.pr); - prSource = "branch-lookup"; - break; - } + yield* commitComputedTitle(null, "pending-retry-deferred"); + return; } - // Completed the cold-start lookup path (possibly with no PR). - branchLookupCompleted = true; - if (prSource === "none") prSource = "branch-lookup"; - } else if (branch === null || project === null) { - yield* Effect.logInfo("Discord thread title sync skipping PR lookup", { - discordChannelId: input.discordChannelId, - t3ThreadId: input.t3ThreadId, - title: thread.title, - branch, - hasProject: project !== null, - worktreePath: thread.worktreePath, - }); - } - const evidence = resolveDiscordTitlePrEvidence({ - stickyPr: stickyBefore, - statusPr, - remoteStatusObserved: remoteObserved, - branchLookupPr, - branchLookupCompleted, - }); - yield* Ref.set(stickyTitlePrRef, evidence.stickyPr); - - const prState = threadTitleChangeRequestState(thread, evidence.effectivePr); - // Unknown evidence: do not paint ▫️ / no-PR from missing data. - const effectivePrState: DiscordThreadPrBadgeState = - prState === "initialized" && !evidence.canApplyNoPrBadge ? null : prState; - - const latest = yield* Ref.get(stateRef); - const mirroredBadges = parseDiscordThreadTitleBadges(latest.mirroredThreadTitle); - const currentBadges = - mirroredBadges.pr !== null || mirroredBadges.activity !== null - ? mirroredBadges - : parseDiscordThreadTitleBadges(latest.attemptedThreadTitle); - const prAllowed = shouldApplyDiscordThreadPrBadge(currentBadges.pr, effectivePrState); - const appliedPr = prAllowed ? effectivePrState : currentBadges.pr; - - // Still nothing actionable (no PR column, no activity, nothing mirrored). - if (appliedPr === null && activity === null && currentBadges.pr === null) { - yield* Effect.logInfo("Discord thread title sync deferred; PR evidence not ready", { + const desiredTitle = decorateDiscordThreadTitle( + threadAfterLookup.title, + { + pr: appliedPr, + activity: activityAfterLookup, + hasFailingChecks: + appliedPr === "open" && evidence.effectivePr?.hasFailingChecks === true, + }, + 100, + ); + yield* Effect.logInfo("Discord thread title sync resolved", { discordChannelId: input.discordChannelId, t3ThreadId: input.t3ThreadId, - remoteObserved, - stickyPr: evidence.stickyPr?.number ?? null, + title: threadAfterLookup.title, + branch, + worktreePath: threadAfterLookup.worktreePath, + projectWorkspaceRoot: project?.workspaceRoot ?? null, + lookupCwds, + cachedStatusRefName: cachedStatus?.refName ?? null, + prNumber: evidence.effectivePr?.number ?? null, + prState: effectivePrState, + appliedPr, + activity: activityAfterLookup, + sessionStatus: threadAfterLookup.session?.status ?? null, prSource, - activity, + remoteObserved, + canApplyNoPrBadge: evidence.canApplyNoPrBadge, + desiredTitle, + currentBadges, + prAllowed, + mirroredThreadTitle: latest.mirroredThreadTitle, + attemptedThreadTitle: latest.attemptedThreadTitle, }); - return; - } - const desiredTitle = decorateDiscordThreadTitle( - thread.title, - { - pr: appliedPr, - activity, - hasFailingChecks: - appliedPr === "open" && evidence.effectivePr?.hasFailingChecks === true, - }, - 100, - ); - yield* Effect.logInfo("Discord thread title sync resolved", { - discordChannelId: input.discordChannelId, - t3ThreadId: input.t3ThreadId, - title: thread.title, - branch, - worktreePath: thread.worktreePath, - projectWorkspaceRoot: project?.workspaceRoot ?? null, - lookupCwds, - cachedStatusRefName: cachedStatus?.refName ?? null, - prNumber: evidence.effectivePr?.number ?? null, - prState: effectivePrState, - appliedPr, - activity, - sessionStatus: thread.session?.status ?? null, - prSource, - remoteObserved, - canApplyNoPrBadge: evidence.canApplyNoPrBadge, - desiredTitle, - currentBadges, - prAllowed, - mirroredThreadTitle: latest.mirroredThreadTitle, - attemptedThreadTitle: latest.attemptedThreadTitle, - }); - // Only skip when Discord already shows the desired dual-slot title. - if (latest.mirroredThreadTitle === desiredTitle) { + yield* commitComputedTitle(desiredTitle, "title-sync"); return; } - const mirroredBase = - latest.mirroredThreadTitle === null ? null : titleBase(latest.mirroredThreadTitle); - const sameTitleBase = mirroredBase === currentBase; - // Refuse pure PR demotion when base unchanged β€” but still allow activity-only - // changes (e.g. keep πŸ”€ and add/remove ⏳). - if (!prAllowed && sameTitleBase && appliedPr === currentBadges.pr) { - // appliedPr kept sticky current; activity may still differ β€” continue apply. - } - - yield* applyDiscordThreadTitle(desiredTitle, "title-sync"); }), ) .pipe( @@ -4127,7 +4292,6 @@ export const runBridge = ( Effect.logWarning("Failed to mirror T3 thread title to Discord thread", { discordChannelId: input.discordChannelId, t3ThreadId: input.t3ThreadId, - title: thread.title, cause: formatAlertCause(cause, 400), }), ), @@ -4173,7 +4337,8 @@ export const runBridge = ( ); } - yield* syncDiscordThreadTitle(thread); + yield* Ref.set(latestThreadRef, thread); + yield* syncDiscordThreadTitle(); const state = yield* Ref.get(stateRef); if (state.mirroredThreadTitle !== null) { yield* Effect.logInfo("Discord thread indicators refreshed", { @@ -4262,8 +4427,10 @@ export const runBridge = ( yield* Ref.update(latestVcsStatusRef, (current) => applyGitStatusStreamEvent(current, event), ); - const currentThread = yield* Ref.get(latestThreadRef); - if (currentThread === null) return; + // Do not capture a thread snapshot here β€” activity must be re-read under + // titleSyncLock from latestThreadRef so a concurrent settle cannot lose to + // a stale "running" VCS callback (⏳ flip-flop after the turn ends). + if ((yield* Ref.get(latestThreadRef)) === null) return; yield* Effect.logInfo("Discord thread title VCS status update received", { discordChannelId: input.discordChannelId, @@ -4274,7 +4441,7 @@ export const runBridge = ( event._tag === "remoteUpdated" || (event._tag === "snapshot" && event.remote !== null), }); - yield* syncDiscordThreadTitle(currentThread); + yield* syncDiscordThreadTitle(); }), ) .pipe( @@ -4826,11 +4993,22 @@ export const runBridge = ( } // --- Secondary: title / pin / side posts (must not starve tip edits) --- - // Best-effort + hard time budget: never fail the primary stream/finalize pass - // because GH PR lookup / title / tasks hung (outer 90s TimeoutError left Discord - // stuck on Working until the next chance event). + // Title first, with its own budget: turn settle must clear ⏳ even when GH + // lookup / tasks would burn the shared secondary timeout. Pending renames + // also retry on the Working heartbeat so idle threads do not stay stale. + yield* syncDiscordThreadTitle().pipe( + Effect.timeout("12 seconds"), + Effect.catchCause((cause) => + Effect.logWarning("Discord thread title sync failed or timed out", { + t3ThreadId: input.t3ThreadId, + cause: formatAlertCause(cause, 300), + }), + ), + Effect.asVoid, + ); + + // Best-effort + hard time budget for remaining secondary work. yield* Effect.gen(function* () { - yield* syncDiscordThreadTitle(thread); yield* syncThreadInfoModelPin(thread); // Tasks first: first-class Discord progress UI from turn.plan (not External User Input). @@ -5051,6 +5229,48 @@ export const runBridge = ( yield* deliveryLock.withPermit(runDeliveryWorker).pipe(Effect.forkDetach); }); + /** + * Retry pending / desynced Discord titles without waiting for another T3 snapshot. + * Rate-limits and secondary timeouts used to leave ⏳ after settle forever. + * Only wakes full sync when needed β€” never cold-starts GH every tick. + */ + const flushDiscordThreadTitleIfNeeded = Effect.gen(function* () { + const pendingTitle = yield* Ref.get(pendingDesiredThreadTitleRef); + const stateNow = yield* Ref.get(stateRef); + const threadNow = yield* Ref.get(latestThreadRef); + const mirroredActivity = parseDiscordThreadTitleBadges(stateNow.mirroredThreadTitle).activity; + const desiredActivity = + threadNow === null + ? null + : resolveDiscordThreadActivityBadgeState({ + sessionStatus: threadNow.session?.status ?? null, + activeTurnId: threadNow.session?.activeTurnId ?? null, + latestTurnState: threadNow.latestTurn?.state ?? null, + latestTurnCompletedAt: threadNow.latestTurn?.completedAt ?? null, + }); + const titleNeedsSync = + (pendingTitle !== null && pendingTitle !== stateNow.mirroredThreadTitle) || + desiredActivity !== mirroredActivity; + if (!titleNeedsSync) return; + yield* syncDiscordThreadTitle(); + }); + + const titleRetryFiber = yield* Effect.gen(function* () { + while (true) { + yield* Effect.sleep("15 seconds"); + yield* flushDiscordThreadTitleIfNeeded.pipe( + Effect.catchCause((cause) => + Effect.logWarning("Periodic Discord title retry failed", { + discordChannelId: input.discordChannelId, + t3ThreadId: input.t3ThreadId, + cause: formatAlertCause(cause, 200), + }), + ), + Effect.asVoid, + ); + } + }).pipe(Effect.forkChild); + const heartbeatFiber = input.presentationMode === "final-only" ? null @@ -5283,6 +5503,7 @@ export const runBridge = ( if (heartbeatFiber !== null) { yield* Fiber.interrupt(heartbeatFiber).pipe(Effect.ignore); } + yield* Fiber.interrupt(titleRetryFiber).pipe(Effect.ignore); yield* Fiber.interrupt(reconcileFiber).pipe(Effect.ignore); yield* stopVcsStatusSubscription; // Do NOT delete open stream tips on stop when the turn is still running. From bbaab612bfeaedab6986c70575e0a6f4c3edc439 Mon Sep 17 00:00:00 2001 From: "omegent-app[bot]" <306514130+omegent-app[bot]@users.noreply.github.com> Date: Sat, 25 Jul 2026 13:10:58 +0000 Subject: [PATCH 2/5] chore(fork-stack): add update to keep feature PRs mergeable Add `pnpm fork:stack update [--push] [pr]` to rebase or replay feature branches onto fork/changes, retarget wrong bases, and document the agent handoff so PRs are not opened against the upstream main mirror. --- AGENTS.md | 28 +++- docs/fork-stack.md | 40 +++++- scripts/fork-stack.test.ts | 59 +++++++- scripts/fork-stack.ts | 275 ++++++++++++++++++++++++++++++++++++- 4 files changed, 388 insertions(+), 14 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 43bac57a355..61ce1c4e7d9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -18,6 +18,12 @@ branches. - Start new work with `pnpm fork:stack start ` and open the PR against `fork/changes`. Ordinary feature/import PRs are not added to `.github/pr-stack.json`; they enter the runnable fork only after being reviewed and merged into `fork/changes`. +- **Never open implementation PRs against `main`.** `main` is the upstream mirror; GitHub will + report conflicts and a huge unrelated diff. Always base and retarget feature PRs on `fork/changes`. +- Before handoff (and whenever a PR is CONFLICTING / behind), run + `pnpm fork:stack update --push` (or `pnpm fork:stack update --push `). That rebases or + replays the feature commits onto latest `origin/fork/changes`, retargets a wrong PR base, and + force-with-lease pushes so the PR stays mergeable. - Independent features use parallel PRs based on `fork/changes`. Chain PRs only when one change genuinely depends on another, and merge that chain bottom-up. - Treat external forks and open upstream PRs as selective import sources. Tim Smart imports land as @@ -53,13 +59,21 @@ branches. When implementation work for a user request is done (code, docs, config β€” not pure Q&A): -1. **Commit** the changes on a feature branch. -2. **Open or update a PR** against the parent required by the private fork stack before handing off. - Use `main` only before cutover or when the work intentionally changes the upstream mirror. -3. **Before pushing follow-ups or saying β€œupdated the PR”**, verify PR state with `gh pr view` (or equivalent): - - If the PR is **open** β†’ push to that branch and update the PR. - - If the PR is **merged** or **closed** β†’ do **not** keep committing on that branch. `git fetch origin main`, create a **new branch from `origin/main`**, re-apply unmerged work, and open a **new PR**. -4. Never assume an earlier PR in the session is still open. +1. **Commit** the changes on a feature branch created with `pnpm fork:stack start ` (from + `fork/changes`). +2. **Open or update a PR against `fork/changes`** before handing off. Do not target `main` unless + the change is intentionally an upstream-mirror / promote projection. +3. **Keep the PR mergeable** before saying β€œupdated the PR” or finishing: + - `pnpm fork:stack update --push` (current branch) or `pnpm fork:stack update --push ` + - Confirm with `gh pr view --json baseRefName,mergeable,mergeStateStatus,url` + - `baseRefName` must be `fork/changes` and `mergeable` should be `MERGEABLE` (CI may still be + `UNSTABLE` while checks run). +4. **Before pushing follow-ups**, verify PR state with `gh pr view` (or equivalent): + - If the PR is **open** β†’ update that branch (prefer `fork:stack update --push`) and push. + - If the PR is **merged** or **closed** β†’ do **not** keep committing on that branch. + `pnpm fork:stack start `, re-apply unmerged work, and open a **new PR** against + `fork/changes`. +5. Never assume an earlier PR in the session is still open. ## Discord-originated pull requests diff --git a/docs/fork-stack.md b/docs/fork-stack.md index 6bca172444f..ad201af750b 100644 --- a/docs/fork-stack.md +++ b/docs/fork-stack.md @@ -53,10 +53,39 @@ The helper starts an independent branch from `fork/changes`: pnpm fork:stack start feature/my-change ``` -Commit and push normally, then open the PR against `fork/changes`. Updating that branch updates the -same PR and reruns PR CI. Ordinary feature and import PRs are deliberately not registered in the -stack manifest, so multiple independent PRs may be open concurrently without editing central -metadata. +Commit and push normally, then open the PR against `fork/changes` (never against `main`). Updating +that branch updates the same PR and reruns PR CI. Ordinary feature and import PRs are deliberately +not registered in the stack manifest, so multiple independent PRs may be open concurrently without +editing central metadata. + +### Keeping feature PRs up to date + +Feature branches drift when `fork/changes` moves (upstream mirror sync or merged siblings). Agents +must leave PRs mergeable at handoff: + +```sh +# Current branch + its open PR +pnpm fork:stack update --push + +# Explicit PR (checks out the head branch, updates, pushes) +pnpm fork:stack update --push 48 + +# Plan only (no push) +pnpm fork:stack update +``` + +`update` will: + +1. fetch latest `origin/fork/changes`; +2. **rebase** when the branch already descends from that tip but is behind; +3. **replay** only the PR’s own commits when the branch was cut from the wrong parent (e.g. stale + local `main` / upstream mirror) so the PR does not carry hundreds of unrelated commits; +4. **retarget** the PR base to `fork/changes` if it still points at `main` or another wrong branch; +5. **force-with-lease push** when `--push` is set; +6. print `gh pr view` mergeability JSON. + +Do not use GitHub β€œUpdate branch” merge commits for these feature PRs; prefer this rebase/replay +path so history stays linear and reviewable. After review, merge the PR into `fork/changes`. That push automatically runs the stack synchronizer: @@ -101,7 +130,8 @@ model is active. ### Multiple features Independent changes use parallel branches and PRs, all based on `fork/changes`. They can be reviewed -and merged in any order; rebase a remaining branch if an earlier merge overlaps it. +and merged in any order; run `pnpm fork:stack update --push` on a remaining branch if an earlier +merge overlaps it or the PR becomes CONFLICTING. Related changes may use one cohesive PR. If separate review is valuable, chain only those PRs by basing the dependent PR on the preceding feature branch. Merge the chain from bottom to top into diff --git a/scripts/fork-stack.test.ts b/scripts/fork-stack.test.ts index bf5ee6cfd88..1f8155e82d0 100644 --- a/scripts/fork-stack.test.ts +++ b/scripts/fork-stack.test.ts @@ -1,7 +1,14 @@ import { describe, expect, it } from "vite-plus/test"; import { parseManifest, StackError, type StackManifest } from "./rebase-pr-stack.ts"; -import { registerPullRequest, stackParentBranch, unregisterTopPullRequest } from "./fork-stack.ts"; +import { + featurePullRequestBaseBranch, + planFeatureBranchUpdate, + registerPullRequest, + shouldRetargetPullRequestBase, + stackParentBranch, + unregisterTopPullRequest, +} from "./fork-stack.ts"; const manifest: StackManifest = { upstreamRemote: "upstream", @@ -17,6 +24,56 @@ describe("fork stack helpers", () => { expect(stackParentBranch(manifest)).toBe("fork/changes"); }); + it("targets ordinary feature PRs at fork/changes", () => { + expect(featurePullRequestBaseBranch(manifest)).toBe("fork/changes"); + expect(shouldRetargetPullRequestBase("main", "fork/changes")).toBe(true); + expect(shouldRetargetPullRequestBase("fork/changes", "fork/changes")).toBe(false); + }); + + it("plans a simple rebase when behind an ancestor base", () => { + expect( + planFeatureBranchUpdate({ + baseIsAncestorOfHead: true, + behindCount: 3, + aheadCount: 1, + pullRequestCommitOids: ["abc"], + }), + ).toEqual({ action: "rebase", replayOids: [] }); + }); + + it("is a noop when already up to date with the base tip", () => { + expect( + planFeatureBranchUpdate({ + baseIsAncestorOfHead: true, + behindCount: 0, + aheadCount: 2, + pullRequestCommitOids: ["abc", "def"], + }), + ).toEqual({ action: "noop", replayOids: [] }); + }); + + it("replays only PR commits when the branch was cut from the wrong parent", () => { + expect( + planFeatureBranchUpdate({ + baseIsAncestorOfHead: false, + behindCount: 50, + aheadCount: 600, + pullRequestCommitOids: ["only-feature-commit"], + }), + ).toEqual({ action: "replay", replayOids: ["only-feature-commit"] }); + }); + + it("rejects misbased branches with no PR commits to replay", () => { + expect(() => + planFeatureBranchUpdate({ + baseIsAncestorOfHead: false, + behindCount: 10, + aheadCount: 10, + pullRequestCommitOids: [], + }), + ).toThrow(/not based on fork\/changes/); + }); + it("registers the permanent fork changes PR first", () => { const next = registerPullRequest(manifest, { number: 201, diff --git a/scripts/fork-stack.ts b/scripts/fork-stack.ts index 86c2890ebe5..cfcacab6ac6 100755 --- a/scripts/fork-stack.ts +++ b/scripts/fork-stack.ts @@ -50,6 +50,55 @@ export function stackParentBranch(manifest: StackManifest): string { return manifest.pullRequests.at(-1)?.branch ?? manifest.forkChangesBranch; } +/** + * Ordinary feature/import PRs always target the private default branch, not the + * upstream mirror (`main`) and not intermediate stack provenance branches. + */ +export function featurePullRequestBaseBranch(manifest: StackManifest): string { + return manifest.forkChangesBranch; +} + +export function shouldRetargetPullRequestBase( + currentBase: string | null | undefined, + expectedBase: string, +): boolean { + if (currentBase === null || currentBase === undefined || currentBase.trim() === "") { + return false; + } + return currentBase !== expectedBase; +} + +/** + * Plan how to bring a feature PR branch up to date with `fork/changes`. + * + * - `rebase` when the base tip is already an ancestor (normal drift). + * - `replay` when the branch was cut from the wrong parent (e.g. upstream `main`) + * and only the PR's own commits should be kept. + * - `noop` when already current. + */ +export function planFeatureBranchUpdate(input: { + readonly baseIsAncestorOfHead: boolean; + readonly behindCount: number; + readonly aheadCount: number; + readonly pullRequestCommitOids: ReadonlyArray; +}): { + readonly action: "noop" | "rebase" | "replay"; + readonly replayOids: ReadonlyArray; +} { + if (input.baseIsAncestorOfHead) { + if (input.behindCount <= 0) { + return { action: "noop", replayOids: [] }; + } + return { action: "rebase", replayOids: [] }; + } + if (input.pullRequestCommitOids.length === 0) { + throw new StackError( + "Branch is not based on fork/changes and no PR commits are available to replay. Re-create the branch with `pnpm fork:stack start `.", + ); + } + return { action: "replay", replayOids: input.pullRequestCommitOids }; +} + export function registerPullRequest( manifest: StackManifest, pullRequest: PullRequestView, @@ -130,10 +179,210 @@ function ensureClean(sourceRoot: string): void { } } +function runAllowFailure( + executable: string, + args: ReadonlyArray, + cwd: string, +): NodeChildProcess.SpawnSyncReturns { + return NodeChildProcess.spawnSync(executable, [...args], { + cwd, + encoding: "utf8", + env: { ...process.env, GIT_TERMINAL_PROMPT: "0" }, + }); +} + +function currentBranchName(sourceRoot: string): string { + const name = run("git", ["branch", "--show-current"], sourceRoot); + if (name === "") { + throw new StackError("Detached HEAD: check out the feature branch before updating."); + } + return name; +} + +function resolveOpenPullRequestForBranch( + sourceRoot: string, + branch: string, +): { readonly number: number; readonly baseRefName: string; readonly headRefName: string } | null { + const listed = run( + "gh", + [ + "pr", + "list", + "--repo", + FORK_REPOSITORY, + "--head", + branch, + "--state", + "open", + "--json", + "number,baseRefName,headRefName", + "--limit", + "1", + ], + sourceRoot, + ); + const rows = JSON.parse(listed) as ReadonlyArray<{ + readonly number: number; + readonly baseRefName: string; + readonly headRefName: string; + }>; + return rows[0] ?? null; +} + +function pullRequestCommitOids(sourceRoot: string, number: number): ReadonlyArray { + const output = run( + "gh", + ["pr", "view", String(number), "--repo", FORK_REPOSITORY, "--json", "commits"], + sourceRoot, + ); + const value = JSON.parse(output) as { + readonly commits: ReadonlyArray<{ readonly oid: string }>; + }; + return value.commits.map((commit) => commit.oid); +} + +/** + * Rebase or replay the current feature branch onto latest `fork/changes`, retarget the + * open PR base if needed, and optionally force-with-lease push so the PR stays mergeable. + */ +function updateFeatureBranch( + sourceRoot: string, + manifest: StackManifest, + options: { + readonly pullRequestNumber?: number | undefined; + readonly push: boolean; + }, +): void { + ensureClean(sourceRoot); + const expectedBase = featurePullRequestBaseBranch(manifest); + run("git", ["fetch", "origin", expectedBase], sourceRoot); + + let branch = currentBranchName(sourceRoot); + let prNumber: number | null = options.pullRequestNumber ?? null; + let prBaseRefName: string | null = null; + + if (options.pullRequestNumber !== undefined) { + const pullRequest = readPullRequest(sourceRoot, options.pullRequestNumber); + if (pullRequest.state.toLowerCase() !== "open") { + throw new StackError( + `PR #${options.pullRequestNumber} is ${pullRequest.state}; only open feature PRs can be updated.`, + ); + } + branch = pullRequest.headRefName; + prNumber = pullRequest.number; + prBaseRefName = pullRequest.baseRefName; + run( + "git", + ["fetch", "origin", `+refs/heads/${branch}:refs/remotes/origin/${branch}`], + sourceRoot, + ); + run("git", ["switch", branch], sourceRoot); + // Prefer the remote tip when updating a named PR so local drift does not win. + const remoteTip = run("git", ["rev-parse", `origin/${branch}`], sourceRoot); + run("git", ["reset", "--hard", remoteTip], sourceRoot); + } else { + const open = resolveOpenPullRequestForBranch(sourceRoot, branch); + if (open !== null) { + prNumber = open.number; + prBaseRefName = open.baseRefName; + } + } + + const baseRef = `origin/${expectedBase}`; + const ancestorCheck = runAllowFailure( + "git", + ["merge-base", "--is-ancestor", baseRef, "HEAD"], + sourceRoot, + ); + const baseIsAncestorOfHead = ancestorCheck.status === 0; + const behindCount = Number(run("git", ["rev-list", "--count", `HEAD..${baseRef}`], sourceRoot)); + const aheadCount = Number(run("git", ["rev-list", "--count", `${baseRef}..HEAD`], sourceRoot)); + const prOids = prNumber === null ? [] : pullRequestCommitOids(sourceRoot, prNumber); + const plan = planFeatureBranchUpdate({ + baseIsAncestorOfHead, + behindCount, + aheadCount, + pullRequestCommitOids: prOids, + }); + + if (plan.action === "rebase") { + const result = runAllowFailure( + "git", + ["-c", "commit.gpgsign=false", "rebase", baseRef], + sourceRoot, + ); + if (result.status !== 0) { + runAllowFailure("git", ["rebase", "--abort"], sourceRoot); + throw new StackError( + `Rebase onto ${expectedBase} failed:\n${result.stderr.trim() || result.stdout.trim()}\nResolve conflicts, then re-run with a clean tree or finish manually.`, + ); + } + console.log(`Rebased ${branch} onto ${expectedBase}.`); + } else if (plan.action === "replay") { + const tipBefore = run("git", ["rev-parse", "HEAD"], sourceRoot); + run("git", ["reset", "--hard", baseRef], sourceRoot); + const cherry = runAllowFailure( + "git", + ["-c", "commit.gpgsign=false", "cherry-pick", ...plan.replayOids], + sourceRoot, + ); + if (cherry.status !== 0) { + runAllowFailure("git", ["cherry-pick", "--abort"], sourceRoot); + run("git", ["reset", "--hard", tipBefore], sourceRoot); + throw new StackError( + `Replay onto ${expectedBase} failed while cherry-picking PR commits:\n${cherry.stderr.trim() || cherry.stdout.trim()}`, + ); + } + console.log( + `Replayed ${plan.replayOids.length} PR commit(s) onto ${expectedBase} (was misbased).`, + ); + } else { + console.log(`${branch} is already up to date with ${expectedBase}.`); + } + + if (prNumber !== null && shouldRetargetPullRequestBase(prBaseRefName, expectedBase)) { + run( + "gh", + ["pr", "edit", String(prNumber), "--repo", FORK_REPOSITORY, "--base", expectedBase], + sourceRoot, + ); + console.log(`Retargeted PR #${prNumber} base ${prBaseRefName} β†’ ${expectedBase}.`); + } + + if (options.push) { + run( + "git", + ["push", "--force-with-lease", "-u", "origin", `HEAD:refs/heads/${branch}`], + sourceRoot, + ); + console.log(`Pushed ${branch} with --force-with-lease.`); + } else { + console.log("Dry run complete (no push). Re-run with --push to update the remote PR branch."); + } + + if (prNumber !== null) { + const status = run( + "gh", + [ + "pr", + "view", + String(prNumber), + "--repo", + FORK_REPOSITORY, + "--json", + "url,baseRefName,mergeable,mergeStateStatus", + ], + sourceRoot, + ); + console.log(status); + } +} + function usage(): string { return `Usage: node scripts/fork-stack.ts start node scripts/fork-stack.ts start-upstream + node scripts/fork-stack.ts update [--push] [pr-number] node scripts/fork-stack.ts promote node scripts/fork-stack.ts adopt node scripts/fork-stack.ts demote @@ -151,13 +400,37 @@ async function main(args: ReadonlyArray): Promise { if (command === "start" && value && extra.length === 0) { ensureClean(sourceRoot); - const parent = stackParentBranch(manifest); + const parent = featurePullRequestBaseBranch(manifest); run("git", ["fetch", "origin", parent], sourceRoot); run("git", ["switch", "-c", value, `origin/${parent}`], sourceRoot); console.log(`Created ${value} from ${parent}. Open its PR against ${parent}.`); return; } + if (command === "update") { + const tokens = [value, ...extra].filter((token): token is string => token !== undefined); + let push = false; + let pullRequestNumber: number | undefined; + for (const token of tokens) { + if (token === "--push") { + push = true; + continue; + } + if (token === "--dry-run") { + push = false; + continue; + } + const number = Number(token); + if (Number.isSafeInteger(number) && number > 0 && pullRequestNumber === undefined) { + pullRequestNumber = number; + continue; + } + throw new StackError(usage()); + } + updateFeatureBranch(sourceRoot, manifest, { pullRequestNumber, push }); + return; + } + if (command === "start-upstream" && value && extra.length === 0) { ensureClean(sourceRoot); run( From f56e37c1df90c1e9ed7a5da0312d3a43b36fe91a Mon Sep 17 00:00:00 2001 From: "omegent-app[bot]" <306514130+omegent-app[bot]@users.noreply.github.com> Date: Sat, 25 Jul 2026 13:11:43 +0000 Subject: [PATCH 3/5] fix(fork-stack): parse gh JSON under FORCE_COLOR agent hosts Disable ANSI color in stack subprocess env so `gh --json` stays valid JSON when agents run with FORCE_COLOR set. --- scripts/fork-stack.ts | 12 ++++++++++-- scripts/rebase-pr-stack.ts | 16 +++++++++++----- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/scripts/fork-stack.ts b/scripts/fork-stack.ts index cfcacab6ac6..e86b288b087 100755 --- a/scripts/fork-stack.ts +++ b/scripts/fork-stack.ts @@ -31,11 +31,19 @@ interface PullRequestCommitsView { readonly commits: ReadonlyArray<{ readonly oid: string }>; } +/** Subprocess env: force plain stdout so `gh --json` is parseable under FORCE_COLOR hosts. */ +function subprocessEnv(): NodeJS.ProcessEnv { + const env = { ...process.env, GIT_TERMINAL_PROMPT: "0", NO_COLOR: "1", CLICOLOR: "0" }; + delete env.FORCE_COLOR; + delete env.CLICOLOR_FORCE; + return env; +} + function run(executable: string, args: ReadonlyArray, cwd: string): string { const result = NodeChildProcess.spawnSync(executable, [...args], { cwd, encoding: "utf8", - env: { ...process.env, GIT_TERMINAL_PROMPT: "0" }, + env: subprocessEnv(), }); if (result.error) throw new StackError(`Unable to run ${executable}: ${result.error.message}`); if (result.status !== 0) { @@ -187,7 +195,7 @@ function runAllowFailure( return NodeChildProcess.spawnSync(executable, [...args], { cwd, encoding: "utf8", - env: { ...process.env, GIT_TERMINAL_PROMPT: "0" }, + env: subprocessEnv(), }); } diff --git a/scripts/rebase-pr-stack.ts b/scripts/rebase-pr-stack.ts index 3f0b672c8cd..4788bf9a585 100644 --- a/scripts/rebase-pr-stack.ts +++ b/scripts/rebase-pr-stack.ts @@ -156,14 +156,20 @@ function run( readonly stateDir?: string; }, ): NodeChildProcess.SpawnSyncReturns { + const baseEnv: NodeJS.ProcessEnv = { + ...process.env, + GIT_TERMINAL_PROMPT: "0", + // Agent hosts often set FORCE_COLOR; that breaks `gh --json` parseability. + NO_COLOR: "1", + CLICOLOR: "0", + ...options.env, + }; + delete baseEnv.FORCE_COLOR; + delete baseEnv.CLICOLOR_FORCE; const result = NodeChildProcess.spawnSync(executable, [...args], { cwd: options.cwd, encoding: "utf8", - env: { - ...process.env, - GIT_TERMINAL_PROMPT: "0", - ...options.env, - }, + env: baseEnv, }); if (result.error) { throw new StackError(`Unable to run ${executable}: ${result.error.message}`, { From c245b3320520267e5daa9e2fd82b8a6e5f15e74d Mon Sep 17 00:00:00 2001 From: "omegent-app[bot]" <306514130+omegent-app[bot]@users.noreply.github.com> Date: Sat, 25 Jul 2026 13:52:59 +0000 Subject: [PATCH 4/5] fix(fork-stack): force plain gh JSON for agent FORCE_COLOR hosts Pass --color=never and strip residual ANSI so stack helpers can parse `gh --json` when FORCE_COLOR is set by the agent environment. --- scripts/fork-stack.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/scripts/fork-stack.ts b/scripts/fork-stack.ts index e86b288b087..dcff290c366 100755 --- a/scripts/fork-stack.ts +++ b/scripts/fork-stack.ts @@ -40,7 +40,12 @@ function subprocessEnv(): NodeJS.ProcessEnv { } function run(executable: string, args: ReadonlyArray, cwd: string): string { - const result = NodeChildProcess.spawnSync(executable, [...args], { + // gh may still colorize under some agent hosts; force plain output for parseable --json. + const finalArgs = + executable === "gh" && !args.includes("--color=never") && !args.includes("--color") + ? ["--color=never", ...args] + : args; + const result = NodeChildProcess.spawnSync(executable, finalArgs, { cwd, encoding: "utf8", env: subprocessEnv(), @@ -51,7 +56,8 @@ function run(executable: string, args: ReadonlyArray, cwd: string): stri `${executable} ${args.join(" ")} failed: ${result.stderr.trim() || result.stdout.trim()}`, ); } - return result.stdout.trim(); + // Strip any residual ANSI color in case a wrapper still injects it. + return result.stdout.replace(/\u001b\[[0-9;]*m/g, "").trim(); } export function stackParentBranch(manifest: StackManifest): string { From 008f8e7d6d1f921acbd0ed6a473716fcfa17a5c5 Mon Sep 17 00:00:00 2001 From: "omegent-app[bot]" <306514130+omegent-app[bot]@users.noreply.github.com> Date: Sat, 25 Jul 2026 13:55:00 +0000 Subject: [PATCH 5/5] fix(fork-stack): strip ANSI from gh JSON under agent FORCE_COLOR Avoid --color flags the t3 gh wrapper rejects; force FORCE_COLOR=0 and strip residual SGR sequences so stack update/rebase can parse --json. --- scripts/fork-stack.ts | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/scripts/fork-stack.ts b/scripts/fork-stack.ts index dcff290c366..a5495621a5e 100755 --- a/scripts/fork-stack.ts +++ b/scripts/fork-stack.ts @@ -31,21 +31,26 @@ interface PullRequestCommitsView { readonly commits: ReadonlyArray<{ readonly oid: string }>; } +/** Strip ANSI color / SGR sequences (agent hosts often set FORCE_COLOR). */ +function stripAnsi(text: string): string { + return text.replace(/\u001b\[[0-9;?]*[a-zA-Z]/g, ""); +} + /** Subprocess env: force plain stdout so `gh --json` is parseable under FORCE_COLOR hosts. */ function subprocessEnv(): NodeJS.ProcessEnv { - const env = { ...process.env, GIT_TERMINAL_PROMPT: "0", NO_COLOR: "1", CLICOLOR: "0" }; - delete env.FORCE_COLOR; - delete env.CLICOLOR_FORCE; - return env; + return { + ...process.env, + GIT_TERMINAL_PROMPT: "0", + NO_COLOR: "1", + CLICOLOR: "0", + FORCE_COLOR: "0", + CLICOLOR_FORCE: "0", + }; } function run(executable: string, args: ReadonlyArray, cwd: string): string { - // gh may still colorize under some agent hosts; force plain output for parseable --json. - const finalArgs = - executable === "gh" && !args.includes("--color=never") && !args.includes("--color") - ? ["--color=never", ...args] - : args; - const result = NodeChildProcess.spawnSync(executable, finalArgs, { + // Do not pass `gh --color=never`: the t3-github-app gh wrapper rejects that flag. + const result = NodeChildProcess.spawnSync(executable, [...args], { cwd, encoding: "utf8", env: subprocessEnv(), @@ -53,11 +58,10 @@ function run(executable: string, args: ReadonlyArray, cwd: string): stri if (result.error) throw new StackError(`Unable to run ${executable}: ${result.error.message}`); if (result.status !== 0) { throw new StackError( - `${executable} ${args.join(" ")} failed: ${result.stderr.trim() || result.stdout.trim()}`, + `${executable} ${args.join(" ")} failed: ${stripAnsi(result.stderr.trim() || result.stdout.trim())}`, ); } - // Strip any residual ANSI color in case a wrapper still injects it. - return result.stdout.replace(/\u001b\[[0-9;]*m/g, "").trim(); + return stripAnsi(result.stdout).trim(); } export function stackParentBranch(manifest: StackManifest): string {