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
5 changes: 4 additions & 1 deletion packages/webui/docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,10 @@ a browser tab strip and the two selections it separates, and amends ADR 0019 wit
them they are the decision record for that feature. ADR 0020 records the accepted Mini App view, the runtime seams it needs, the
desktop capabilities it deliberately drops, and the contract-baseline question it leaves open; it
is the sole decision record for Mini App hosting. ADR 0021 records the model picker as one popup
with two states, and the cascade it replaces. A new decision takes the next free number.
with two states, and the cascade it replaces. ADR 0024 records the client-side workspace boundary
for a diff body: a diff entry outside the workspace keeps its name and its counts and loses its
content, the boundary is lexical, and a diff whose workspace root the client does not know is not
withheld. A new decision takes the next free number.

| Retired | Was | Why it is not in the log | Where the live facts are |
| --- | --- | --- | --- |
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
# The client draws the workspace boundary for a diff body

**Status:** Accepted and implemented.
**Date:** 2026-10-23.

A turn's diff lists the files a turn touched, and one of those files can be **outside the
workspace**. The runtime's capture layer records an out-of-workspace write as an absolute path, so
the entry reaches the client with that file's content attached, and the client used to render it:
the diff card's own review panel and the workspace review panel both put the patch text on screen.

The **WebUI client** now draws that boundary. A diff entry whose path resolves outside the
workspace root keeps its name, its status and its added/deleted line counts, and loses its body —
the diff text, the patch hunks, the file's content. The entry itself stays in the list, and the
surface says what is withheld instead of showing it.

## Why the boundary is drawn here

The fix that stopped the collection at the source was made upstream in
`packages/local-runtime/src/turns/file-changes.ts`, and it was **reverted in PR #240** because it
changed a package this fork may not change: the sixteen upstream packages are outside the boundary
of this branch, and a fix that reaches into one of them cannot be merged here. The upstream fix was
therefore not wrong — it was in the wrong layer for this repository.

So the boundary is drawn on the client side, which is where the content becomes visible. The client
is the last point before the content reaches a screen, and it is the only layer this branch may
change.

This is a **mitigation with a stated scope**, not a replacement for the upstream fix. The upstream
fix also stopped the content from being *collected*; the client-side boundary stops it from being
*shown*. The content still travels between the runtime and the browser for an out-of-workspace
file. Closing that gap properly needs the upstream change.

## The rule is lexical

The browser has no `path` module and no filesystem, so the comparison is made on posix-style
strings, the same normalization the message file-reference reader uses
(`client/features/transcript/projection/message-file-reference.ts`), extended to resolve `..` and
to compare absolute paths segment by segment.

- A **relative** path is under the workspace root by construction, and is refused when it climbs
out of it (`../outside.txt`, `src/../../outside.txt`).
- An **absolute** path is compared with the root segment by segment, so a sibling directory whose
name starts with the root's name (`/work/project-backup` against the root `/work/project`) is
outside. A string prefix would have called it inside.
- The comparison is **case-sensitive except for a Windows drive letter**, and it accepts both
separator spellings, because the runtime writes the path in the host's spelling.

## An unknown root is not a boundary

A diff entry is only withheld when the client **knows** the workspace root. When the root is
missing — a session with no workspace, a surface that has none to give — the entries are shown as
they are.

This is deliberate. A check that hid everything it could not place would hide ordinary content as
well, and the failure mode of "the diff shows nothing" is worse than the one it prevents: it is
silent, it looks like a bug, and it applies to the files a person actually worked on. The check is
therefore allowed to say "inside" only when it can compare, and the surfaces that have no root keep
the previous behaviour.

The same rule is why the boundary is applied by a **projection** and not by the components: the
projection is handed the root, decides once, and hands the entries on. A component that renders
the entries it is given cannot leak a body it was never given.

## What this decision does not settle

- **A symlink inside the workspace that points outside it.** The entry's path is a relative one, so
it stays contained here, and the browser cannot see what it resolves to. Telling the two apart
needs `realpath` on every target, with careful handling of files that do not exist yet, on the
runtime side — the same limit the upstream fix states. The test suite says this out loud rather
than leaving it to be discovered.
- **The content still travels.** Nothing here stops the runtime from sending an out-of-workspace
file's patch to the browser; it stops the browser from showing it. Only the upstream fix stops
the collection.
- **The session-diff operation.** The client can ask the runtime for a whole session's diff
(`getSessionDiff`), and no surface consumes that answer today. When one does, it goes through the
same projection.
2 changes: 1 addition & 1 deletion packages/webui/docs/architecture/module-map.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ where code lives. Locations, not command ownership. The businesses are the direc
| `client/conversation/` | The conversation substrate's mechanisms: `stream-lease.ts`, `stream-loop.ts`, `request-freshness.ts`, `session-scope.ts`, `transcript-history.ts`, `transcript-request-ownership.ts`, `stream-instrumentation.ts` | Called by application owners, not by components. |
| `client/platform/` | Browser IO: `transport.ts`, `event-channel.ts`, `storage.ts`, `wire.ts`, `connection-health.ts`, `browser-dom.ts`, `team-mode.ts`, `no-project.ts`, `session-import.ts`, `session-transfer-{target,download}.ts` | Adapters only; they decide no business rule. |
| `client/bindings/` | React subscriptions and browser effects: `application-context.tsx`, `browser-capabilities.tsx`, `browser-effects.ts`, `composer-store.ts`, `event-channel-adapter.ts`, `navigation.ts`, `port-slices-context.tsx`, `use-connection-health.ts`, `use-query-state.tsx`, `use-session-state.ts`, `version-cells.ts` | Components read snapshots and invoke named commands here; they receive neither a store writer nor a sink factory. |
| `client/projection/common/` | Pure derivations no single business owns: `router.ts`, `event-kinds.ts`, `event-parsers.ts`, `context-menu.ts`, `flyout-position.ts`, `overlay-geometry.ts`, `outside-close.ts`, `paths.ts`, `unread-badge.ts`, `goal-message.ts` | No React, no transport, no browser global. Business-owned projections live in `client/features/<business>/projection/`. |
| `client/projection/common/` | Pure derivations no single business owns: `router.ts`, `event-kinds.ts`, `event-parsers.ts`, `context-menu.ts`, `diff-workspace-boundary.ts`, `flyout-position.ts`, `overlay-geometry.ts`, `outside-close.ts`, `paths.ts`, `unread-badge.ts`, `goal-message.ts` | No React, no transport, no browser global. Business-owned projections live in `client/features/<business>/projection/`. |
| `client/shell/` | Page composition and the shell frame: `WebuiClientFoundationApp.tsx`, `ArchonPage.tsx`, `ArchonShell.tsx`, `ConversationSurface.tsx`, `ConnectionStatus.tsx`, `StartupFallback.tsx`, `WebuiErrorBoundary.tsx`, `WebuiZoneBoundary.tsx`, `NotFound.tsx`, `icons.tsx`, and the `use-*` shell hooks | Compose and present; create no owners, write no application state. |
| `client/ui/` | Presentation primitives shared across businesses: `ContextMenu.tsx`, `ToggleSwitch.tsx`, `activity-phrase.ts`, `settings-glyphs.tsx`, `settings-layout.tsx` (the settings page skeleton: page, section, card, row, divider) | Reaches no business at all. |
| `client/styles/` | `index.css` (the single compiled entry), `tokens.css`, `shell.css` | See `../design/design-language.md`. |
Expand Down
26 changes: 26 additions & 0 deletions packages/webui/docs/known-gaps.md
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,32 @@ cells, while the Mini App gate compares the raw kind string inside its own servi
change to this client's row can reach. The accepted shape moves the Mini App service into the
policy as a wiring cell this client's row carries.

## 10. A symlink out of the workspace is not detectable by the diff boundary

**Implemented, with a stated limit.** A turn's diff can name a file outside the workspace, and the
client now withholds that file's body: the entry keeps its name, its status and its line counts,
and the surface says the content is not shown. The decision and its reasoning are in
[ADR 0024](adr/0024-the-client-draws-the-workspace-boundary-for-a-diff-body.md).

What the boundary cannot do is tell a **symlink inside the workspace that points outside it** from
an ordinary file. The entry's path is a relative one, so it stays contained, and the browser has no
filesystem to resolve it against. Closing that needs `realpath` on every target, with careful
handling of files that do not exist yet, on the runtime side — which is also where the reverted
upstream fix would have had to live. The unit test says so out loud rather than leaving it to be
discovered.

Two further limits are stated rather than hidden, and both are deliberate:

- The comparison is **lexical**. It reads the path as written; it does not resolve it.
- A diff entry is only withheld when the client **knows** the workspace root. Without one the
entries are shown as they are, because a check that hid everything it could not place would hide
ordinary content too.

The upstream half of this is still open: the fix that stopped the runtime from *collecting* an
out-of-workspace file's content was reverted in PR #240 because it changed a package this fork may
not change, so the content still travels between the runtime and the browser. The client stops it
from being shown; only the upstream change stops it from being collected.

## Reported as missing, and actually correct

- **The thirteen runtime-owner cells are not read by this package.** That is expected: they are
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ import {
type WebuiWorktreeSourceSession,
type WebuiWorktreeWorkspace,
} from "../../sessions/projection/worktree-state.public.js";
import {
isWebuiDiffPathWithinWorkspace,
WEBUI_DIFF_OUTSIDE_WORKSPACE_NOTICE,
} from "../../../projection/common/diff-workspace-boundary.js";
import { selectWebuiCatalogFlatPage } from "../../sessions/application/session-catalog.public.js";
import { useWebuiSessionStoreContext } from "../../../bindings/application-context.js";
import {
Expand Down Expand Up @@ -164,6 +168,7 @@ export function ReviewSettingsPage({ workspaceDir, onOpenFileLine }: {
state={state}
visible={visible}
filtering={filtering}
workspaceDir={effectiveWorkspaceDir}
onQueryChange={(query) => {
dispatch({ type: "query-changed", query });
// Live search: the owner is asked for every keystroke, and the reducer
Expand All @@ -174,10 +179,11 @@ export function ReviewSettingsPage({ workspaceDir, onOpenFileLine }: {
onClearFilters={() => dispatch({ type: "clear-filters" })}
onToggleFile={(fileId, willExpand, file) => {
dispatch({ type: "toggle-file", fileId });
if (willExpand && !state.diffs[fileId] && !state.diffErrors[fileId] && state.reviewSnapshotId) {
/* A file outside the workspace is not asked for its patch at all: the
* content is not going to be shown, so it does not have to travel. */
if (willExpand && !state.diffs[fileId] && !state.diffErrors[fileId] && state.reviewSnapshotId && isWebuiDiffPathWithinWorkspace(effectiveWorkspaceDir, file.path)) {
void loadDiffs([fileId], state.reviewSnapshotId);
}
void file;
}}
onOpenFileLine={onOpenFileLine}
/>;
Expand Down Expand Up @@ -278,10 +284,14 @@ export function WebuiReviewPanel({ state, visible, filtering, loading, note, emp
<ul className="webui-review-files">
{rows.map((file) => {
const expanded = state.expandedFileIds.includes(file.fileId);
const diff = state.diffs[file.fileId];
const diffError = state.diffErrors[file.fileId];
const pending = state.loadingFileIds.includes(file.fileId);
return <li className="webui-review-file" key={file.fileId} data-testid="review-file" data-file-id={file.fileId} data-webui-review-file-status={file.status}>
/* The workspace boundary decides whether the file's patch is shown.
* The entry stays in the list either way — the name and the counts
* are what a person can still act on. */
const withinWorkspace = isWebuiDiffPathWithinWorkspace(workspaceDir, file.path);
const diff = withinWorkspace ? state.diffs[file.fileId] : undefined;
const diffError = withinWorkspace ? state.diffErrors[file.fileId] : undefined;
const pending = withinWorkspace && state.loadingFileIds.includes(file.fileId);
return <li className="webui-review-file" key={file.fileId} data-testid="review-file" data-file-id={file.fileId} data-webui-review-file-status={file.status} data-webui-review-outside-workspace={withinWorkspace ? undefined : "true"}>
<button
type="button"
className="webui-review-file-heading"
Expand All @@ -297,6 +307,7 @@ export function WebuiReviewPanel({ state, visible, filtering, loading, note, emp
</span>
</button>
{expanded ? <div className="webui-review-file-body" data-testid="review-file-body">
{!withinWorkspace ? <p className="webui-review-file-error" role="status" data-testid="review-file-outside-workspace">{WEBUI_DIFF_OUTSIDE_WORKSPACE_NOTICE}</p> : null}
{pending ? <p className="webui-review-loading" role="status">正在读取这个文件的补丁…</p> : null}
{diffError ? <p className="webui-review-file-error" role="status" data-testid="review-file-error">{diffError}</p> : null}
{diff ? <ol className="webui-review-lines" data-testid="review-lines">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,7 @@ export function WebuiAssistantBody({
* edited-files summary sits at the end of the message. */}
<WebuiDiffCard
sessionId={sessionId}
workspaceDir={workspaceDir}
assistantMessageId={assistantMessageId ?? messageId}
turnId={turnId}
changeSetId={changeSetId}
Expand Down
21 changes: 17 additions & 4 deletions packages/webui/src/client/features/transcript/ui/DiffCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,14 @@ import { WebuiIconDiffFile, WebuiIconDiffSummary } from "../../../shell/icons.js
import type { WebuiTurnDiffView } from "../../../../shared/contracts/session.js";
import type { WebuiDiffState } from "../../review/contracts/review-view.js";
import { useWebuiTurnReview } from "../../review/bindings/use-turn-review.public.js";
import {
projectWebuiDiffFiles,
WEBUI_DIFF_OUTSIDE_WORKSPACE_NOTICE,
} from "../../../projection/common/diff-workspace-boundary.js";

export function WebuiDiffCard({
sessionId,
workspaceDir,
assistantMessageId,
turnId,
changeSetId,
Expand All @@ -28,6 +33,9 @@ export function WebuiDiffCard({
onReview,
}: {
readonly sessionId?: string;
/** The session's workspace root. It is what the boundary is judged against;
* without it the entries are shown as they are. */
readonly workspaceDir?: string;
readonly assistantMessageId?: string;
readonly turnId?: string;
readonly changeSetId?: string;
Expand Down Expand Up @@ -75,7 +83,10 @@ export function WebuiDiffCard({
);
if (!view) return null;
if (!view || (view.fileChanges ?? []).length === 0) return null;
const files = view.fileChanges ?? [];
// The workspace boundary is applied to the entries themselves, not to the
// markup below: a file outside the workspace arrives here without a body, so
// there is nothing for the renderer to leak even if it forgets to ask.
const files = projectWebuiDiffFiles(view.fileChanges, workspaceDir);
const shown = expanded ? files : files.slice(0, 3);
const totalAdded = files.reduce((sum, file) => sum + file.additions, 0);
const totalDeleted = files.reduce((sum, file) => sum + file.deletions, 0);
Expand Down Expand Up @@ -127,9 +138,9 @@ export function WebuiDiffCard({
</div>
<ul className="webui-diff-files">
{shown.map((file) => (
<li className="webui-diff-file" key={file.file} data-webui-diff-file="true" data-file-path={file.file}>
<li className="webui-diff-file" key={file.file} data-webui-diff-file="true" data-file-path={file.file} data-webui-diff-outside-workspace={file.withinWorkspace ? undefined : "true"}>
<span className="webui-diff-file-icon" data-testid="turn-diff-file-icon"><WebuiIconDiffFile fileName={basenameOf(file.file)} /></span>
<button type="button" className="webui-diff-file-name" data-testid="turn-diff-file-name" title={file.file} onClick={() => { if (view) onReview?.(view, file.file); }}>{basenameOf(file.file)}</button>
<button type="button" className="webui-diff-file-name" data-testid="turn-diff-file-name" title={file.file} onClick={() => { if (view) onReview?.({ ...view, fileChanges: files }, file.file); }}>{basenameOf(file.file)}</button>
<span className="webui-diff-file-stats" data-webui-diff-file-stats="true">
<span className="webui-diff-add">{`+${file.additions}`}</span>
{file.deletions > 0 ? <span className="webui-diff-del">{`-${file.deletions}`}</span> : null}
Expand All @@ -147,7 +158,9 @@ export function WebuiDiffCard({
{files.map((file) => (
<details key={`${file.file}-review`} open>
<summary>{file.file}</summary>
{file.diff ? <pre>{file.diff}</pre> : file.patch ? <pre>{JSON.stringify(file.patch, null, 2)}</pre> : <p>当前运行时没有提供该文件的 patch 预览。</p>}
{file.withinWorkspace
? file.diff ? <pre>{file.diff}</pre> : file.patch ? <pre>{JSON.stringify(file.patch, null, 2)}</pre> : <p>当前运行时没有提供该文件的 patch 预览。</p>
: <p data-testid="turn-diff-outside-workspace">{WEBUI_DIFF_OUTSIDE_WORKSPACE_NOTICE}</p>}
</details>
))}
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import type {
} from "../../../../shared/contracts/session.js";
import type { WebuiTranscriptCommands } from "../contracts/session-read-port.js";
import type { WebuiMessageFileReference } from "../projection/message-file-reference.js";
import { projectWebuiDiffFiles } from "../../../projection/common/diff-workspace-boundary.js";
import type { WorkspacePanelCommand } from "../../workspace/contracts/panel-command.js";
import type {
WebuiHistoricalTurnView,
Expand Down Expand Up @@ -261,7 +262,7 @@ export function MessageItem({
onOpenFile={(reference) => { if (sessionId && workspaceDir) onOpenFile?.({ sessionId, workspaceDir, reference }); }}
onOpenTurnReview={(review, selectedPath) => {
if (!sessionId || !workspaceDir) return;
onOpenTurnReview?.({ type: "open-turn-review", sessionId, workspaceDir, messageId, assistantMessageId: review.sourceMessageId ?? assistantMessageId ?? messageId, ...(turnId ? { turnId } : {}), ...(review.changeSetId ? { changeSetId: review.changeSetId } : {}), files: review.fileChanges, ...(selectedPath ? { selectedPath } : {}) });
onOpenTurnReview?.({ type: "open-turn-review", sessionId, workspaceDir, messageId, assistantMessageId: review.sourceMessageId ?? assistantMessageId ?? messageId, ...(turnId ? { turnId } : {}), ...(review.changeSetId ? { changeSetId: review.changeSetId } : {}), files: projectWebuiDiffFiles(review.fileChanges, workspaceDir), ...(selectedPath ? { selectedPath } : {}) });
}}
assistantMessageId={assistantMessageId ?? initialDiff?.sourceMessageId ?? messageId}
turnId={turnId}
Expand Down
Loading
Loading