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
2 changes: 2 additions & 0 deletions .changeset/calm-extension-runtime.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
---
---
338 changes: 74 additions & 264 deletions src/ui/App.tsx

Large diffs are not rendered by default.

181 changes: 181 additions & 0 deletions src/ui/hooks/useExtensionCommandRunner.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,181 @@
import { describe, expect, test } from "bun:test";
import { testRender } from "@opentui/react/test-utils";
import { act } from "react";
import type {
ExtensionCommandContext,
ExtensionCommandControls,
ExtensionDialogs,
ExtensionFileViewControls,
ExtensionKeyboardModeControls,
ExtensionLineHighlightControls,
ExtensionPaneControls,
ExtensionReviewControls,
ExtensionReviewNavigation,
ExtensionReviewSelection,
ExtensionWorkspace,
} from "../../extension-api/types";
import { createEmptyExtensionLoadResult, type RegisteredCommand } from "../../extensions/types";
import { useExtensionCommandRunner } from "./useExtensionCommandRunner";

const commandControls = {} as ExtensionCommandControls;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Partial mocks bypass typing

The new hook tests construct partial extension-capability fixtures with {} as ... assertions instead of the required @total-typescript/shoehorn helpers. This bypasses structural checking and allows capability contracts to change without useful compile-time failures; the same pattern appears in useExtensionEventContextProvider.test.tsx.

Context Used: testing.mdc Cursor rule (source)

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ui/hooks/useExtensionCommandRunner.test.tsx
Line: 20

Comment:
**Partial mocks bypass typing**

The new hook tests construct partial extension-capability fixtures with `{} as ...` assertions instead of the required `@total-typescript/shoehorn` helpers. This bypasses structural checking and allows capability contracts to change without useful compile-time failures; the same pattern appears in `useExtensionEventContextProvider.test.tsx`.

**Context Used:** testing.mdc Cursor rule ([source](https://github.com/modem-dev/modem/blob/main/.cursor/rules/testing.mdc))

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it 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!

const dialogs = {} as ExtensionDialogs;
const fileViews = {} as ExtensionFileViewControls;
const keyboardModes = {} as ExtensionKeyboardModeControls;
const highlights = {} as ExtensionLineHighlightControls;
const navigation = {} as ExtensionReviewNavigation;
const panes = {} as ExtensionPaneControls;
const review = {} as ExtensionReviewControls;
const workspace = {} as ExtensionWorkspace;
const selection = Object.freeze({
file: null,
hunkIndex: null,
currentLine: null,
}) as ExtensionReviewSelection;

/** Mount the command runner and expose its stable invocation callback. */
async function renderRunner({
createPanes = () => panes,
extensions = createEmptyExtensionLoadResult("/repo"),
}: {
createPanes?: () => ExtensionPaneControls;
extensions?: ReturnType<typeof createEmptyExtensionLoadResult>;
} = {}) {
let run!: (registered: RegisteredCommand) => void;

function Harness() {
run = useExtensionCommandRunner({
commandControls,
createDialogs: () => dialogs,
createFileViewControls: () => fileViews,
createKeyboardModeControls: () => keyboardModes,
createLineHighlightControls: () => highlights,
createNavigation: () => navigation,
createPaneControls: createPanes,
createReviewControls: () => review,
createWorkspaceControls: () => workspace,
extensions,
getSelection: () => selection,
});
return <text>runner</text>;
}

const setup = await testRender(<Harness />, { width: 20, height: 2 });
await act(async () => setup.renderOnce());
return { current: () => run, extensions, setup };
}

/** Build one registered command around a test handler. */
function command(handler: RegisteredCommand["handler"]): RegisteredCommand {
return {
extensionId: "probe",
command: { id: "run", title: "Run", key: "y" },
handler,
};
}

describe("useExtensionCommandRunner", () => {
test("composes every public capability and freezes selection at invocation", async () => {
const harness = await renderRunner();
let context: ExtensionCommandContext | undefined;

try {
harness.current()(
command((ctx) => {
context = ctx;
}),
);

expect(context).toMatchObject({
commands: commandControls,
dialogs,
fileViews,
highlights,
keyboardModes,
navigation,
panes,
review,
selection,
sidebars: panes,
workspace,
});
expect(context?.cwd).toBe("/repo");
expect(Object.isFrozen(context?.selection)).toBe(true);
} finally {
await act(async () => harness.setup.renderer.destroy());
}
});

test("contains context-construction and handler throws with the attributed warning", async () => {
const harness = await renderRunner({
createPanes: () => {
throw new Error("context boom");
},
});
const notifications: Array<{ message: string; type: string }> = [];
const unsubscribe = harness.extensions.notifications.subscribe((notification) =>
notifications.push(notification),
);

try {
expect(() => harness.current()(command(() => {}))).not.toThrow();
expect(notifications.map(({ message, type }) => ({ message, type }))).toEqual([
{
message: 'Extension probe failed command "run" • context boom',
type: "warning",
},
]);
} finally {
unsubscribe();
await act(async () => harness.setup.renderer.destroy());
}
});

test("contains synchronous handler throws with the same attributed warning", async () => {
const harness = await renderRunner();
const notifications: Array<{ message: string; type: string }> = [];
const unsubscribe = harness.extensions.notifications.subscribe((notification) =>
notifications.push(notification),
);

try {
expect(() =>
harness.current()(
command(() => {
throw new Error("sync boom");
}),
),
).not.toThrow();
expect(notifications.map(({ message, type }) => ({ message, type }))).toEqual([
{
message: 'Extension probe failed command "run" • sync boom',
type: "warning",
},
]);
} finally {
unsubscribe();
await act(async () => harness.setup.renderer.destroy());
}
});

test("contains rejected handler promises with the same attributed warning", async () => {
const harness = await renderRunner();
const notifications: Array<{ message: string; type: string }> = [];
const unsubscribe = harness.extensions.notifications.subscribe((notification) =>
notifications.push(notification),
);

try {
harness.current()(command(async () => Promise.reject("async boom")));
await act(async () => Bun.sleep(0));
expect(notifications.map(({ message, type }) => ({ message, type }))).toEqual([
{
message: 'Extension probe failed command "run" • async boom',
type: "warning",
},
]);
} finally {
unsubscribe();
await act(async () => harness.setup.renderer.destroy());
}
});
});
111 changes: 111 additions & 0 deletions src/ui/hooks/useExtensionCommandRunner.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
/**
* Builds extension command contexts and contains failures at the command boundary.
*
* Each invocation freezes selection and review-bound controls at the keypress that started it,
* while navigation and public Hunk commands continue reading committed App state after awaits.
* Context construction, synchronous handler throws, and rejected handler promises all report the
* existing attributed warning without escaping into keyboard or menu dispatch.
*/

import { useCallback } from "react";
import type {
ExtensionCommandContext,
ExtensionCommandControls,
ExtensionDialogs,
ExtensionFileViewControls,
ExtensionKeyboardModeControls,
ExtensionLineHighlightControls,
ExtensionPaneControls,
ExtensionReviewControls,
ExtensionReviewNavigation,
ExtensionReviewSelection,
ExtensionWorkspace,
} from "../../extension-api/types";
import type { ExtensionLoadResult, RegisteredCommand } from "../../extensions/types";

/** Describe an extension command failure without assuming an Error instance. */
function commandFailureMessage(registered: RegisteredCommand, error: unknown) {
const detail = error instanceof Error ? error.message || error.name : String(error);
return (
`Extension ${registered.extensionId} failed command "${registered.command.id}" • ` + detail
);
}

/** Construct and invoke extension commands against the current committed runtime. */
export function useExtensionCommandRunner({
commandControls,
createDialogs,
createFileViewControls,
createKeyboardModeControls,
createLineHighlightControls,
createNavigation,
createPaneControls,
createReviewControls,
createWorkspaceControls,
extensions,
getSelection,
}: {
commandControls: ExtensionCommandControls;
createDialogs: (extensionId: string) => ExtensionDialogs;
createFileViewControls: (extensionId: string) => ExtensionFileViewControls;
createKeyboardModeControls: (
extensionId: string,
registry: ExtensionLoadResult["registry"] | undefined,
) => ExtensionKeyboardModeControls;
createLineHighlightControls: (extensionId: string) => ExtensionLineHighlightControls;
createNavigation: (extensionId: string) => ExtensionReviewNavigation;
createPaneControls: (extensionId: string) => ExtensionPaneControls;
createReviewControls: () => ExtensionReviewControls;
createWorkspaceControls: (extensionId: string) => ExtensionWorkspace;
extensions?: ExtensionLoadResult;
getSelection: () => ExtensionReviewSelection;
}) {
return useCallback(
(registered: RegisteredCommand) => {
const report = (error: unknown) => {
extensions?.context.notify(commandFailureMessage(registered, error), "warning");
};

try {
const panes = createPaneControls(registered.extensionId);
// Build the complete context before invoking the handler; selection is frozen here.
const context: ExtensionCommandContext = {
cwd: extensions?.context.cwd ?? process.cwd(),
commands: commandControls,
keyboardModes: createKeyboardModeControls(registered.extensionId, extensions?.registry),
notify: (message, type) => extensions?.context.notify(message, type),
panes,
sidebars: panes,
fileViews: createFileViewControls(registered.extensionId),
highlights: createLineHighlightControls(registered.extensionId),
review: createReviewControls(),
selection: getSelection(),
dialogs: createDialogs(registered.extensionId),
workspace: createWorkspaceControls(registered.extensionId),
navigation: createNavigation(registered.extensionId),
};

const returned = registered.handler(context);
// Route async rejections through the same warning as synchronous failures.
if (returned && typeof (returned as PromiseLike<void>).then === "function") {
Promise.resolve(returned).catch(report);
}
} catch (error) {
report(error);
}
},
[
commandControls,
createDialogs,
createFileViewControls,
createKeyboardModeControls,
createLineHighlightControls,
createNavigation,
createPaneControls,
createReviewControls,
createWorkspaceControls,
extensions,
getSelection,
],
);
}
Loading
Loading