diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 3abc94925..66e89b882 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -49,7 +49,6 @@ - Fixed the `write` approval gate misclassifying `xd://` device writes as `exec` when the mounted tool declared a function-valued (argument-dependent) `approval`: the gate discarded the function and never decoded the device JSON payload, so read/write device operations prompted in non-yolo modes their approval mode permits. It now parses valid object payloads and evaluates the mounted tool's normal approval decision, while malformed JSON, non-object payloads, and unknown devices still fall back to `exec` and prompt ([#5727](https://github.com/can1357/oh-my-pi/issues/5727)). - Fixed custom LSP servers such as `roslyn-language-server` crashing after initialization when they request unconfigured `workspace/configuration` sections; missing settings now receive the spec-required `null` instead of `{}` ([#5745](https://github.com/can1357/oh-my-pi/issues/5745)). - Fixed late user-initiated bash results and minimized-output artifacts being recorded in whichever session or branch was active when execution finished; bash now retains its originating transcript across `new_session`/`switch_session`/`branch`/tree navigation, and an intentionally dropped session stays deleted instead of being recreated by a straggling result ([#5743](https://github.com/can1357/oh-my-pi/issues/5743)). -- Fixed the editor status line silently dropping lower-priority segments in narrow terminals; configured segments now flow onto continuation rows in priority order ([#5749](https://github.com/can1357/oh-my-pi/issues/5749)). - Fixed Claude Code marketplace plugins with `scope: "local"` leaking skills, hooks, tools, commands, and MCP servers into unrelated projects ([#5750](https://github.com/can1357/oh-my-pi/issues/5750)). - Fixed headless `omp -p` waiting indefinitely after a completed turn when final mnemopi consolidation stalls; print mode now applies the same bounded consolidation shutdown budget as interactive exit and reaps the embed worker ([#5753](https://github.com/can1357/oh-my-pi/issues/5753)). - Fixed explicit-tool sessions bypassing `xd://` presentation for ambient discoverable custom and MCP tools, which sent their schemas top-level and could exceed provider tool limits or trigger schema-compatibility errors. diff --git a/packages/coding-agent/src/modes/components/status-line/component.test.ts b/packages/coding-agent/src/modes/components/status-line/component.test.ts index e5d8c2295..0f5efdeb9 100644 --- a/packages/coding-agent/src/modes/components/status-line/component.test.ts +++ b/packages/coding-agent/src/modes/components/status-line/component.test.ts @@ -77,10 +77,7 @@ describe("StatusLineComponent", () => { // Let's get the border and see if Prewalk is rendered. const border = statusLine.getTopBorder(100); // SGR codes might be included, so we check if the stripped content contains "Prewalk" - const stripped = border.lines - .map(line => line.content) - .join("\n") - .replace(/\x1b\[[0-9;]*m/g, ""); + const stripped = border.content.replace(/\x1b\[[0-9;]*m/g, ""); expect(stripped).toContain("Prewalk"); }); }); diff --git a/packages/coding-agent/src/modes/components/status-line/component.ts b/packages/coding-agent/src/modes/components/status-line/component.ts index dea228f5b..cd9d98c41 100644 --- a/packages/coding-agent/src/modes/components/status-line/component.ts +++ b/packages/coding-agent/src/modes/components/status-line/component.ts @@ -2,7 +2,7 @@ import * as fs from "node:fs"; import * as path from "node:path"; import type { AgentMessage } from "@oh-my-pi/pi-agent-core"; import type { AssistantMessage, UsageLimit, UsageReport } from "@oh-my-pi/pi-ai"; -import { type Component, type EditorTopBorder, truncateToWidth, visibleWidth } from "@oh-my-pi/pi-tui"; +import { type Component, truncateToWidth, visibleWidth } from "@oh-my-pi/pi-tui"; import { getProjectDir } from "@oh-my-pi/pi-utils"; import { settings } from "../../../config/settings"; import type { AgentSession } from "../../../session/agent-session"; @@ -1131,7 +1131,7 @@ export class StatusLineComponent implements Component { return theme.fg("statusLineSubagents", `${theme.icon.agents} ${this.#subagentCount} ${noun}`); } - #buildStatusLine(width: number): string[] { + #buildStatusLine(width: number): string { const effectiveSettings = this.#resolveSettings(); const includePath = hasPathSegment(effectiveSettings.leftSegments) || hasPathSegment(effectiveSettings.rightSegments); @@ -1166,12 +1166,15 @@ export class StatusLineComponent implements Component { const sepAnsi = theme.getFgAnsi("statusLineSep"); const subagentBadge = this.#subagentBadgeText(); + // Collect visible segment contents const leftParts: string[] = []; + const leftSegIds: StatusLineSegmentId[] = []; for (const segId of effectiveSettings.leftSegments) { if (subagentBadge && segId === "subagents") continue; const rendered = renderSegment(segId, ctx); if (rendered.visible && rendered.content) { leftParts.push(rendered.content); + leftSegIds.push(segId); } } @@ -1191,9 +1194,10 @@ export class StatusLineComponent implements Component { if (subagentBadge) { rightParts.unshift(subagentBadge); } - if (leftParts.length === 0 && rightParts.length === 0) return []; - const topFillWidth = Math.max(0, width); + const left = [...leftParts]; + const right = [...rightParts]; + const leftSepWidth = visibleWidth(separatorDef.left); const rightSepWidth = visibleWidth(separatorDef.right); // Transparent mode drops powerline caps (they need a bg fill to bridge), @@ -1208,34 +1212,65 @@ export class StatusLineComponent implements Component { return partsWidth + sepTotal + 2 + capWidth; }; - // Preset order is priority order: fill rows with left segments first, then - // right segments. A segment wider than one row is clipped only after it has - // been isolated, so it never displaces or discards later segments. - const groups: Array<{ left: string[]; right: string[] }> = [{ left: [], right: [] }]; - if (topFillWidth === 0) { - groups[0]!.left.push(...leftParts); - groups[0]!.right.push(...rightParts); - } else { - const orderedParts: Array<{ side: "left" | "right"; parts: string[] }> = [ - { side: "left", parts: leftParts }, - { side: "right", parts: rightParts }, - ]; - for (const { side, parts } of orderedParts) { - for (const part of parts) { - let current = groups[groups.length - 1]!; - const currentSide = current[side]; - currentSide.push(part); - const leftWidth = groupWidth(current.left, leftCapWidth, leftSepWidth); - const rightWidth = groupWidth(current.right, rightCapWidth, rightSepWidth); - const totalWidth = leftWidth + rightWidth + (leftWidth > 0 && rightWidth > 0 ? 1 : 0); - if (totalWidth > topFillWidth && current.left.length + current.right.length > 1) { - currentSide.pop(); - current = { left: [], right: [] }; - current[side].push(part); - groups.push(current); + let leftWidth = groupWidth(left, leftCapWidth, leftSepWidth); + let rightWidth = groupWidth(right, rightCapWidth, rightSepWidth); + const totalWidth = () => leftWidth + rightWidth + (left.length > 0 && right.length > 0 ? 1 : 0); + + if (topFillWidth > 0) { + while (totalWidth() > topFillWidth && right.length > 0) { + right.pop(); + rightWidth = groupWidth(right, rightCapWidth, rightSepWidth); + } + // Shrink path before dropping left segments — path is the only elastic segment + const pathIdx = leftSegIds.indexOf("path"); + if (pathIdx >= 0 && totalWidth() > topFillWidth) { + const overflow = totalWidth() - topFillWidth; + const currentPathVW = visibleWidth(left[pathIdx]); + const minPathVW = 8; // icon + ellipsis + a few chars + const shrinkable = currentPathVW - minPathVW; + if (shrinkable > 0) { + const shrinkBy = Math.min(shrinkable, overflow); + const currentMaxLen = ctx.options.path?.maxLength ?? 40; + let newMaxLen = Math.max(4, Math.min(currentMaxLen, currentPathVW) - shrinkBy); + const pathCtx = (maxLen: number): SegmentContext => ({ + ...ctx, + options: { ...ctx.options, path: { ...ctx.options.path, maxLength: maxLen } }, + }); + let reRendered = renderSegment("path", pathCtx(newMaxLen)); + if (reRendered.visible && reRendered.content) { + // maxLength governs path text, not icon prefix; iterate to compensate + for (let i = 0; i < 8; i++) { + const saved = currentPathVW - visibleWidth(reRendered.content); + if (saved >= shrinkBy) break; + const nextMaxLen = Math.max(4, newMaxLen - (shrinkBy - saved)); + if (nextMaxLen >= newMaxLen) break; // no progress or hit floor + newMaxLen = nextMaxLen; + const adjusted = renderSegment("path", pathCtx(newMaxLen)); + if (!adjusted.visible || !adjusted.content) break; + reRendered = adjusted; + } + left[pathIdx] = reRendered.content; + leftWidth = groupWidth(left, leftCapWidth, leftSepWidth); } } } + const leftOverflowDropIndex = (): number => { + // Preserve the current working directory as long as possible. The + // previous right-to-left pop could collapse a normal-width bar to + // just the model segment, hiding the path before less-critical left + // segments such as model/mode/collab were removed. + for (let i = leftSegIds.length - 1; i >= 0; i--) { + if (leftSegIds[i] !== "path") return i; + } + return left.length - 1; + }; + + while (totalWidth() > topFillWidth && left.length > 0) { + const dropIdx = leftOverflowDropIndex(); + left.splice(dropIdx, 1); + leftSegIds.splice(dropIdx, 1); + leftWidth = groupWidth(left, leftCapWidth, leftSepWidth); + } } const renderGroup = (parts: string[], direction: "left" | "right"): string => { @@ -1260,46 +1295,35 @@ export class StatusLineComponent implements Component { return content; }; + const leftGroup = renderGroup(left, "left"); + const rightGroup = renderGroup(right, "right"); + if (!leftGroup && !rightGroup) return ""; + + if (topFillWidth === 0 || left.length === 0 || right.length === 0) { + return leftGroup + (leftGroup && rightGroup ? " " : "") + rightGroup; + } + + const gapWidth = Math.max(1, topFillWidth - leftWidth - rightWidth); const sessionName = effectiveSettings.sessionAccent !== false ? this.session.sessionManager?.getSessionName() : undefined; const accentHex = sessionName ? getSessionAccentHex(sessionName, theme.getMajorThemeColorHexes(), theme.accentSurfaceLuminance) : undefined; const gapColor = getSessionAccentAnsi(accentHex) ?? theme.getFgAnsi("border"); - const lines: string[] = []; - for (const group of groups) { - const leftGroup = renderGroup(group.left, "left"); - const rightGroup = renderGroup(group.right, "right"); - if (!leftGroup && !rightGroup) continue; - - let content = leftGroup || rightGroup; - if (leftGroup && rightGroup) { - const leftWidth = groupWidth(group.left, leftCapWidth, leftSepWidth); - const rightWidth = groupWidth(group.right, rightCapWidth, rightSepWidth); - const gapWidth = Math.max(1, topFillWidth - leftWidth - rightWidth); - content = `${leftGroup}${gapColor}${theme.boxRound.horizontal.repeat(gapWidth)}\x1b[39m${rightGroup}`; - } - if (topFillWidth > 0 && visibleWidth(content) > topFillWidth) { - content = truncateToWidth(content, topFillWidth); - } - lines.push(content); - } - return lines; + const gapFill = `${gapColor}${theme.boxRound.horizontal.repeat(gapWidth)}\x1b[39m`; + return leftGroup + gapFill + rightGroup; } - /** Builds the prioritized status rows consumed by the editor's top border. */ - getTopBorder(width: number): EditorTopBorder { - let contents = this.#buildStatusLine(width); - if (this.#focusedAgentId) { + getTopBorder(width: number): { content: string; width: number } { + let content = this.#buildStatusLine(width); + if (this.#focusedAgentId && content) { // Dim the whole bar while focus-proxied. Group/cap terminators emit full // `\x1b[0m` resets that would cancel faint mid-bar, so re-open it after each. - contents = contents.map(content => `\x1b[2m${content.replaceAll("\x1b[0m", "\x1b[0m\x1b[2m")}\x1b[22m`); + content = `\x1b[2m${content.replaceAll("\x1b[0m", "\x1b[0m\x1b[2m")}\x1b[22m`; } return { - lines: contents.map(content => ({ - content, - width: visibleWidth(content), - })), + content, + width: visibleWidth(content), }; } diff --git a/packages/coding-agent/src/modes/controllers/selector-controller.ts b/packages/coding-agent/src/modes/controllers/selector-controller.ts index 5ec5069b5..d1f95d1bf 100644 --- a/packages/coding-agent/src/modes/controllers/selector-controller.ts +++ b/packages/coding-agent/src/modes/controllers/selector-controller.ts @@ -201,10 +201,7 @@ export class SelectorController { getStatusLinePreview: () => { // Return the rendered status line for inline preview const availableWidth = this.ctx.editor.getTopBorderAvailableWidth(this.ctx.ui.terminal.columns); - return this.ctx.statusLine - .getTopBorder(availableWidth) - .lines.map(line => line.content) - .join("\n"); + return this.ctx.statusLine.getTopBorder(availableWidth).content; }, onPluginsChanged: async () => { const projectPath = await resolveActiveProjectRegistryPath(this.ctx.sessionManager.getCwd()); diff --git a/packages/coding-agent/test/status-line-context-cache.test.ts b/packages/coding-agent/test/status-line-context-cache.test.ts index 8f5948236..c465653c4 100644 --- a/packages/coding-agent/test/status-line-context-cache.test.ts +++ b/packages/coding-agent/test/status-line-context-cache.test.ts @@ -214,7 +214,7 @@ describe("StatusLineComponent context breakdown", () => { }); const border = comp.getTopBorder(80); - expect(border.lines.length).toBeGreaterThan(0); + expect(border.content.length).toBeGreaterThan(0); expect(usageCalls()).toBe(0); }); @@ -232,11 +232,7 @@ describe("StatusLineComponent context breakdown", () => { }); // 5000 / 272000 → 1.8%, window formatted as 272K (matches the footer gauge). - const plain = comp - .getTopBorder(80) - .lines.map(line => line.content) - .join("\n") - .replaceAll(/\x1b\[[0-9;]*m/g, ""); + const plain = comp.getTopBorder(80).content.replaceAll(/\x1b\[[0-9;]*m/g, ""); expect(plain).toContain("1.8%/272K"); }); @@ -253,11 +249,7 @@ describe("StatusLineComponent context breakdown", () => { separator: "powerline-thin", }); - const plain = comp - .getTopBorder(80) - .lines.map(line => line.content) - .join("\n") - .replaceAll(/\x1b\[[0-9;]*m/g, ""); + const plain = comp.getTopBorder(80).content.replaceAll(/\x1b\[[0-9;]*m/g, ""); expect(plain).toContain("0.5%/272K"); }); @@ -275,11 +267,7 @@ describe("StatusLineComponent context breakdown", () => { separator: "powerline-thin", }); - const plain = comp - .getTopBorder(80) - .lines.map(line => line.content) - .join("\n") - .replaceAll(/\x1b\[[0-9;]*m/g, ""); + const plain = comp.getTopBorder(80).content.replaceAll(/\x1b\[[0-9;]*m/g, ""); expect(plain).toContain("5K/?"); expect(plain).not.toContain("0.0%/0"); }); diff --git a/packages/coding-agent/test/status-line-overflow.test.ts b/packages/coding-agent/test/status-line-overflow.test.ts index 50e506124..a4a860e00 100644 --- a/packages/coding-agent/test/status-line-overflow.test.ts +++ b/packages/coding-agent/test/status-line-overflow.test.ts @@ -3,6 +3,7 @@ import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; import { resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +import type { StatusLineSegmentId } from "@oh-my-pi/pi-coding-agent/config/settings-schema"; import { StatusLineComponent } from "@oh-my-pi/pi-coding-agent/modes/components/status-line"; import type { SegmentContext } from "@oh-my-pi/pi-coding-agent/modes/components/status-line/segments"; import { renderSegment } from "@oh-my-pi/pi-coding-agent/modes/components/status-line/segments"; @@ -143,20 +144,14 @@ describe("status line session accent", () => { it("paints the gap with the session accent when enabled", () => { const ansi = accentAnsi(); expect(ansi).toBeDefined(); - const border = buildComponent(true) - .getTopBorder(80) - .lines.map(line => line.content) - .join("\n"); + const border = buildComponent(true).getTopBorder(80).content; expect(border).toContain(`${ansi}${theme.boxRound.horizontal}`); }); it("paints the gap with the border color and omits the session accent when disabled", () => { const ansi = accentAnsi(); expect(ansi).toBeDefined(); - const border = buildComponent(false) - .getTopBorder(80) - .lines.map(line => line.content) - .join("\n"); + const border = buildComponent(false).getTopBorder(80).content; // Positive: gap is rendered with the theme border color. expect(border).toContain(`${theme.getFgAnsi("border")}${theme.boxRound.horizontal}`); // Negative: the gap-painting pattern (accent ANSI directly followed by a horizontal @@ -201,14 +196,186 @@ describe("path segment truncation at varying maxLength", () => { }); }); -describe("overflow continuation lines for left segments", () => { - it("preserves model and path on separate rows when they cannot fit together", () => { +describe("overflow: path shrinks before git is dropped", () => { + let tmpDir: string; + + beforeAll(() => { + // Long dir name guarantees the path segment is wide + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "omp-overflow-a-very-long-worktree-directory-name-here-")); + setProjectDir(tmpDir); + }); + + /** + * Simulates the overflow algorithm from #buildStatusLine: + * render left segments, then shrink path before popping, same as production code. + */ + function simulateOverflow( + width: number, + leftSegmentIds: StatusLineSegmentId[], + ctx: SegmentContext, + ): { surviving: StatusLineSegmentId[]; contents: string[] } { + const left: string[] = []; + const leftSegIds: StatusLineSegmentId[] = []; + for (const segId of leftSegmentIds) { + const rendered = renderSegment(segId, ctx); + if (rendered.visible && rendered.content) { + left.push(rendered.content); + leftSegIds.push(segId); + } + } + + // Simplified groupWidth: sum of visible widths + padding between segments + const groupWidth = () => { + if (left.length === 0) return 0; + const partsWidth = left.reduce((sum, p) => sum + visibleWidth(p), 0); + // Each separator gap ~ 3 chars, plus 2 for outer padding + return partsWidth + Math.max(0, left.length - 1) * 3 + 2; + }; + + // Path shrink step (mirrors production code) + const pathIdx = leftSegIds.indexOf("path"); + if (pathIdx >= 0 && groupWidth() > width) { + const overflow = groupWidth() - width; + const currentPathVW = visibleWidth(left[pathIdx]); + const minPathVW = 8; + const shrinkable = currentPathVW - minPathVW; + if (shrinkable > 0) { + const shrinkBy = Math.min(shrinkable, overflow); + const currentMaxLen = ctx.options.path?.maxLength ?? 40; + let newMaxLen = Math.max(4, Math.min(currentMaxLen, currentPathVW) - shrinkBy); + const pathCtx = (maxLen: number): SegmentContext => ({ + ...ctx, + options: { ...ctx.options, path: { ...ctx.options.path, maxLength: maxLen } }, + }); + let reRendered = renderSegment("path", pathCtx(newMaxLen)); + if (reRendered.visible && reRendered.content) { + for (let i = 0; i < 8; i++) { + const saved = currentPathVW - visibleWidth(reRendered.content); + if (saved >= shrinkBy) break; + const nextMaxLen = Math.max(4, newMaxLen - (shrinkBy - saved)); + if (nextMaxLen >= newMaxLen) break; + newMaxLen = nextMaxLen; + const adjusted = renderSegment("path", pathCtx(newMaxLen)); + if (!adjusted.visible || !adjusted.content) break; + reRendered = adjusted; + } + left[pathIdx] = reRendered.content; + } + } + } + + // Left-segment fallback loop. + const leftOverflowDropIndex = (): number => { + for (let i = leftSegIds.length - 1; i >= 0; i--) { + if (leftSegIds[i] !== "path") return i; + } + return left.length - 1; + }; + while (groupWidth() > width && left.length > 0) { + const dropIdx = leftOverflowDropIndex(); + left.splice(dropIdx, 1); + leftSegIds.splice(dropIdx, 1); + } + + return { surviving: [...leftSegIds], contents: [...left] }; + } + + it("keeps git segment when path can be shrunk to fit", () => { + const ctx = createCtx({ pathMaxLength: 40, branch: "feat/long-branch-name" }); + // Use a width that's tight but should fit both after path shrinks + const fullPath = renderSegment("path", ctx); + const fullGit = renderSegment("git", ctx); + const bothWidth = visibleWidth(fullPath.content) + visibleWidth(fullGit.content); + // Set width to ~60% of both segments — forces shrink but should keep both + const tightWidth = Math.floor(bothWidth * 0.6) + 10; + + const result = simulateOverflow(tightWidth, ["path", "git"], ctx); + + expect(result.surviving).toContain("git"); + expect(result.surviving).toContain("path"); + }); + + it("drops git only when terminal is extremely narrow", () => { + const ctx = createCtx({ pathMaxLength: 40, branch: "main" }); + // Absurdly narrow — even minimally-truncated path won't fit with git + const result = simulateOverflow(5, ["path", "git"], ctx); + + // At 5 columns, nothing fits + expect(result.surviving.length).toBeLessThanOrEqual(1); + }); + + it("is a no-op when there is enough space", () => { + const ctx = createCtx({ pathMaxLength: 40, branch: "main" }); + const result = simulateOverflow(200, ["path", "git"], ctx); + + expect(result.surviving).toEqual(["path", "git"]); + }); + + it("shrinks a short path when maxLength exceeds actual path length", () => { + // Short dir name — rendered path is well under the configured maxLength. + const shortDir = fs.mkdtempSync(path.join(os.tmpdir(), "omp-short-")); + setProjectDir(shortDir); + try { + const maxLength = 160; + const ctx = createCtx({ pathMaxLength: maxLength, branch: "feat/long-branch-name" }); + const fullPath = renderSegment("path", ctx); + const fullGit = renderSegment("git", ctx); + const pathVW = visibleWidth(fullPath.content); + const gitVW = visibleWidth(fullGit.content); + + // Sanity: path is shorter than maxLength — this is the bug scenario. + // macOS temp paths can exceed 80 columns once the path icon is included. + expect(pathVW).toBeLessThan(maxLength); + + // Width that fits a shrunken path + git but not the full path + git + const tightWidth = Math.floor(pathVW * 0.5) + gitVW + 10; + + const result = simulateOverflow(tightWidth, ["path", "git"], ctx); + + expect(result.surviving).toContain("path"); + expect(result.surviving).toContain("git"); + } finally { + // Restore for other tests + setProjectDir(tmpDir); + } + }); + it("preserves git when overflow is only 1-2 columns", () => { + const shortDir = fs.mkdtempSync(path.join(os.tmpdir(), "omp-narrow-ovf-")); + setProjectDir(shortDir); + try { + const ctx = createCtx({ pathMaxLength: 80, branch: "main" }); + const fullPath = renderSegment("path", ctx); + const fullGit = renderSegment("git", ctx); + const pathVW = visibleWidth(fullPath.content); + const gitVW = visibleWidth(fullGit.content); + + // Compute exact full width using the test's groupWidth formula: + // partsWidth + (numParts - 1) * 3 + 2 + const fullWidth = pathVW + gitVW + (2 - 1) * 3 + 2; + + // Overflow by exactly 2 columns — the scenario the single-pass missed + const result = simulateOverflow(fullWidth - 2, ["path", "git"], ctx); + + expect(result.surviving).toContain("path"); + expect(result.surviving).toContain("git"); + + // Path must have actually shrunk (proves the loop ran) + const shrunkPathVW = visibleWidth(result.contents[result.surviving.indexOf("path")]); + expect(shrunkPathVW).toBeLessThan(pathVW); + } finally { + setProjectDir(tmpDir); + } + }); +}); + +describe("overflow: path survives before model", () => { + it("drops the model segment before the cwd path when both cannot fit", () => { const root = fs.mkdtempSync(path.join(os.tmpdir(), "omp-statusline-overflow-")); const cwd = path.join(root, "cwdxyz"); fs.mkdirSync(cwd); setProjectDir(cwd); - const modelName = `MODEL_MUST_CONTINUE_${"x".repeat(24)}`; + const modelName = `MODEL_SHOULD_DROP_${"x".repeat(24)}`; const session = createStatusLineSession("overflow test", modelName); const component = new StatusLineComponent(session); const pathOptions = { @@ -239,37 +406,22 @@ describe("overflow continuation lines for left segments", () => { } as SegmentContext; const pi = renderSegment("pi", ctx).content; const model = renderSegment("model", ctx).content; + const minPath = renderSegment("path", { + ...ctx, + options: { ...ctx.options, path: { ...pathOptions, maxLength: 4 } }, + }).content; const separatorWidth = visibleWidth(theme.sep.space); - const width = visibleWidth(pi) + visibleWidth(model) + separatorWidth + 3; + const groupWidth = (parts: string[]) => + parts.reduce((sum, part) => sum + visibleWidth(part), 0) + + Math.max(0, parts.length - 1) * (separatorWidth + 2) + + 2; + const width = groupWidth([pi, model]) + 1; - const border = component.getTopBorder(width); - const rendered = stripAnsi(border.lines.map(line => line.content).join("\n")); + expect(groupWidth([pi, model, minPath])).toBeGreaterThan(width); + expect(groupWidth([pi, minPath])).toBeLessThanOrEqual(width); - expect(border.lines.length).toBeGreaterThan(1); - expect(rendered).toContain(modelName); + const rendered = stripAnsi(component.getTopBorder(width).content); expect(rendered).toContain("xyz"); - }); -}); - -describe("overflow continuation lines", () => { - it("preserves lower-priority segments when one line is too narrow", () => { - const component = new StatusLineComponent(createStatusLineSession("SESSION_MUST_CONTINUE", "PRIORITY_MODEL")); - component.updateSettings({ - preset: "custom", - leftSegments: ["model"], - rightSegments: ["session_name"], - separator: "none", - sessionAccent: false, - transparent: true, - segmentOptions: { - model: { showThinkingLevel: false }, - }, - }); - - const border = component.getTopBorder(24); - const rendered = stripAnsi(border.lines.map(line => line.content).join("\n")); - - expect(rendered).toContain("PRIORITY_MODEL"); - expect(rendered).toContain("SESSION_MUST_CONTINUE"); + expect(rendered).not.toContain("MODEL_SHOULD_DROP"); }); }); diff --git a/packages/coding-agent/test/status-line-settings-cache.test.ts b/packages/coding-agent/test/status-line-settings-cache.test.ts index 35a501a6c..7ff92b50f 100644 --- a/packages/coding-agent/test/status-line-settings-cache.test.ts +++ b/packages/coding-agent/test/status-line-settings-cache.test.ts @@ -127,14 +127,7 @@ describe("StatusLineComponent effective settings cache", () => { expect(secondEffective.separator).toBe("slash"); expect(secondEffective.sessionAccent).toBe(false); expect(secondEffective.segmentOptions.path?.maxLength).toBe(12); - expect( - stripVTControlCharacters( - component - .getTopBorder(80) - .lines.map(line => line.content) - .join("\n"), - ), - ).toContain("Cache Session"); + expect(stripVTControlCharacters(component.getTopBorder(80).content)).toContain("Cache Session"); expect(component.render(80)).toEqual(["lint running"]); }); @@ -164,7 +157,7 @@ describe("StatusLineComponent effective settings cache", () => { const customComponent = makeComponent({ preset: "custom", leftSegments: [], rightSegments: [] }); expect(customComponent.getEffectiveSettingsForTest().leftSegments).toEqual([]); expect(customComponent.getEffectiveSettingsForTest().rightSegments).toEqual([]); - expect(customComponent.getTopBorder(120)).toEqual({ lines: [] }); + expect(customComponent.getTopBorder(120)).toEqual({ content: "", width: 0 }); }); it("surfaces active subagents even when custom segments omit subagents", () => { @@ -172,12 +165,7 @@ describe("StatusLineComponent effective settings cache", () => { component.setSubagentCount(2); - const content = stripVTControlCharacters( - component - .getTopBorder(120) - .lines.map(line => line.content) - .join("\n"), - ); + const content = stripVTControlCharacters(component.getTopBorder(120).content); expect(content).toContain("2 agents"); expect(content).not.toContain("running"); }); @@ -185,22 +173,10 @@ describe("StatusLineComponent effective settings cache", () => { it("keeps plan and hook state dynamic without settings invalidation", () => { const component = makeComponent({ preset: "custom", leftSegments: ["mode"], rightSegments: [] }); const effective = component.getEffectiveSettingsForTest(); - expect( - component - .getTopBorder(80) - .lines.map(line => line.content) - .join("\n"), - ).toBe(""); + expect(component.getTopBorder(80).content).toBe(""); component.setPlanModeStatus({ enabled: true, paused: false }); - expect( - stripVTControlCharacters( - component - .getTopBorder(80) - .lines.map(line => line.content) - .join("\n"), - ), - ).toContain("Plan"); + expect(stripVTControlCharacters(component.getTopBorder(80).content)).toContain("Plan"); expect(component.getEffectiveSettingsForTest()).toBe(effective); component.setHookStatus("hook", "hook running"); diff --git a/packages/coding-agent/test/status-line-transparent.test.ts b/packages/coding-agent/test/status-line-transparent.test.ts index dab9963fd..559a988a6 100644 --- a/packages/coding-agent/test/status-line-transparent.test.ts +++ b/packages/coding-agent/test/status-line-transparent.test.ts @@ -71,18 +71,12 @@ describe("status line transparent background", () => { // otherwise the negative case below would be vacuous. expect(themeBg).toMatch(/\x1b\[48;/); - const border = buildComponent(false) - .getTopBorder(80) - .lines.map(line => line.content) - .join("\n"); + const border = buildComponent(false).getTopBorder(80).content; expect(border).toContain(themeBg); }); it("drops the theme bg fill and powerline caps when enabled", () => { - const border = buildComponent(true) - .getTopBorder(80) - .lines.map(line => line.content) - .join("\n"); + const border = buildComponent(true).getTopBorder(80).content; const themeBg = theme.getBgAnsi("statusLineBg"); // No 48; (background) ANSI escape anywhere in the rendered bar — every bg is diff --git a/packages/coding-agent/test/status-line-usage-refresh.test.ts b/packages/coding-agent/test/status-line-usage-refresh.test.ts index b4b0f4f84..8c76fd30e 100644 --- a/packages/coding-agent/test/status-line-usage-refresh.test.ts +++ b/packages/coding-agent/test/status-line-usage-refresh.test.ts @@ -151,26 +151,12 @@ describe("StatusLineComponent usage refresh", () => { vi.advanceTimersByTime(2_000); await flushMicrotasks(); - expect( - plain( - component - .getTopBorder(80) - .lines.map(line => line.content) - .join("\n"), - ), - ).not.toContain("5h"); + expect(plain(component.getTopBorder(80).content)).not.toContain("5h"); late.resolve(usageReport(42)); await flushMicrotasks(); - expect( - plain( - component - .getTopBorder(80) - .lines.map(line => line.content) - .join("\n"), - ), - ).toContain("5h 42%"); + expect(plain(component.getTopBorder(80).content)).toContain("5h 42%"); }); it("re-fetches usage immediately when the session rotates to another org under the same email", async () => { diff --git a/packages/coding-agent/test/status-line-usage.test.ts b/packages/coding-agent/test/status-line-usage.test.ts index 9acf654df..d596312d4 100644 --- a/packages/coding-agent/test/status-line-usage.test.ts +++ b/packages/coding-agent/test/status-line-usage.test.ts @@ -101,12 +101,7 @@ describe("usage status-line segment", () => { component.refreshUsageInBackground(); await flushUsageRefresh(); - const content = stripVTControlCharacters( - component - .getTopBorder(200) - .lines.map(line => line.content) - .join("\n"), - ); + const content = stripVTControlCharacters(component.getTopBorder(200).content); expect(content).toContain("prolite"); expect(content).toContain("5h"); @@ -128,12 +123,7 @@ describe("usage status-line segment", () => { component.refreshUsageInBackground(); await flushUsageRefresh(); - const content = stripVTControlCharacters( - component - .getTopBorder(200) - .lines.map(line => line.content) - .join("\n"), - ); + const content = stripVTControlCharacters(component.getTopBorder(200).content); expect(content).toContain("prolite"); expect(content).not.toContain("stale"); @@ -172,12 +162,7 @@ describe("usage status-line segment", () => { component.refreshUsageInBackground(); await flushUsageRefresh(); - const content = stripVTControlCharacters( - component - .getTopBorder(200) - .lines.map(line => line.content) - .join("\n"), - ); + const content = stripVTControlCharacters(component.getTopBorder(200).content); expect(content).toContain("prolite"); expect(content).toContain("24%"); @@ -241,32 +226,15 @@ describe("usage status-line segment", () => { component.refreshUsageInBackground(); await flushUsageRefresh(); - expect( - stripVTControlCharacters( - component - .getTopBorder(200) - .lines.map(line => line.content) - .join("\n"), - ), - ).toContain("80%"); + expect(stripVTControlCharacters(component.getTopBorder(200).content)).toContain("80%"); provider = "anthropic"; model.provider = provider; - const immediate = stripVTControlCharacters( - component - .getTopBorder(200) - .lines.map(line => line.content) - .join("\n"), - ); + const immediate = stripVTControlCharacters(component.getTopBorder(200).content); expect(immediate).not.toContain("80%"); await flushUsageRefresh(); - const refreshed = stripVTControlCharacters( - component - .getTopBorder(200) - .lines.map(line => line.content) - .join("\n"), - ); + const refreshed = stripVTControlCharacters(component.getTopBorder(200).content); expect(refreshed).toContain("24%"); }); @@ -292,12 +260,7 @@ describe("usage status-line segment", () => { component.refreshUsageInBackground(); await flushUsageRefresh(); - const content = stripVTControlCharacters( - component - .getTopBorder(200) - .lines.map(line => line.content) - .join("\n"), - ); + const content = stripVTControlCharacters(component.getTopBorder(200).content); expect(content).toContain("5h"); expect(content).toContain("24%"); diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index 388316d2d..e7f168a49 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -2,10 +2,6 @@ ## [Unreleased] -### Breaking Changes - -- Changed `EditorTopBorder` to expose ordered `lines` instead of one `content`/`width` pair, allowing the editor to frame every continuation row rather than truncate one oversized status row ([#5749](https://github.com/can1357/oh-my-pi/issues/5749)). - ### Added - Added a fullscreen overlay mouse-tracking opt-out so selection-first dialogs can preserve native terminal text selection ([#5711](https://github.com/can1357/oh-my-pi/issues/5711)). diff --git a/packages/tui/src/components/editor.ts b/packages/tui/src/components/editor.ts index c65c8ba9d..386abe5ed 100644 --- a/packages/tui/src/components/editor.ts +++ b/packages/tui/src/components/editor.ts @@ -352,18 +352,11 @@ export interface EditorTheme { hintStyle?: (text: string) => string; } -/** One styled row supplied for the editor's top border. */ -export interface EditorTopBorderLine { - /** Status content with any ANSI styling already applied. */ - content: string; - /** Visible cell width of {@link content}. */ - width: number; -} - -/** Ordered status rows rendered above the editor input. */ export interface EditorTopBorder { - /** Styled rows in display order; the first row forms the box top. */ - lines: readonly EditorTopBorderLine[]; + /** The status content (already styled) */ + content: string; + /** Visible width of the content */ + width: number; } interface HistoryEntry { @@ -475,13 +468,12 @@ export class Editor implements Component, Focusable { onAutocompleteCancel?: () => void; disableSubmit: boolean = false; - // Custom top border (for status line integration). Either an eager border + // Custom top border (for status line integration). Either an eager `content` // (set once, reused every frame) or a `provider` that recomputes lazily just // before the editor paints — the second form lets the host coalesce // per-event rebuilds down to one per rendered frame (see #4145). #topBorderContent?: EditorTopBorder; #topBorderProvider?: (availableWidth: number) => EditorTopBorder | undefined; - #topBorderLineCount = 1; #borderVisible = true; constructor(theme: EditorTheme) { @@ -703,7 +695,7 @@ export class Editor implements Component, Focusable { #getVisibleContentHeight(contentLines: number): number { if (this.#maxHeight === undefined) return contentLines; - const verticalChrome = this.#borderVisible ? Math.max(2, this.#topBorderLineCount + 1) : 0; + const verticalChrome = this.#borderVisible ? 2 : 0; return Math.max(1, this.#maxHeight - verticalChrome); } @@ -831,16 +823,6 @@ export class Editor implements Component, Focusable { const topRight = this.borderColor(`${box.horizontal.repeat(paddingX)}${box.topRight}`); const bottomLeft = this.borderColor(`${box.bottomLeft}${box.horizontal}${padding(Math.max(0, paddingX - 1))}`); const horizontal = this.borderColor(box.horizontal); - const topFillWidth = Math.max(0, width - borderWidth * 2); - // Provider (lazy) wins over eager content — a host that installs both - // wants the coalesced path; falling back to eager keeps existing - // setTopBorder callers working unchanged. - const topBorder = borderVisible - ? this.#topBorderProvider - ? this.#topBorderProvider(topFillWidth) - : this.#topBorderContent - : undefined; - this.#topBorderLineCount = topBorder?.lines.length ?? 1; // Layout the text const layoutLines = this.#layoutText(layoutWidth); @@ -852,26 +834,23 @@ export class Editor implements Component, Focusable { if (borderVisible) { // Render top border: ╭─ [status content] ────────────────╮ - if (topBorder?.lines.length) { - for (let index = 0; index < topBorder.lines.length; index++) { - const line = topBorder.lines[index]!; - let content = line.content; - let contentWidth = line.width; - if (contentWidth > topFillWidth) { - content = truncateToWidth(content, topFillWidth); - contentWidth = visibleWidth(content); - } - const fillWidth = Math.max(0, topFillWidth - contentWidth); - if (index === 0) { - result.push(topLeft + content + this.borderColor(box.horizontal.repeat(fillWidth)) + topRight); - } else { - result.push( - this.borderColor(`${box.vertical}${padding(paddingX)}`) + - content + - padding(fillWidth) + - this.borderColor(`${padding(paddingX)}${box.vertical}`), - ); - } + const topFillWidth = Math.max(0, width - borderWidth * 2); + // Provider (lazy) wins over eager content — a host that installs both + // wants the coalesced path; falling back to eager keeps existing + // setTopBorder callers working unchanged. + const topBorder = this.#topBorderProvider ? this.#topBorderProvider(topFillWidth) : this.#topBorderContent; + if (topBorder) { + const { content, width: statusWidth } = topBorder; + if (statusWidth <= topFillWidth) { + // Status fits - add fill after it + const fillWidth = topFillWidth - statusWidth; + result.push(topLeft + content + this.borderColor(box.horizontal.repeat(fillWidth)) + topRight); + } else { + // Status too long - truncate it + const truncated = truncateToWidth(content, Math.max(0, topFillWidth - 1)); + const truncatedWidth = visibleWidth(truncated); + const fillWidth = Math.max(0, topFillWidth - truncatedWidth); + result.push(topLeft + truncated + this.borderColor(box.horizontal.repeat(fillWidth)) + topRight); } } else { result.push(topLeft + horizontal.repeat(topFillWidth) + topRight); diff --git a/packages/tui/test/editor-top-border-provider.test.ts b/packages/tui/test/editor-top-border-provider.test.ts index 066bb8551..0c0598339 100644 --- a/packages/tui/test/editor-top-border-provider.test.ts +++ b/packages/tui/test/editor-top-border-provider.test.ts @@ -17,11 +17,11 @@ * 4. Clearing the provider falls back to the eager slot. */ import { describe, expect, it } from "bun:test"; -import { Editor, type EditorTopBorder } from "../src/components/editor"; +import { Editor, type EditorTopBorder } from "@oh-my-pi/pi-tui/components/editor"; import { defaultEditorTheme } from "./test-themes"; function stubTopBorder(label: string): EditorTopBorder { - return { lines: [{ content: label, width: label.length }] }; + return { content: label, width: label.length }; } describe("Editor lazy top-border provider (#4145)", () => { @@ -91,27 +91,3 @@ describe("Editor lazy top-border provider (#4145)", () => { expect(widths[1]).toBe(editor.getTopBorderAvailableWidth(120)); }); }); - -describe("Editor top-border continuation lines", () => { - it("frames every status row and stays within the height cap", () => { - const editor = new Editor(defaultEditorTheme); - editor.setTopBorder({ - lines: [ - { content: "PRIMARY", width: 7 }, - { content: "CONTINUATION", width: 12 }, - ], - }); - editor.setMaxHeight(4); - editor.setText("first\nsecond"); - editor.focused = true; - editor.setUseTerminalCursor(true); - editor.setImeSafeCursorLayout(true); - - const frame = editor.render(24); - - expect(frame[0]).toContain("PRIMARY"); - expect(frame[1]).toContain("CONTINUATION"); - expect(frame[1]).toContain(defaultEditorTheme.symbols.boxRound.vertical); - expect(frame).toHaveLength(4); - }); -});