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
4 changes: 3 additions & 1 deletion apps/desktop/electron/main/plugin-view-host.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@ export type PluginViewOpenRequest = {
* What this view should show, when the opener knows (D320 follow-up).
*
* A work-panel view is opened either from the tool launcher, which has no
* specific subject, or from a chat file reference, which does. The value is
* specific subject, from a chat file reference, which does, or by a plan or
* goal approval artifact, whose host-chosen view receives the artifact path
* (D452). The value is
* opaque to the host: it travels as the entry URL's `piViewOpen` query
* parameter on creation and as the `view:open` event afterwards, and the
* plugin decides what it means. `pi.browser` uses its own chrome channel
Expand Down
5 changes: 3 additions & 2 deletions apps/desktop/src/components/PlanApprovalBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import type {
ProposalKind,
} from "@pi-desktop/shared";
import { useAppStore } from "../stores/app-store";
import { fileWorkPanelTab } from "../lib/work-panel-tabs";
import { preferredFileWorkPanelTab } from "../lib/work-panel-tabs";
import { PLAN_APPROVAL_DEFAULT_MODE } from "../lib/plan-mode-state";
import {
readPlanApprovalMode,
Expand Down Expand Up @@ -56,6 +56,7 @@ export function PlanApprovalBar({ proposal }: { proposal: PlanProposal }) {
const { t } = useTranslation();
const resolvePlan = useAppStore((state) => state.resolvePlan);
const showToast = useAppStore((state) => state.showToast);
const pluginViews = useAppStore((state) => state.pluginViews);
const openWorkPanelTabForSession = useAppStore(
(state) => state.openWorkPanelTabForSession,
);
Expand Down Expand Up @@ -129,7 +130,7 @@ export function PlanApprovalBar({ proposal }: { proposal: PlanProposal }) {
if (!artifactPath) return;
openWorkPanelTabForSession(
proposal.sessionId,
fileWorkPanelTab(artifactPath),
preferredFileWorkPanelTab(artifactPath, pluginViews),
);
};

Expand Down
13 changes: 6 additions & 7 deletions apps/desktop/src/hooks/use-preview-target.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@ import { useAppStore } from "../stores/app-store";
import { api } from "../lib/api";
import { isHtmlFilePath, toWorkspaceRel, type ChatPreviewTarget } from "../lib/chat-links";
import { openHttpUrl } from "../lib/open-http-url";
import { FILE_MANAGER_PLUGIN_TAB, fileManagerPluginTab } from "../lib/work-panel-tabs";
import {
FILE_MANAGER_PLUGIN_TAB,
fileManagerPluginTab,
hasPluginView,
} from "../lib/work-panel-tabs";

/**
* Open one target the transcript named.
Expand Down Expand Up @@ -62,12 +66,7 @@ export function useOpenChatFileRef() {
const showToast = useAppStore((s) => s.showToast);

const fileViewAvailable = useMemo(
() =>
pluginViews.some(
(view) =>
view.pluginId === FILE_MANAGER_PLUGIN_TAB.pluginId &&
view.viewId === FILE_MANAGER_PLUGIN_TAB.viewId,
),
() => hasPluginView(pluginViews, FILE_MANAGER_PLUGIN_TAB),
[pluginViews],
);

Expand Down
29 changes: 29 additions & 0 deletions apps/desktop/src/lib/work-panel-tabs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,35 @@ export function fileManagerPluginTab(location: string): WorkPanelTab {
};
}

/** The identity of one plugin-contributed view, as tabs and manifests key it. */
export type PluginViewRef = { pluginId: string; viewId: string };

/** Whether that view is currently launchable in the work panel. */
export function hasPluginView(
views: readonly PluginViewRef[],
target: PluginViewRef,
): boolean {
return views.some(
(view) => view.pluginId === target.pluginId && view.viewId === target.viewId,
);
}

/**
* The tab the host opens a project file in when the host, not the user, chose
* the file: the bundled file view whenever it is launchable, and the host file
* tab otherwise — the same preference and fallback a chat file reference
* already uses, so a plan or goal artifact lands where the user's other file
* work lives. The bundle is never required: an absent view leaves the host tab.
*/
export function preferredFileWorkPanelTab(
path: string,
pluginViews: readonly PluginViewRef[],
): WorkPanelTab {
return hasPluginView(pluginViews, FILE_MANAGER_PLUGIN_TAB)
? fileManagerPluginTab(path)
: fileWorkPanelTab(path);
}

export function parsePluginViewRef(
resource: string | undefined,
): { pluginId: string; viewId: string } | null {
Expand Down
17 changes: 14 additions & 3 deletions apps/desktop/src/stores/app-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ import {
import { settleStoppedAssistantMetrics } from "../lib/context-usage";
import { formatToolValue } from "../lib/tool-display";
import { withReviewChangeState } from "../lib/workspace-review";
import { fileWorkPanelTab } from "../lib/work-panel-tabs";
import { preferredFileWorkPanelTab } from "../lib/work-panel-tabs";
import {
clearSessionPermissions,
enqueuePermission,
Expand Down Expand Up @@ -312,12 +312,13 @@ export type AppState = import("./app-state").AppState;
function openPlanArtifact(
proposal: PlanProposal,
openWorkPanelTabForSession: AppState["openWorkPanelTabForSession"],
pluginViews: AppState["pluginViews"],
) {
const relativePath = proposal.artifact?.relativePath;
if (!relativePath) return;
openWorkPanelTabForSession(
proposal.sessionId,
fileWorkPanelTab(relativePath),
preferredFileWorkPanelTab(relativePath, pluginViews),
);
}

Expand Down Expand Up @@ -671,8 +672,18 @@ export const useAppStore = create<AppState>((set, get) => {
unreadNotificationCount: notifications.unreadCount,
sessionOutcomes: latestSessionOutcomes(notifications.notifications),
});

// The artifact's surface depends on which plugin views are launchable, and
// the launcher list is only read after `ready`. Resolve it before the
// restore, so the approval artifact does not fall back to the host file tab
// and then take a second tab from `selectSession`.
await get().refreshPluginViews();
for (const proposal of activePendingPlans) {
openPlanArtifact(proposal, get().openWorkPanelTabForSession);
openPlanArtifact(
proposal,
get().openWorkPanelTabForSession,
get().pluginViews,
);
}
saveSidebarPreferences(preferencesFromState(get()));
if (currentWorkspace?.path) {
Expand Down
13 changes: 11 additions & 2 deletions apps/desktop/src/stores/slices/events-slice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export type EventsSliceDependencies = StoreAccess & {
openPlanArtifact: (
proposal: NonNullable<AppState["pendingPlans"][string]>,
openWorkPanelTabForSession: AppState["openWorkPanelTabForSession"],
pluginViews: AppState["pluginViews"],
) => void;
notifyInteractivePrompt: (
sessionId: string,
Expand Down Expand Up @@ -133,7 +134,11 @@ export function createEventsSlice({
});
const checkpoint = get().planCheckpoints[event.sessionId];
if (event.state === "awaiting_approval" && isPendingPlan(checkpoint)) {
openPlanArtifact(checkpoint, get().openWorkPanelTabForSession);
openPlanArtifact(
checkpoint,
get().openWorkPanelTabForSession,
get().pluginViews,
);
}
if (event.state === "awaiting_approval" && !event.proposal) {
void get().restorePendingPlan(event.sessionId);
Expand Down Expand Up @@ -327,7 +332,11 @@ export function createEventsSlice({
if (event.state === "awaiting_approval") {
const checkpoint = get().planCheckpoints[envelope.sessionId];
if (isPendingPlan(checkpoint)) {
openPlanArtifact(checkpoint, get().openWorkPanelTabForSession);
openPlanArtifact(
checkpoint,
get().openWorkPanelTabForSession,
get().pluginViews,
);
}
void get().restorePendingPlan(envelope.sessionId);
notifyInteractivePrompt(envelope.sessionId, "plan");
Expand Down
7 changes: 6 additions & 1 deletion apps/desktop/src/stores/slices/session-slice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ export type SessionSliceDependencies = StoreAccess & {
openPlanArtifact: (
proposal: PlanProposal,
openWorkPanelTabForSession: AppState["openWorkPanelTabForSession"],
pluginViews: AppState["pluginViews"],
) => void;
rememberSessionCompactions: (
sessionId: string,
Expand Down Expand Up @@ -196,7 +197,11 @@ export function createSessionSlice({
),
}));
if (checkpoint && activeProposal) {
openPlanArtifact(checkpoint, get().openWorkPanelTabForSession);
openPlanArtifact(
checkpoint,
get().openWorkPanelTabForSession,
get().pluginViews,
);
}
return activeProposal ? "pending" : "terminal";
} catch {
Expand Down
5 changes: 4 additions & 1 deletion apps/desktop/test/plan-approval-settings.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@ const interactionSource = readStoreModuleSync("slices/interaction-slice.ts");

test("plan approval exposes only the artifact and remembers the selected mode", () => {
assert.match(approvalBar, /proposal\.title/);
assert.match(approvalBar, /fileWorkPanelTab\(artifactPath\)/);
assert.match(
approvalBar,
/preferredFileWorkPanelTab\(artifactPath, pluginViews\)/,
);
assert.match(approvalBar, /openWorkPanelTabForSession/);
assert.match(approvalBar, /const isPending = proposal\.status === "pending"/);
assert.match(approvalBar, /PLAN_APPROVAL_DEFAULT_MODE/);
Expand Down
37 changes: 36 additions & 1 deletion apps/desktop/test/plan-mode-source-contract.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,10 @@ test("plan approval sends exact identities and waits for host confirmation", ()
assert.doesNotMatch(transcriptSource, /PlanApprovalCard|plan-approval-card/);
assert.doesNotMatch(transcriptSource, /\bpendingPlan\b/);
assert.match(storeSource, /openPlanArtifact/);
assert.match(storeSource, /fileWorkPanelTab\(relativePath\)/);
assert.match(
storeSource,
/preferredFileWorkPanelTab\(relativePath, pluginViews\)/,
);
assert.match(barSource, /const isPending = proposal\.status === "pending"/);
const resolveBlock = interactionSource.slice(interactionSource.indexOf("resolvePlan: async"));
assert.match(resolveBlock, /await api\.resolvePlan\(resolution\)/);
Expand All @@ -176,6 +179,38 @@ test("plan approval sends exact identities and waits for host confirmation", ()
assert.doesNotMatch(resolveBlock, /finally[\s\S]*pendingPlans/);
});

test("the startup artifact restore resolves launchable views first", () => {
// The artifact's surface comes from the launchable plugin views, and the
// renderer only reads that list after `ready`. Opening the artifact before
// that read used the host file tab and then took a second tab when
// `selectSession` restored the same approval.
const bootstrapStart = storeSource.indexOf("bootstrap: async");
assert.ok(bootstrapStart > -1, "bootstrap is declared in the store source");
const bootstrap = storeSource.slice(bootstrapStart);
const resolvedViews = bootstrap.indexOf("await get().refreshPluginViews();");
// The same loop shape also runs once before the restore, so search from the
// refresh rather than from the top of `bootstrap`.
const restoreLoop = bootstrap.indexOf(
"for (const proposal of activePendingPlans)",
resolvedViews,
);

assert.ok(resolvedViews > -1, "bootstrap resolves the launchable views");
assert.ok(
restoreLoop > resolvedViews,
"the view list resolves before the pending-plan restore loop",
);
assert.ok(
bootstrap.indexOf("openPlanArtifact(", resolvedViews) > restoreLoop,
"no artifact opens before that loop",
);
// Every slice call site forwards the live list, so a stub list cannot hide
// the wrong surface behind a green run.
for (const slice of [eventsSource, sessionSource]) {
assert.match(slice, /openPlanArtifact\([\s\S]{0,120}?get\(\)\.pluginViews/);
}
});

test("plan approval bar paints the composer plate over the transparent dock", () => {
const barRule = composerCss.match(/\.plan-approval-bar \{[\s\S]*?\n\}/)?.[0] ?? "";
assert.match(barRule, /background:\s*var\(--ds-bg-composer\)/);
Expand Down
27 changes: 27 additions & 0 deletions apps/desktop/test/work-panel-tabs.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,20 @@ import assert from "node:assert/strict";
import test from "node:test";

const {
FILE_MANAGER_PLUGIN_TAB,
activateWorkPanelTabState,
browserPluginTab,
closeWorkPanelTabState,
emptyWorkPanelContext,
fileWorkPanelTab,
hasPluginView,
isKnownWorkPanelTab,
isToolWorkPanelTab,
normalizeWorkPanelFilePath,
newWorkPanelTab,
openWorkPanelTabState,
pluginWorkPanelTab,
preferredFileWorkPanelTab,
replaceWorkPanelTabState,
sanitizeWorkPanelTabsState,
switchWorkPanelContextState,
Expand Down Expand Up @@ -139,6 +142,30 @@ test("only plugin views are launchable tools", () => {
assert.equal(isKnownWorkPanelTab(newWorkPanelTab()), true);
});

test("a host-chosen project file prefers the bundled file view", () => {
// A plan or goal artifact is project markdown the host opens for the user, so
// it lands in the same view the user's own file work uses. The bundle is
// never required: without that view the host file tab remains.
const fileView = { pluginId: "pi.file-manager", viewId: "manager" };
const openedInView = preferredFileWorkPanelTab("plans/plan.md", [fileView]);

assert.equal(hasPluginView([fileView], FILE_MANAGER_PLUGIN_TAB), true);
assert.equal(
hasPluginView(
[{ pluginId: "pi.browser", viewId: "browser" }],
FILE_MANAGER_PLUGIN_TAB,
),
false,
);
assert.equal(openedInView.kind, "plugin");
assert.equal(openedInView.id, "plugin:pi.file-manager/manager");
assert.equal(openedInView.location, "plans/plan.md");
assert.deepEqual(
preferredFileWorkPanelTab("plans/plan.md", []),
fileWorkPanelTab("plans/plan.md"),
);
});

test("empty work panel context has no visible or retained resource state", () => {
assert.deepEqual(emptyWorkPanelContext(), {
open: false,
Expand Down
3 changes: 2 additions & 1 deletion docs/adr/0019-work-panel-subsystems.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ The retained work-panel subsystems are:
navigation policy, permission denial, external popup handling, and
measured bounds clamp.
2. **Review.** Message-owned review snapshots and guarded rollback remain
host-integrated surfaces opened by successful workspace Write/Edit artifacts.
host-integrated; Review opens only on explicit user action and never from a
tool result (D451).
3. **Files.** Project browsing is supplied by the bundled `pi.files` plugin over
the public contributed-view and filesystem APIs.
4. **Transcript resources.** File and URL artifacts remain session-scoped tabs
Expand Down
4 changes: 2 additions & 2 deletions docs/adr/0042-message-scoped-inline-review-cards.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ second renderer-owned diff or durable ownership map.
3. Failed, denied, scratch, clean, non-Git, and missing-workspace results do not
render cards. A background session's card remains attached to its own
transcript and never appears in the currently visible session. The full
Review tab remains available as the all-files current-worktree view and
continues to open from successful workspace artifacts.
Review tab remains available as the all-files current-worktree view; it
opens only on explicit user action and never from a tool result (D451).
4. Remove the session-to-workspace review ownership map. Inline card presence
is derived from the transcript message, the active workspace, and the
shared workspace diff; no review-specific persistence or protocol field is
Expand Down
9 changes: 5 additions & 4 deletions docs/adr/0068-work-panel-keyboard-entry.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,11 @@ the missing capability is panel entry rather than a new resource protocol.
panel context to open at its committed width without creating or activating a
resource tab. Existing tabs, active resource, and Browser resource remain
unchanged.
3. The empty panel header remains the manual resource chooser for Browser
and in-scope plugin views. Artifact triggers continue to create and activate
resources atomically, and background-session artifacts cannot open the
visible panel.
3. The empty panel header remains the manual resource chooser for the Review
row, Browser, and in-scope plugin views. Review opens only on explicit user
action (D451), while a plan or goal approval artifact still creates or
activates a tab in its originating session (D452); background-session
artifacts cannot open the visible panel.
4. The shortcut is ignored while Settings is active and is a no-op without an
active session. No host protocol, IPC channel, or native application-menu
command is added.
Expand Down
10 changes: 6 additions & 4 deletions docs/adr/0105-files-as-a-bundled-plugin.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@ has a different ownership boundary.
2. The bundled plugin is enabled by default, cannot be uninstalled, and can be
disabled by the user. Its filesystem access uses the public permission-gated
read APIs.
3. Only the Files *tool* migrates. Transcript-owned `file:<path>` resources and
Review artifacts remain host-rendered and message/session scoped.
3. Only the Files *tool* migrates. Transcript-owned `file:<path>` resources
stay as they are. Review remains the user-opened surface over the same
transcript-owned evidence and is never opened by a tool result (D451).
4. Browser chrome and agent CDP ship as bundled plugin `pi.browser` (ADR 0170).
The guest `WebContentsView` and debugger remain host window machinery,
reached only through the public `pi.browser.*` API.
Expand All @@ -43,8 +44,9 @@ has a different ownership boundary.

- The shipped plugin is a real consumer of the public contributed-view and
filesystem APIs; gaps in those APIs are caught by a first-party feature.
- The launcher lists Browser and active plugin views. Review and file resources
are opened by conversation artifacts.
- The launcher lists the Review row plus Browser and in-scope plugin views.
File resources are opened by conversation artifacts; Review opens only on
explicit user action (D451).
- The plugin trust boundary stays unchanged: no plugin permission can spawn an
interactive shell.

Expand Down
5 changes: 3 additions & 2 deletions docs/adr/0108-remove-built-in-interactive-terminal.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ invocations and bounded output remain visible in the conversation.

1. Remove the work-panel interactive terminal. The panel retains
plugin-contributed views including the bundled Files and Browser views
(ADR 0170), and Review/file tabs opened by conversation artifacts.
(ADR 0170), file tabs opened by conversation artifacts, and the Review tab
the user opens themselves (D451).
2. Keep Agent Bash unchanged. It remains a permission-aware, non-interactive
agent tool whose command, output, status, copy behavior, and `IconTerminal`
presentation stay in the transcript. Generic lifecycle values such as
Expand All @@ -48,7 +49,7 @@ invocations and bounded output remain visible in the conversation.
## Consequences

- The work panel has no interactive shell tab or terminal launcher, and its
empty state lists Browser and in-scope plugin views only.
empty state lists the Review row, Browser, and in-scope plugin views.
- Desktop packaging no longer carries the PTY native module or terminal
renderer dependencies, reducing native build and release surface.
- Interactive shell workflows require an external terminal. Agent Bash remains
Expand Down
Loading
Loading