smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(coding-agent-tools): fixed OSC8 links to use resolved file paths from session context

Browse Source 
- Fixed edit/read/search/ast-edit/ast-grep outputs to resolve OSC8 links from session cwd.
- Fixed grouped-file output classification to honor headerBase and fileScope for parent path resolution.
- Fixed read and write renderers to use resolved source/resolved paths as hyperlink targets.
This commit is contained in:
can1357
2026-06-07 05:37:10 +02:00
parent c8f920e744
commit 52f7defeab
11 changed files with 280 additions and 106 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/src/edit/renderer.ts
+11 -5
View File
@@ -185,10 +185,14 @@ function getOperationTitle(op: Operation | undefined): string {
function formatEditPathDisplay(
rawPath: string,
uiTheme: Theme,
options?: { rename?: string; firstChangedLine?: number },
options?: { rename?: string; firstChangedLine?: number; linkPath?: string; renameLinkPath?: string },
): string {
// `rawPath`/`rename` are shown (cwd-relative) but the OSC 8 link targets the
// absolute path when known — a relative `rawPath` would yield a `file:///rel`
// URI that resolves against filesystem root instead of cwd.
const linkTarget = options?.linkPath || rawPath;
let pathDisplay = rawPath
? fileHyperlink(rawPath, uiTheme.fg("accent", shortenPath(rawPath)))
? fileHyperlink(linkTarget, uiTheme.fg("accent", shortenPath(rawPath)))
: uiTheme.fg("toolOutput", "…");
if (options?.firstChangedLine) {
@@ -196,7 +200,8 @@ function formatEditPathDisplay(
}
if (options?.rename) {
pathDisplay += ` ${uiTheme.fg("dim", "→")} ${fileHyperlink(options.rename, uiTheme.fg("accent", shortenPath(options.rename)))}`;
const renameTarget = options.renameLinkPath || options.rename;
pathDisplay += ` ${uiTheme.fg("dim", "→")} ${fileHyperlink(renameTarget, uiTheme.fg("accent", shortenPath(options.rename)))}`;
}
return pathDisplay;
@@ -205,7 +210,7 @@ function formatEditPathDisplay(
function formatEditDescription(
rawPath: string,
uiTheme: Theme,
options?: { rename?: string; firstChangedLine?: number },
options?: { rename?: string; firstChangedLine?: number; linkPath?: string; renameLinkPath?: string },
): { language: string; description: string } {
const language = getLanguageFromPath(rawPath) ?? "text";
const icon = uiTheme.fg("muted", uiTheme.getLangIcon(language));
@@ -539,7 +544,8 @@ function renderSingleFileResult(
const firstChangedLine =
(editDiffPreview && "firstChangedLine" in editDiffPreview ? editDiffPreview.firstChangedLine : undefined) ||
(details && !isError ? details.firstChangedLine : undefined);
const { description } = formatEditDescription(rawPath, uiTheme, { rename, firstChangedLine });
const linkPath = details && "path" in details ? details.path : undefined;
const { description } = formatEditDescription(rawPath, uiTheme, { rename, firstChangedLine, linkPath });
// Change stats ride inline on the header bar next to the path.
const previewDiff = editDiffPreview && !("error" in editDiffPreview) ? editDiffPreview.diff : undefined;
packages/coding-agent/src/modes/utils/ui-helpers.ts
-14
View File
@@ -74,9 +74,6 @@ export class UiHelpers {
* we update the previous status line instead of appending new ones to avoid log spam.
*/
showStatus(message: string, options?: { dim?: boolean }): void {
if (this.ctx.isBackgrounded) {
return;
}
const children = this.ctx.chatContainer.children;
const last = children.length > 0 ? children[children.length - 1] : undefined;
const secondLast = children.length > 1 ? children[children.length - 2] : undefined;
@@ -534,9 +531,6 @@ export class UiHelpers {
}
clearEditor(): void {
if (this.ctx.isBackgrounded) {
return;
}
this.ctx.editor.setText("");
this.ctx.pendingImages = [];
this.ctx.pendingImageLinks = [];
@@ -545,18 +539,10 @@ export class UiHelpers {
}
showError(errorMessage: string): void {
if (this.ctx.isBackgrounded) {
process.stderr.write(`Error: ${errorMessage}\n`);
return;
}
this.ctx.present([new Spacer(1), new Text(theme.fg("error", `Error: ${errorMessage}`), 1, 0)]);
}
showWarning(warningMessage: string): void {
if (this.ctx.isBackgrounded) {
process.stderr.write(`Warning: ${warningMessage}\n`);
return;
}
this.ctx.present([new Spacer(1), new Text(theme.fg("warning", `Warning: ${warningMessage}`), 1, 0)]);
}
packages/coding-agent/src/tools/ast-edit.ts
+5 -1
View File
@@ -159,6 +159,9 @@ export interface AstEditToolDetails {
/** Absolute base directory used during the edit. Used by the renderer to resolve
* display-relative paths to absolute paths for OSC 8 hyperlinks. */
searchPath?: string;
/** Session cwd at edit time. Display header paths are cwd-relative, so the
* renderer resolves them against this; `searchPath` is the scope target. */
cwd?: string;
}
export class AstEditTool implements AgentTool<typeof astEditSchema, AstEditToolDetails> {
@@ -272,6 +275,7 @@ export class AstEditTool implements AgentTool<typeof astEditSchema, AstEditToolD
...(cappedParseErrors.length > 0 ? { parseErrors: cappedParseErrors, parseErrorsTotal } : {}),
scopePath,
searchPath: resolvedSearchPath,
cwd: this.session.cwd,
files: fileList,
fileReplacements: [],
};
@@ -560,7 +564,7 @@ export const astEditToolRenderer = {
const allLines = textContent.split("\n");
// Resolve hyperlinks over the whole output so nested directory headers
// reconstruct across the blank-line groups the tree list collapses by.
const contexts = classifyGroupedLines(allLines, details?.searchPath);
const contexts = classifyGroupedLines(allLines, details?.cwd ?? details?.searchPath, details?.searchPath);
const styledLines = allLines.map((line, index) => {
const ctx = contexts[index]!;
// Swap the inner code-frame gutter `│` for a space so it does not nest a
packages/coding-agent/src/tools/ast-grep.ts
+5 -1
View File
@@ -117,6 +117,9 @@ export interface AstGrepToolDetails {
/** Absolute base directory used during search. Used by the renderer to resolve
* display-relative paths to absolute paths for OSC 8 hyperlinks. */
searchPath?: string;
/** Session cwd at search time. Display header/match paths are cwd-relative, so
* the renderer resolves them against this; `searchPath` is the scope target. */
cwd?: string;
}
export class AstGrepTool implements AgentTool<typeof astGrepSchema, AstGrepToolDetails> {
@@ -206,6 +209,7 @@ export class AstGrepTool implements AgentTool<typeof astGrepSchema, AstGrepToolD
...(cappedParseErrors.length > 0 ? { parseErrors: cappedParseErrors, parseErrorsTotal } : {}),
scopePath,
searchPath: resolvedSearchPath,
cwd: this.session.cwd,
files: fileList,
fileMatches: [],
};
@@ -395,7 +399,7 @@ export const astGrepToolRenderer = {
const allLines = textContent.split("\n");
// Resolve hyperlinks over the whole output so nested directory headers
// reconstruct across the blank-line groups the tree list collapses by.
const contexts = classifyGroupedLines(allLines, details?.searchPath);
const contexts = classifyGroupedLines(allLines, details?.cwd ?? details?.searchPath, details?.searchPath);
const styledLines = allLines.map((line, index) => {
const ctx = contexts[index]!;
if (ctx.kind === "dir") {
packages/coding-agent/src/tools/grouped-file-output.ts
+15 -4
View File
@@ -237,12 +237,23 @@ function resolveGroupedPath(parent: string | undefined, name: string): string |
* each header and body line can be linked back to its absolute filesystem path.
* Reconstruction is stack-based (not per-blank-group) so nested directory headers
* resolve correctly across the whole output.
*
* `headerBase` is the directory the displayed (folded) header paths are relative
* to — for grep/ast tools that is the session cwd, since display paths are
* formatted relative to cwd regardless of the (sub)directory the search was
* scoped to. `fileScope` is the initial owning file for body lines that appear
* before any header (single-file scopes have no `#` headers); it defaults to
* `headerBase` and should be passed the scoped file's absolute path.
*/
export function classifyGroupedLines(lines: readonly string[], searchBase: string | undefined): GroupedLineContext[] {
export function classifyGroupedLines(
lines: readonly string[],
headerBase: string | undefined,
fileScope: string | undefined = headerBase,
): GroupedLineContext[] {
const result: GroupedLineContext[] = [];
const dirAtDepth = new Map<number, string>();
// Body lines before any header (single-file scopes) link to the search root.
let currentFile = searchBase;
// Body lines before any header (single-file scopes) link to the scoped file.
let currentFile = fileScope;
const clearDeeper = (depth: number) => {
for (const key of dirAtDepth.keys()) {
@@ -264,7 +275,7 @@ export function classifyGroupedLines(lines: readonly string[], searchBase: strin
result.push({ kind: "file", depth, isUrl: true });
continue;
}
const parent = depth > 1 ? dirAtDepth.get(depth - 1) : searchBase;
const parent = depth > 1 ? dirAtDepth.get(depth - 1) : headerBase;
if (rest.endsWith("/")) {
const name = rest.slice(0, -1).replace(HEADER_SUFFIX_RE, "");
const abs = resolveGroupedPath(parent, name);
packages/coding-agent/src/tools/read.ts
+20 -6
View File
@@ -2337,10 +2337,20 @@ function firstReadSelectorLine(sel: string | undefined): number | undefined {
}
}
/** Absolute fs path the read result actually resolved to, used as the OSC 8 link
* target when the structured `resolvedPath` isn't set (the common plain-file and
* image reads only record the path in `meta.source`). URL/internal sources are
* not fs paths, so only `type: "path"` qualifies. */
function readSourceFsPath(details: ReadToolDetails | undefined): string | undefined {
const source = details?.meta?.source;
return source?.type === "path" ? source.value : undefined;
}
function formatReadPathLink(
rawPath: string,
options: {
resolvedPath?: string;
sourcePath?: string;
suffixResolution?: { from: string; to: string };
offset?: number;
fallbackLabel?: string;
@@ -2352,7 +2362,7 @@ function formatReadPathLink(
const plainDisplayPath = options.suffixResolution
? shortenPath(options.suffixResolution.to)
: shortenPath(basePath || options.resolvedPath || options.fallbackLabel || rawPath);
const target = options.resolvedPath ?? tryResolveInternalUrlSync(basePath);
const target = options.resolvedPath ?? options.sourcePath ?? tryResolveInternalUrlSync(basePath);
const line = firstReadSelectorLine(split.sel) ?? options.offset;
const linkOptions = line !== undefined ? { line } : undefined;
const displayPath = target ? fileHyperlink(target, plainDisplayPath, linkOptions) : plainDisplayPath;
@@ -2403,7 +2413,9 @@ export const readToolRenderer = {
const rawErrorText = result.content?.find(c => c.type === "text")?.text ?? "";
const errorText = (rawErrorText || "Unknown error").replace(/^Error:\s*/, "");
const rawPath = args?.file_path || args?.path || "";
const filePath = formatReadPathLink(rawPath, { offset: args?.offset }) || shortenPath(rawPath);
const filePath =
formatReadPathLink(rawPath, { offset: args?.offset, sourcePath: readSourceFsPath(result.details) }) ||
shortenPath(rawPath);
let title = filePath ? `Read ${filePath}` : "Read";
if (args?.offset !== undefined || args?.limit !== undefined) {
const startLine = args.offset ?? 1;
@@ -2454,6 +2466,7 @@ export const readToolRenderer = {
const suffix = details?.suffixResolution;
const displayPath = formatReadPathLink(rawPath, {
resolvedPath: details?.resolvedPath,
sourcePath: readSourceFsPath(details),
suffixResolution: suffix,
fallbackLabel: "image",
});
@@ -2486,12 +2499,13 @@ export const readToolRenderer = {
}
const suffix = details?.suffixResolution;
// resolvedPath is the absolute fs path for fs-backed reads (regular files plus
// local:// / memory:// / skill:// / artifact:// resources). Fall back to a sync
// resolver for fs-backed internal URLs so the title is clickable even before the
// result lands or if the handler didn't populate resolvedPath.
// resolvedPath is the absolute fs path when a read resolved/corrected the
// input (suffix match, internal URL, archive/sqlite/notebook); plain file
// reads only record the absolute path in meta.source, so fall back to that
// (and then to a sync internal-URL resolver) to keep the title clickable.
const displayPath = formatReadPathLink(rawPath, {
resolvedPath: details?.resolvedPath,
sourcePath: readSourceFsPath(details),
suffixResolution: suffix,
offset: args?.offset,
});
packages/coding-agent/src/tools/search.ts
+18 -3
View File
@@ -621,6 +621,10 @@ export interface SearchToolDetails {
/** Absolute base directory used during search. Used by the renderer to resolve
* display-relative paths to absolute paths for OSC 8 hyperlinks. */
searchPath?: string;
/** Session cwd at search time. The renderer resolves the display-relative
* (cwd-relative) header/match paths against this for OSC 8 hyperlinks;
* `searchPath` is the scope label target, not the display-path base. */
cwd?: string;
/** User-supplied paths whose base directory was missing on disk. The tool
* skipped these and continued with the surviving entries; surfaced as a
* non-fatal warning in the renderer and in the model-facing text. */
@@ -1003,6 +1007,7 @@ export class SearchTool implements AgentTool<typeof searchSchema, SearchToolDeta
const details: SearchToolDetails = {
scopePath,
searchPath,
cwd: this.session.cwd,
matchCount: 0,
fileCount: 0,
files: [],
@@ -1129,6 +1134,7 @@ export class SearchTool implements AgentTool<typeof searchSchema, SearchToolDeta
const details: SearchToolDetails = {
scopePath,
searchPath,
cwd: this.session.cwd,
matchCount: selectedMatches.length,
fileCount: fileList.length,
files: fileList,
@@ -1215,10 +1221,11 @@ const URL_HEADER_PREFIX_RE = /^#+\s+/;
function renderSearchDisplayLines(
lines: readonly string[],
searchBase: string | undefined,
headerBase: string | undefined,
fileScope: string | undefined,
uiTheme: Theme,
): RenderedSearchLine[] {
const contexts = classifyGroupedLines(lines, searchBase);
const contexts = classifyGroupedLines(lines, headerBase, fileScope);
// `classifyGroupedLines` can't resolve internal URLs (TUI-only), so track the
// resolved URL target here and use it for the body lines that follow.
let urlFile: string | undefined;
@@ -1439,7 +1446,15 @@ export const searchToolRenderer = {
const allLines = textContent.split("\n");
// Resolve hyperlinks once over the whole output so a nested directory stack
// reconstructs correctly across blank-line group boundaries.
const renderedLines = renderSearchDisplayLines(allLines, details?.searchPath, uiTheme);
// Header/match display paths are cwd-relative, so resolve them against cwd
// (falling back to searchPath for legacy results that predate `cwd`); the
// scoped file's absolute path seeds body lines in single-file searches.
const renderedLines = renderSearchDisplayLines(
allLines,
details?.cwd ?? details?.searchPath,
details?.searchPath,
uiTheme,
);
const matchGroups = groupLineIndicesByBlank(allLines).map(indices => indices.map(i => renderedLines[i]!));
const extraLines: string[] = [];
packages/coding-agent/src/tools/write.ts
+23 -7
View File
@@ -18,7 +18,7 @@ import { getDiagnosticsLedger } from "../lsp/diagnostics-ledger";
import { getLanguageFromPath, highlightCode, type Theme } from "../modes/theme/theme";
import writeDescription from "../prompts/tools/write.md" with { type: "text" };
import type { ToolSession } from "../sdk";
import { framedBlock, renderStatusLine } from "../tui";
import { fileHyperlink, framedBlock, renderStatusLine } from "../tui";
import { resolveFileDisplayMode } from "../utils/file-display-mode";
import { truncateForPrompt } from "./approval";
import { parseArchivePathCandidates } from "./archive-reader";
@@ -77,6 +77,9 @@ export interface WriteToolDetails {
meta?: OutputMeta;
/** Set when the file was auto-chmod'd because content begins with a `#!` shebang. */
madeExecutable?: boolean;
/** Absolute filesystem path the write resolved to. Used by the renderer to wrap
* the (possibly cwd-relative) header path in an OSC 8 `file://` hyperlink. */
resolvedPath?: string;
}
/**
@@ -416,7 +419,7 @@ export class WriteTool implements AgentTool<typeof writeSchema, WriteToolDetails
}`;
return {
content: [{ type: "text", text: `Successfully wrote ${content.length} bytes to ${outputPath}` }],
details: {},
details: { resolvedPath: resolvedArchivePath.absolutePath },
};
}
@@ -532,7 +535,10 @@ export class WriteTool implements AgentTool<typeof writeSchema, WriteToolDetails
}
invalidateFsScanAfterWrite(resolvedSqlitePath.absolutePath);
return toolResult<WriteToolDetails>({}).text(resultText).sourcePath(resolvedSqlitePath.absolutePath).done();
return toolResult<WriteToolDetails>({ resolvedPath: resolvedSqlitePath.absolutePath })
.text(resultText)
.sourcePath(resolvedSqlitePath.absolutePath)
.done();
} catch (error) {
if (isEnoent(error)) {
throw new ToolError(`SQLite database '${displayPath}' not found`);
@@ -593,12 +599,13 @@ export class WriteTool implements AgentTool<typeof writeSchema, WriteToolDetails
if (!diagnostics) {
return {
content: [{ type: "text", text: resultText }],
details: {},
details: { resolvedPath: absolutePath },
};
}
return {
content: [{ type: "text", text: resultText }],
details: {
resolvedPath: absolutePath,
diagnostics,
meta: outputMeta()
.diagnostics(diagnostics.summary, diagnostics.messages ?? [])
@@ -871,7 +878,10 @@ export class WriteTool implements AgentTool<typeof writeSchema, WriteToolDetails
if (stripped) {
resultText += `\nNote: auto-stripped hashline display prefixes from content before writing.`;
}
return { content: [{ type: "text", text: resultText }], details: {} };
return {
content: [{ type: "text", text: resultText }],
details: { resolvedPath: absolutePath },
};
}
const diagnostics = await this.#writethrough(absolutePath, cleanContent, signal, undefined, batchRequest);
@@ -888,13 +898,14 @@ export class WriteTool implements AgentTool<typeof writeSchema, WriteToolDetails
if (!diagnostics) {
return {
content: [{ type: "text", text: resultText }],
details: { madeExecutable: madeExecutable || undefined },
details: { resolvedPath: absolutePath, madeExecutable: madeExecutable || undefined },
};
}
return {
content: [{ type: "text", text: resultText }],
details: {
resolvedPath: absolutePath,
diagnostics,
madeExecutable: madeExecutable || undefined,
meta: outputMeta()
@@ -1050,7 +1061,12 @@ export const writeToolRenderer = {
const fileContent = args?.content || "";
const lang = getLanguageFromPath(rawPath);
const langIcon = uiTheme.fg("muted", uiTheme.getLangIcon(lang));
const pathDisplay = filePath ? uiTheme.fg("accent", filePath) : uiTheme.fg("toolOutput", "…");
// The header shows the cwd-relative path but links to the absolute path the
// write resolved to (args.path may be relative, which would yield a broken
// `file://` URI). Falls back to plain text when the result lacks a path.
const linkTarget = result.details?.resolvedPath;
const styledPath = filePath ? uiTheme.fg("accent", filePath) : uiTheme.fg("toolOutput", "…");
const pathDisplay = filePath && linkTarget ? fileHyperlink(linkTarget, styledPath) : styledPath;
if (result.isError) {
const errorText = result.content?.find(c => c.type === "text")?.text ?? "";