From c5440162b87b210b498857985765ba42279603a8 Mon Sep 17 00:00:00 2001 From: haoqixu Date: Wed, 24 Jun 2026 20:12:56 +0800 Subject: [PATCH] fix(coding-agent): show resources before messages when resuming session --- .../src/modes/interactive/interactive-mode.ts | 44 ++++++---- .../test/interactive-mode-status.test.ts | 40 +++++---- .../5943-session-start-notify.test.ts | 83 +++++++++++++++++++ 3 files changed, 134 insertions(+), 33 deletions(-) diff --git a/packages/coding-agent/src/modes/interactive/interactive-mode.ts b/packages/coding-agent/src/modes/interactive/interactive-mode.ts index fe733f20..7b6dbbe8 100644 --- a/packages/coding-agent/src/modes/interactive/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive/interactive-mode.ts @@ -265,6 +265,7 @@ export interface InteractiveModeOptions { export class InteractiveMode { private runtimeHost: AgentSessionRuntime; private ui: TUI; + private loadedResourcesContainer: Container; private chatContainer: Container; private pendingMessagesContainer: Container; private statusContainer: Container; @@ -401,6 +402,7 @@ export class InteractiveMode { this.ui = new TUI(new ProcessTerminal(), this.settingsManager.getShowHardwareCursor()); this.ui.setClearOnShrink(this.settingsManager.getClearOnShrink()); this.headerContainer = new Container(); + this.loadedResourcesContainer = new Container(); this.chatContainer = new Container(); this.pendingMessagesContainer = new Container(); this.statusContainer = new Container(); @@ -636,8 +638,10 @@ export class InteractiveMode { console.log(theme.fg("dim", `Model scope: ${modelList}${cycleHint}`)); } - // Add header container as first child. Populate it after detectThemeIfUnset. + // Add header container as first child. Populate it after applying theme settings. + // Keep loaded resources before chat so restored session messages never precede them. this.ui.addChild(this.headerContainer); + this.ui.addChild(this.loadedResourcesContainer); this.ui.addChild(this.chatContainer); this.ui.addChild(this.pendingMessagesContainer); @@ -1330,6 +1334,9 @@ export class InteractiveMode { force?: boolean; showDiagnosticsWhenQuiet?: boolean; }): void { + // Resource rendering is idempotent; chat clears no longer clear this separate container. + this.loadedResourcesContainer.clear(); + const showListing = options?.force || this.options.verbose || !this.settingsManager.getQuietStartup(); const showDiagnostics = showListing || options?.showDiagnosticsWhenQuiet === true; if (!showListing && !showDiagnostics) { @@ -1357,8 +1364,8 @@ export class InteractiveMode { 0, 0, ); - this.chatContainer.addChild(section); - this.chatContainer.addChild(new Spacer(1)); + this.loadedResourcesContainer.addChild(section); + this.loadedResourcesContainer.addChild(new Spacer(1)); }; const skillsResult = this.session.resourceLoader.getSkills(); @@ -1395,7 +1402,7 @@ export class InteractiveMode { if (showListing) { const contextFiles = this.session.resourceLoader.getAgentsFiles().agentsFiles; if (contextFiles.length > 0) { - this.chatContainer.addChild(new Spacer(1)); + this.loadedResourcesContainer.addChild(new Spacer(1)); const contextList = contextFiles .map((f) => theme.fg("dim", ` ${this.formatDisplayPath(f.path)}`)) .join("\n"); @@ -1478,17 +1485,19 @@ export class InteractiveMode { const skillDiagnostics = skillsResult.diagnostics; if (skillDiagnostics.length > 0) { const warningLines = this.formatDiagnostics(skillDiagnostics, sourceInfos); - this.chatContainer.addChild(new Text(`${theme.fg("warning", "[Skill conflicts]")}\n${warningLines}`, 0, 0)); - this.chatContainer.addChild(new Spacer(1)); + this.loadedResourcesContainer.addChild( + new Text(`${theme.fg("warning", "[Skill conflicts]")}\n${warningLines}`, 0, 0), + ); + this.loadedResourcesContainer.addChild(new Spacer(1)); } const promptDiagnostics = promptsResult.diagnostics; if (promptDiagnostics.length > 0) { const warningLines = this.formatDiagnostics(promptDiagnostics, sourceInfos); - this.chatContainer.addChild( + this.loadedResourcesContainer.addChild( new Text(`${theme.fg("warning", "[Prompt conflicts]")}\n${warningLines}`, 0, 0), ); - this.chatContainer.addChild(new Spacer(1)); + this.loadedResourcesContainer.addChild(new Spacer(1)); } const extensionDiagnostics: ResourceDiagnostic[] = []; @@ -1508,17 +1517,19 @@ export class InteractiveMode { if (extensionDiagnostics.length > 0) { const warningLines = this.formatDiagnostics(extensionDiagnostics, sourceInfos); - this.chatContainer.addChild( + this.loadedResourcesContainer.addChild( new Text(`${theme.fg("warning", "[Extension issues]")}\n${warningLines}`, 0, 0), ); - this.chatContainer.addChild(new Spacer(1)); + this.loadedResourcesContainer.addChild(new Spacer(1)); } const themeDiagnostics = themesResult.diagnostics; if (themeDiagnostics.length > 0) { const warningLines = this.formatDiagnostics(themeDiagnostics, sourceInfos); - this.chatContainer.addChild(new Text(`${theme.fg("warning", "[Theme conflicts]")}\n${warningLines}`, 0, 0)); - this.chatContainer.addChild(new Spacer(1)); + this.loadedResourcesContainer.addChild( + new Text(`${theme.fg("warning", "[Theme conflicts]")}\n${warningLines}`, 0, 0), + ); + this.loadedResourcesContainer.addChild(new Spacer(1)); } } } @@ -1651,6 +1662,7 @@ export class InteractiveMode { } private renderCurrentSessionState(): void { + this.loadedResourcesContainer.clear(); this.chatContainer.clear(); this.pendingMessagesContainer.clear(); this.compactionQueuedMessages = []; @@ -3606,9 +3618,11 @@ export class InteractiveMode { if (isExpandable(activeHeader)) { activeHeader.setExpanded(expanded); } - for (const child of this.chatContainer.children) { - if (isExpandable(child)) { - child.setExpanded(expanded); + for (const container of [this.loadedResourcesContainer, this.chatContainer]) { + for (const child of container.children) { + if (isExpandable(child)) { + child.setExpanded(expanded); + } } } this.ui.requestRender(); diff --git a/packages/coding-agent/test/interactive-mode-status.test.ts b/packages/coding-agent/test/interactive-mode-status.test.ts index 6ba7db91..7d75d383 100644 --- a/packages/coding-agent/test/interactive-mode-status.test.ts +++ b/packages/coding-agent/test/interactive-mode-status.test.ts @@ -119,11 +119,13 @@ describe("InteractiveMode.showStatus", () => { describe("InteractiveMode.setToolsExpanded", () => { test("applies expansion state to the active header and chat entries", () => { const header = { setExpanded: vi.fn() }; + const loadedResourcesChild = { setExpanded: vi.fn() }; const chatChild = { setExpanded: vi.fn() }; const fakeThis: any = { toolOutputExpanded: false, customHeader: undefined, builtInHeader: header, + loadedResourcesContainer: { children: [loadedResourcesChild] }, chatContainer: { children: [chatChild] }, ui: { requestRender: vi.fn() }, }; @@ -132,6 +134,7 @@ describe("InteractiveMode.setToolsExpanded", () => { expect(fakeThis.toolOutputExpanded).toBe(true); expect(header.setExpanded).toHaveBeenCalledWith(true); + expect(loadedResourcesChild.setExpanded).toHaveBeenCalledWith(true); expect(chatChild.setExpanded).toHaveBeenCalledWith(true); expect(fakeThis.ui.requestRender).toHaveBeenCalledTimes(1); }); @@ -441,6 +444,7 @@ describe("InteractiveMode.showLoadedResources", () => { const fakeThis: any = { options: { verbose: options.verbose ?? false }, toolOutputExpanded: options.toolOutputExpanded ?? false, + loadedResourcesContainer: new Container(), chatContainer: new Container(), settingsManager: { getQuietStartup: () => options.quietStartup, @@ -619,7 +623,7 @@ describe("InteractiveMode.showLoadedResources", () => { force: false, }); - const output = renderAll(fakeThis.chatContainer); + const output = renderAll(fakeThis.loadedResourcesContainer); expect(output).toContain("[Skills]"); expect(output).toContain("commit"); expect(output).not.toContain("resource-list"); @@ -636,7 +640,7 @@ describe("InteractiveMode.showLoadedResources", () => { force: false, }); - const output = renderAll(fakeThis.chatContainer); + const output = renderAll(fakeThis.loadedResourcesContainer); expect(output).toContain("[Skills]"); expect(output).toContain("resource-list"); expect(output).not.toContain("commit"); @@ -654,7 +658,7 @@ describe("InteractiveMode.showLoadedResources", () => { force: false, }); - const output = renderAll(fakeThis.chatContainer); + const output = renderAll(fakeThis.loadedResourcesContainer); expect(output).toContain("[Skills]"); expect(output).toContain("resource-list"); expect(output).not.toContain("commit"); @@ -670,7 +674,7 @@ describe("InteractiveMode.showLoadedResources", () => { force: false, }); - const output = renderAll(fakeThis.chatContainer); + const output = renderAll(fakeThis.loadedResourcesContainer); expect(output).toContain("[Extensions]"); expect(output).toContain("answer.ts, btw.ts"); expect(output).not.toContain("extensions/answer.ts"); @@ -687,7 +691,7 @@ describe("InteractiveMode.showLoadedResources", () => { force: false, }); - expect(normalizeRenderedOutput(fakeThis.chatContainer)).toMatchInlineSnapshot(` + expect(normalizeRenderedOutput(fakeThis.loadedResourcesContainer)).toMatchInlineSnapshot(` "[Extensions] @scope/pi-scoped, answer.ts, cli-extension.ts, HazAT/pi-interactive-subagents, HazAT/pi-interactive-subagents:subagents, local-index, pi-markdown-preview, user-index"`); }); @@ -733,7 +737,7 @@ describe("InteractiveMode.showLoadedResources", () => { force: false, }); - expect(normalizeRenderedOutput(fakeThis.chatContainer)).toMatchInlineSnapshot(` + expect(normalizeRenderedOutput(fakeThis.loadedResourcesContainer)).toMatchInlineSnapshot(` "[Extensions] alpha/one, beta/one, gamma/one"`); }); @@ -761,7 +765,7 @@ describe("InteractiveMode.showLoadedResources", () => { force: false, }); - expect(normalizeRenderedOutput(fakeThis.chatContainer)).toMatchInlineSnapshot(` + expect(normalizeRenderedOutput(fakeThis.loadedResourcesContainer)).toMatchInlineSnapshot(` "[Extensions] plan-mode"`); }); @@ -789,7 +793,7 @@ describe("InteractiveMode.showLoadedResources", () => { force: false, }); - expect(normalizeRenderedOutput(fakeThis.chatContainer)).toMatchInlineSnapshot(` + expect(normalizeRenderedOutput(fakeThis.loadedResourcesContainer)).toMatchInlineSnapshot(` "[Extensions] plan-mode"`); }); @@ -826,7 +830,7 @@ describe("InteractiveMode.showLoadedResources", () => { force: false, }); - expect(normalizeRenderedOutput(fakeThis.chatContainer)).toMatchInlineSnapshot(` + expect(normalizeRenderedOutput(fakeThis.loadedResourcesContainer)).toMatchInlineSnapshot(` "[Extensions] plan-mode, webfetch.ts"`); }); @@ -863,7 +867,7 @@ describe("InteractiveMode.showLoadedResources", () => { force: false, }); - expect(normalizeRenderedOutput(fakeThis.chatContainer)).toMatchInlineSnapshot(` + expect(normalizeRenderedOutput(fakeThis.loadedResourcesContainer)).toMatchInlineSnapshot(` "[Extensions] bar, foo"`); }); @@ -900,7 +904,7 @@ describe("InteractiveMode.showLoadedResources", () => { force: false, }); - expect(normalizeRenderedOutput(fakeThis.chatContainer)).toMatchInlineSnapshot(` + expect(normalizeRenderedOutput(fakeThis.loadedResourcesContainer)).toMatchInlineSnapshot(` "[Extensions] alpha/tools, beta/tools"`); }); @@ -928,7 +932,7 @@ describe("InteractiveMode.showLoadedResources", () => { force: false, }); - expect(normalizeRenderedOutput(fakeThis.chatContainer)).toMatchInlineSnapshot(` + expect(normalizeRenderedOutput(fakeThis.loadedResourcesContainer)).toMatchInlineSnapshot(` "[Extensions] main.ts"`); }); @@ -956,7 +960,7 @@ describe("InteractiveMode.showLoadedResources", () => { force: false, }); - expect(normalizeRenderedOutput(fakeThis.chatContainer)).toMatchInlineSnapshot(` + expect(normalizeRenderedOutput(fakeThis.loadedResourcesContainer)).toMatchInlineSnapshot(` "[Extensions] pi-markdown-preview"`); }); @@ -972,7 +976,7 @@ describe("InteractiveMode.showLoadedResources", () => { force: false, }); - expect(normalizeRenderedOutput(fakeThis.chatContainer)).toMatchInlineSnapshot(` + expect(normalizeRenderedOutput(fakeThis.loadedResourcesContainer)).toMatchInlineSnapshot(` "[Extensions] project /tmp/project/.pi/extensions/answer.ts @@ -1003,7 +1007,7 @@ describe("InteractiveMode.showLoadedResources", () => { force: false, }); - const output = renderAll(fakeThis.chatContainer).replace(/\\/g, "/"); + const output = renderAll(fakeThis.loadedResourcesContainer).replace(/\\/g, "/"); expect(output).toContain("[Context]"); expect(output).toContain("~/.pi/agent/AGENTS.md, AGENTS.md"); expect(output).not.toContain(`${cwd.replace(/\\/g, "/")}/AGENTS.md`); @@ -1023,7 +1027,7 @@ describe("InteractiveMode.showLoadedResources", () => { force: false, }); - const output = renderAll(fakeThis.chatContainer).replace(/\\/g, "/"); + const output = renderAll(fakeThis.loadedResourcesContainer).replace(/\\/g, "/"); expect(output).toContain("[Context]"); expect(output).toContain("~/.pi/agent/AGENTS.md"); expect(output).toContain("~/Development/pi-mono/AGENTS.md"); @@ -1042,7 +1046,7 @@ describe("InteractiveMode.showLoadedResources", () => { showDiagnosticsWhenQuiet: true, }); - expect(fakeThis.chatContainer.children).toHaveLength(0); + expect(fakeThis.loadedResourcesContainer.children).toHaveLength(0); }); test("still shows diagnostics on quiet startup when requested", () => { @@ -1057,7 +1061,7 @@ describe("InteractiveMode.showLoadedResources", () => { showDiagnosticsWhenQuiet: true, }); - const output = renderAll(fakeThis.chatContainer); + const output = renderAll(fakeThis.loadedResourcesContainer); expect(output).toContain("[Skill conflicts]"); expect(output).not.toContain("[Skills]"); }); diff --git a/packages/coding-agent/test/suite/regressions/5943-session-start-notify.test.ts b/packages/coding-agent/test/suite/regressions/5943-session-start-notify.test.ts index db7ce26d..2c8d88e0 100644 --- a/packages/coding-agent/test/suite/regressions/5943-session-start-notify.test.ts +++ b/packages/coding-agent/test/suite/regressions/5943-session-start-notify.test.ts @@ -1,4 +1,5 @@ import { fauxAssistantMessage } from "@earendil-works/pi-ai"; +import { Container, Text } from "@earendil-works/pi-tui"; import { describe, expect, it, vi } from "vitest"; import type { AgentSessionEvent } from "../../../src/core/agent-session.ts"; import type { ExtensionUIContext } from "../../../src/core/extensions/index.ts"; @@ -43,6 +44,35 @@ function createUiContext( }; } +type LoadedResourcesResult = { [K in keyof T]: T[K] } & { diagnostics: [] }; + +type LoadedResourcesContext = { + loadedResourcesContainer: Container; + chatContainer: Container; + options: { verbose?: boolean }; + settingsManager: { getQuietStartup: () => boolean }; + sessionManager: { getCwd: () => string }; + session: { + promptTemplates: []; + resourceLoader: { + getAgentsFiles: () => LoadedResourcesResult<{ agentsFiles: Array<{ path: string }> }>; + getSkills: () => LoadedResourcesResult<{ skills: [] }>; + getPrompts: () => LoadedResourcesResult<{ prompts: [] }>; + getThemes: () => LoadedResourcesResult<{ themes: [] }>; + getExtensions: () => { extensions: []; errors: [] }; + }; + extensionRunner: { + getCommandDiagnostics: () => []; + getShortcutDiagnostics: () => []; + getRegisteredCommands: () => []; + }; + }; + getStartupExpansionState: () => boolean; + formatDisplayPath: (resourcePath: string) => string; + formatContextPath: (resourcePath: string) => string; + getBuiltInCommandConflictDiagnostics: (extensionRunner: LoadedResourcesContext["session"]["extensionRunner"]) => []; +}; + type RebindContext = { unsubscribe?: () => void; applyRuntimeSettings: () => void; @@ -97,6 +127,10 @@ type ReloadCommandContext = { }; type InteractiveModePrototype = { + showLoadedResources( + this: LoadedResourcesContext, + options?: { extensions?: Array<{ path: string }>; force?: boolean; showDiagnosticsWhenQuiet?: boolean }, + ): void; rebindCurrentSession(this: RebindContext, options?: { renderBeforeBind?: boolean }): Promise; handleReloadCommand(this: ReloadCommandContext): Promise; }; @@ -183,7 +217,56 @@ function getMessageText(event: MessageEvent): string { .join(""); } +function createLoadedResourcesContext(): LoadedResourcesContext { + return { + loadedResourcesContainer: new Container(), + chatContainer: new Container(), + options: { verbose: true }, + settingsManager: { getQuietStartup: () => false }, + sessionManager: { getCwd: () => "/repo" }, + session: { + promptTemplates: [], + resourceLoader: { + getAgentsFiles: () => ({ agentsFiles: [{ path: "/repo/AGENTS.md" }], diagnostics: [] }), + getSkills: () => ({ skills: [], diagnostics: [] }), + getPrompts: () => ({ prompts: [], diagnostics: [] }), + getThemes: () => ({ themes: [], diagnostics: [] }), + getExtensions: () => ({ extensions: [], errors: [] }), + }, + extensionRunner: { + getCommandDiagnostics: () => [], + getShortcutDiagnostics: () => [], + getRegisteredCommands: () => [], + }, + }, + getStartupExpansionState: () => false, + formatDisplayPath: (resourcePath) => resourcePath, + formatContextPath: (resourcePath) => resourcePath.replace("/repo/", ""), + getBuiltInCommandConflictDiagnostics: () => [], + }; +} + describe("regression #5943: session_start transient UI", () => { + it("renders loaded resources before restored messages without stale entries", () => { + initTheme("dark", false); + const context = createLoadedResourcesContext(); + const root = new Container(); + root.addChild(context.loadedResourcesContainer); + root.addChild(context.chatContainer); + context.loadedResourcesContainer.addChild(new Text("stale resources", 0, 0)); + context.chatContainer.addChild(new Text("restored message", 0, 0)); + + interactiveModePrototype.showLoadedResources.call(context); + + const chatRendered = context.chatContainer.render(80).join("\n"); + expect(chatRendered).toContain("restored message"); + expect(chatRendered).not.toContain("[Context]"); + + const rendered = root.render(80).join("\n"); + expect(rendered).not.toContain("stale resources"); + expect(rendered.indexOf("[Context]")).toBeLessThan(rendered.indexOf("restored message")); + }); + it("renders replacement session state before session_start handlers can notify", async () => { const events: string[] = []; const harness = await createHarness({