Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/mcp/daemonState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ export class HunkDaemonState {
title: entry.registration.title,
sourceLabel: entry.registration.sourceLabel,
launchedAt: entry.registration.launchedAt,
terminal: entry.registration.terminal,
fileCount: entry.registration.files.length,
files: entry.registration.files,
snapshot: entry.snapshot,
Expand Down
20 changes: 20 additions & 0 deletions src/mcp/sessionRegistration.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,25 @@
import { randomUUID } from "node:crypto";
import { spawnSync } from "node:child_process";
import type { AppBootstrap } from "../core/types";
import { hunkLineRange } from "../core/liveComments";
import { resolveSessionTerminalMetadata } from "./sessionTerminalMetadata";
import type { HunkSessionRegistration, HunkSessionSnapshot, SessionFileSummary } from "./types";

/** Resolve the TTY device path for the current process, if available. */
function ttyname(): string | undefined {
if (!process.stdin.isTTY) {
return undefined;
}

try {
const result = spawnSync("tty", [], { stdio: ["inherit", "pipe", "pipe"] });
const name = result.stdout?.toString().trim();
return name && !name.startsWith("not a tty") ? name : undefined;
} catch {
return undefined;
Comment on lines +14 to +19
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Prefer exit-code check over stdout string matching

The current guard against a failed tty invocation checks whether stdout starts with "not a tty", which is an English-locale assumption. On non-English systems the tty command may emit a localised error string that passes through this check, potentially storing a garbage path. Checking result.status === 0 is locale-independent and more robust:

Suggested change
try {
const result = spawnSync("tty", [], { stdio: ["inherit", "pipe", "pipe"] });
const name = result.stdout?.toString().trim();
return name && !name.startsWith("not a tty") ? name : undefined;
} catch {
return undefined;
const result = spawnSync("tty", [], { stdio: ["inherit", "pipe", "pipe"] });
if (result.status !== 0) {
return undefined;
}
const name = result.stdout?.toString().trim();
return name || undefined;

}
}

function inferRepoRoot(bootstrap: AppBootstrap) {
return bootstrap.input.kind === "git" ||
bootstrap.input.kind === "show" ||
Expand All @@ -24,6 +41,8 @@ function buildSessionFiles(bootstrap: AppBootstrap): SessionFileSummary[] {

/** Build the daemon-facing metadata for one live Hunk TUI session. */
export function createSessionRegistration(bootstrap: AppBootstrap): HunkSessionRegistration {
const terminal = resolveSessionTerminalMetadata({ tty: ttyname() });

return {
sessionId: randomUUID(),
pid: process.pid,
Expand All @@ -33,6 +52,7 @@ export function createSessionRegistration(bootstrap: AppBootstrap): HunkSessionR
title: bootstrap.changeset.title,
sourceLabel: bootstrap.changeset.sourceLabel,
launchedAt: new Date().toISOString(),
terminal,
files: buildSessionFiles(bootstrap),
};
}
Expand Down
120 changes: 120 additions & 0 deletions src/mcp/sessionTerminalMetadata.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
import type { SessionTerminalLocation, SessionTerminalMetadata } from "./types";

function trimmed(value: string | undefined) {
const normalized = value?.trim();
return normalized && normalized.length > 0 ? normalized : undefined;
}

function sameLocation(left: SessionTerminalLocation, right: SessionTerminalLocation) {
return (
left.source === right.source &&
left.tty === right.tty &&
left.windowId === right.windowId &&
left.tabId === right.tabId &&
left.paneId === right.paneId &&
left.terminalId === right.terminalId &&
left.sessionId === right.sessionId
);
}

function pushLocation(locations: SessionTerminalLocation[], location: SessionTerminalLocation) {
if (!locations.some((existing) => sameLocation(existing, location))) {
locations.push(location);
}
}

function inferLocationSource(program: string | undefined) {
const normalized = program?.trim().toLowerCase();
if (!normalized) {
return "terminal";
}

if (normalized === "iterm.app" || normalized === "iterm2") {
return "iterm2";
}

if (normalized === "ghostty") {
return "ghostty";
}

if (normalized === "apple_terminal" || normalized === "apple terminal") {
return "terminal.app";
}

return "terminal";
}

function parseHierarchicalIds(sessionId: string) {
const prefix = sessionId.split(":", 1)[0]?.trim();
if (!prefix) {
return {};
}

const match = /^w(?<window>\d+)t(?<tab>\d+)(?:p(?<pane>\d+))?$/i.exec(prefix);
if (!match?.groups) {
return {};
}

return {
windowId: match.groups.window,
tabId: match.groups.tab,
paneId: match.groups.pane,
} satisfies Pick<SessionTerminalLocation, "windowId" | "tabId" | "paneId">;
Comment on lines +58 to +62
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Optional group spreads an explicit undefined property

When the hierarchical ID has no pane (e.g. "w1t2:ABC"), match.groups.pane is undefined. Spreading { windowId: "1", tabId: "2", paneId: undefined } into the location object creates an explicit paneId: undefined key. While JSON serialisation drops it, it may surprise code that iterates the object's own keys. Consider filtering out undefined entries before returning:

Suggested change
return {
windowId: match.groups.window,
tabId: match.groups.tab,
paneId: match.groups.pane,
} satisfies Pick<SessionTerminalLocation, "windowId" | "tabId" | "paneId">;
return Object.fromEntries(
Object.entries({
windowId: match.groups.window,
tabId: match.groups.tab,
paneId: match.groups.pane,
}).filter(([, v]) => v !== undefined),
) as Pick<SessionTerminalLocation, "windowId" | "tabId" | "paneId">;

}

/**
* Capture terminal- and multiplexer-facing location metadata for one Hunk TUI session.
*
* The structure is intentionally generic so we can layer tmux, iTerm2, Ghostty,
* and future terminal integrations without adding a new top-level field for each one.
*/
export function resolveSessionTerminalMetadata({
env = process.env,
tty,
}: {
env?: NodeJS.ProcessEnv;
tty?: string;
} = {}): SessionTerminalMetadata | undefined {
const termProgram = trimmed(env.TERM_PROGRAM);
const lcTerminal = trimmed(env.LC_TERMINAL);
const program =
termProgram?.toLowerCase() === "tmux" && lcTerminal ? lcTerminal : (termProgram ?? lcTerminal);
const locations: SessionTerminalLocation[] = [];

const ttyPath = trimmed(tty);
if (ttyPath) {
pushLocation(locations, { source: "tty", tty: ttyPath });
}

const tmuxPane = trimmed(env.TMUX_PANE);
if (tmuxPane) {
pushLocation(locations, { source: "tmux", paneId: tmuxPane });
}

const iTermSessionId = trimmed(env.ITERM_SESSION_ID);
if (iTermSessionId) {
pushLocation(locations, {
source: "iterm2",
sessionId: iTermSessionId,
...parseHierarchicalIds(iTermSessionId),
});
}

const terminalSessionId = trimmed(env.TERM_SESSION_ID);
if (terminalSessionId && terminalSessionId !== iTermSessionId) {
pushLocation(locations, {
source: inferLocationSource(program),
sessionId: terminalSessionId,
...parseHierarchicalIds(terminalSessionId),
});
}

if (!program && locations.length === 0) {
return undefined;
}

return {
program,
locations,
};
}
17 changes: 17 additions & 0 deletions src/mcp/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,21 @@ export interface SelectedHunkSummary {
newRange?: [number, number];
}

export interface SessionTerminalLocation {
source: string;
tty?: string;
windowId?: string;
tabId?: string;
paneId?: string;
terminalId?: string;
sessionId?: string;
}

export interface SessionTerminalMetadata {
program?: string;
locations: SessionTerminalLocation[];
}

export interface HunkSessionRegistration {
sessionId: string;
pid: number;
Expand All @@ -31,6 +46,7 @@ export interface HunkSessionRegistration {
title: string;
sourceLabel: string;
launchedAt: string;
terminal?: SessionTerminalMetadata;
files: SessionFileSummary[];
}

Expand Down Expand Up @@ -237,6 +253,7 @@ export interface ListedSession {
title: string;
sourceLabel: string;
launchedAt: string;
terminal?: SessionTerminalMetadata;
fileCount: number;
files: SessionFileSummary[];
snapshot: HunkSessionSnapshot;
Expand Down
77 changes: 73 additions & 4 deletions src/session/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import type {
RemovedCommentResult,
SelectedSessionContext,
SessionLiveCommentSummary,
SessionTerminalLocation,
SessionTerminalMetadata,
} from "../mcp/types";
import {
HUNK_SESSION_API_PATH,
Expand Down Expand Up @@ -359,32 +361,99 @@ function formatSelectedSummary(session: ListedSession) {
return filePath === "(none)" ? filePath : `${filePath} hunk ${hunkNumber}`;
}

function formatTerminalLocation(location: SessionTerminalLocation) {
const parts: string[] = [];

if (location.tty) {
parts.push(location.tty);
}

if (location.windowId) {
parts.push(`window ${location.windowId}`);
}

if (location.tabId) {
parts.push(`tab ${location.tabId}`);
}

if (location.paneId) {
parts.push(`pane ${location.paneId}`);
}

if (location.terminalId) {
parts.push(`terminal ${location.terminalId}`);
}

if (location.sessionId) {
parts.push(`session ${location.sessionId}`);
}

return parts.length > 0 ? parts.join(", ") : "present";
}

function resolveSessionTerminal(session: ListedSession) {
return session.terminal;
}
Comment on lines +394 to +396
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Trivial passthrough wrapper can be inlined

resolveSessionTerminal returns session.terminal unchanged. The two call sites can access the field directly, avoiding an unnecessary indirection that suggests the function performs some computation. Replace both usages with session.terminal directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


function formatTerminalLines(
terminal: SessionTerminalMetadata | undefined,
{
headerLabel,
locationLabel,
}: {
headerLabel: string;
locationLabel: string;
},
) {
if (!terminal) {
return [];
}

return [
...(terminal.program ? [`${headerLabel}: ${terminal.program}`] : []),
...terminal.locations.map(
(location) => `${locationLabel}[${location.source}]: ${formatTerminalLocation(location)}`,
),
];
}

function formatListOutput(sessions: ListedSession[]) {
if (sessions.length === 0) {
return "No active Hunk sessions.\n";
}

return `${sessions
.map((session) =>
[
.map((session) => {
const terminal = resolveSessionTerminal(session);
return [
`${session.sessionId} ${session.title}`,
` repo: ${session.repoRoot ?? session.cwd}`,
...formatTerminalLines(terminal, {
headerLabel: " terminal",
locationLabel: " location",
}),
` focus: ${formatSelectedSummary(session)}`,
` files: ${session.fileCount}`,
` comments: ${session.snapshot.liveCommentCount}`,
].join("\n"),
)
].join("\n");
})
.join("\n\n")}\n`;
}

function formatSessionOutput(session: ListedSession) {
const terminal = resolveSessionTerminal(session);

return [
`Session: ${session.sessionId}`,
`Title: ${session.title}`,
`Source: ${session.sourceLabel}`,
`Repo: ${session.repoRoot ?? session.cwd}`,
`Input: ${session.inputKind}`,
`Launched: ${session.launchedAt}`,
...formatTerminalLines(terminal, {
headerLabel: "Terminal",
locationLabel: "Location",
}),
`Selected: ${formatSelectedSummary(session)}`,
`Agent notes visible: ${session.snapshot.showAgentNotes ? "yes" : "no"}`,
`Live comments: ${session.snapshot.liveCommentCount}`,
Expand Down
Loading
Loading