diff --git a/apps/cloud/src/services/sources-shadowing.node.test.ts b/apps/cloud/src/services/sources-shadowing.node.test.ts new file mode 100644 index 000000000..02400bcf0 --- /dev/null +++ b/apps/cloud/src/services/sources-shadowing.node.test.ts @@ -0,0 +1,125 @@ +// Workspace + global source listing — verifies that when a workspace source +// shadows a global source by namespace, both rows show up in +// `sources.list` from workspace context, the inner workspace row has no +// `overriddenBy`, and the outer global row carries the workspace scope id +// in its `overriddenBy` field. The cloud sidebar renders the latter as a +// muted `Overridden` entry; see `apps/cloud/src/web/shell.tsx#SourceList`. + +import { describe, expect, it } from "@effect/vitest"; +import { Effect } from "effect"; + +import { + asOrg, + asWorkspace, + orgScopeId, + testWorkspaceScopeId, +} from "./__test-harness__/api-harness"; + +const SHADOW_SPEC = JSON.stringify({ + openapi: "3.0.0", + info: { title: "Shadow API", version: "1.0.0" }, + paths: { + "/ping": { + get: { + operationId: "ping", + summary: "ping", + responses: { "200": { description: "ok" } }, + }, + }, + }, +}); + +describe("sources.list with workspace + global shadowing", () => { + it.effect( + "returns both rows with scopeId + overriddenBy when workspace shadows a global namespace", + () => + Effect.gen(function* () { + const org = `org_${crypto.randomUUID()}`; + const slug = `ws_${crypto.randomUUID().slice(0, 8)}`; + const namespace = `ns_${crypto.randomUUID().replace(/-/g, "_")}`; + const orgScope = orgScopeId(org); + const wsScope = testWorkspaceScopeId(org, slug); + + // Add a global source first. + yield* asOrg(org, (client) => + client.openapi.addSpec({ + params: { scopeId: orgScope }, + payload: { spec: SHADOW_SPEC, namespace }, + }), + ); + + // Then add a workspace source under the same namespace, which + // shadows the global one in this workspace. + yield* asWorkspace(org, slug, (client) => + client.openapi.addSpec({ + params: { scopeId: wsScope }, + payload: { spec: SHADOW_SPEC, namespace }, + }), + ); + + // Listing from workspace context — both rows should be returned, + // with the outer global row marked `overriddenBy: `. + const wsSources = yield* asWorkspace(org, slug, (client) => + client.sources.list({ params: { scopeId: wsScope } }), + ); + const matches = wsSources.filter((s) => s.id === namespace); + expect(matches).toHaveLength(2); + + const effective = matches.find((s) => s.overriddenBy === undefined); + const shadowed = matches.find((s) => s.overriddenBy !== undefined); + expect(effective).toBeDefined(); + expect(shadowed).toBeDefined(); + expect(effective!.scopeId).toBe(wsScope); + expect(shadowed!.scopeId).toBe(orgScope); + expect(shadowed!.overriddenBy).toBe(wsScope); + + // Listing from global context — only the global row, no override. + const orgSources = yield* asOrg(org, (client) => + client.sources.list({ params: { scopeId: orgScope } }), + ); + const orgMatches = orgSources.filter((s) => s.id === namespace); + expect(orgMatches).toHaveLength(1); + expect(orgMatches[0]!.scopeId).toBe(orgScope); + expect(orgMatches[0]!.overriddenBy).toBeUndefined(); + }), + ); + + it.effect( + "non-shadowing workspace + global namespaces both appear without override flags", + () => + Effect.gen(function* () { + const org = `org_${crypto.randomUUID()}`; + const slug = `ws_${crypto.randomUUID().slice(0, 8)}`; + const wsNamespace = `ws_only_${crypto.randomUUID().replace(/-/g, "_")}`; + const orgNamespace = `org_only_${crypto.randomUUID().replace(/-/g, "_")}`; + const orgScope = orgScopeId(org); + const wsScope = testWorkspaceScopeId(org, slug); + + yield* asOrg(org, (client) => + client.openapi.addSpec({ + params: { scopeId: orgScope }, + payload: { spec: SHADOW_SPEC, namespace: orgNamespace }, + }), + ); + yield* asWorkspace(org, slug, (client) => + client.openapi.addSpec({ + params: { scopeId: wsScope }, + payload: { spec: SHADOW_SPEC, namespace: wsNamespace }, + }), + ); + + const wsSources = yield* asWorkspace(org, slug, (client) => + client.sources.list({ params: { scopeId: wsScope } }), + ); + + const wsRow = wsSources.find((s) => s.id === wsNamespace); + const orgRow = wsSources.find((s) => s.id === orgNamespace); + expect(wsRow).toBeDefined(); + expect(orgRow).toBeDefined(); + expect(wsRow!.scopeId).toBe(wsScope); + expect(orgRow!.scopeId).toBe(orgScope); + expect(wsRow!.overriddenBy).toBeUndefined(); + expect(orgRow!.overriddenBy).toBeUndefined(); + }), + ); +}); diff --git a/apps/cloud/src/web/shell.tsx b/apps/cloud/src/web/shell.tsx index a8a36cbe8..18dcde0f2 100644 --- a/apps/cloud/src/web/shell.tsx +++ b/apps/cloud/src/web/shell.tsx @@ -1,8 +1,11 @@ import { Link, Outlet, useLocation, useNavigate } from "@tanstack/react-router"; -import { useEffect, useRef, useState } from "react"; +import { useEffect, useRef, useState, type ReactNode } from "react"; import { useAtomValue } from "@effect/atom-react"; import { useSourcesWithPending } from "@executor-js/react/api/optimistic"; -import { useActiveWriteScopeId } from "@executor-js/react/api/scope-context"; +import { + useActiveWriteScopeId, + useScopeStack, +} from "@executor-js/react/api/scope-context"; import { Button } from "@executor-js/react/components/button"; import { Skeleton } from "@executor-js/react/components/skeleton"; import * as AsyncResult from "effect/unstable/reactivity/AsyncResult"; @@ -137,12 +140,100 @@ function NavItem(props: { // ── SourceList ─────────────────────────────────────────────────────────── +// A source in the listing — taken from the API response shape so we can +// reason about scope buckets and override state without a second type +// declaration. Mirrors `Source` from `@executor-js/sdk` minus optimistic +// flags added by `useSourcesWithPending`. +type SidebarSource = { + readonly id: string; + readonly name: string; + readonly kind: string; + readonly url?: string; + readonly scopeId?: string; + readonly overriddenBy?: string; +}; + +function SourceLink(props: { + source: SidebarSource; + pathname: string; + orgHandle: string; + workspaceSlug: string | null; + onNavigate?: () => void; + overridden?: boolean; +}) { + const { source: s, pathname, orgHandle, workspaceSlug, onNavigate, overridden } = props; + const detailPath = workspaceSlug + ? `/${orgHandle}/${workspaceSlug}/sources/${s.id}` + : `/${orgHandle}/sources/${s.id}`; + const active = pathname === detailPath || pathname.startsWith(`${detailPath}/`); + const to = workspaceSlug + ? "/$org/$workspace/sources/$namespace" + : "/$org/sources/$namespace"; + const params: Record = workspaceSlug + ? { org: orgHandle, workspace: workspaceSlug, namespace: s.id } + : { org: orgHandle, namespace: s.id }; + return ( + + + {s.name} + {overridden ? ( + + Overridden + + ) : ( + + {s.kind} + + )} + + ); +} + +function SidebarSectionLabel(props: { children: ReactNode }) { + return ( +
+ {props.children} +
+ ); +} + function SourceList(props: { pathname: string; onNavigate?: () => void }) { const { orgHandle } = useOrgRoute(); const workspace = useOptionalWorkspaceRoute(); const scopeId = useActiveWriteScopeId(); + const stack = useScopeStack(); const sources = useSourcesWithPending(scopeId); + // Identify which scopes count as "workspace bucket" vs "global bucket". + // The executor builds the workspace stack as + // `[user_workspace, workspace, user_org, org]`. Sources owned by either + // of the first two scopes are workspace sources; the rest are global + // (including `user-org` overrides, which v1 doesn't write but we won't + // hide if they exist). + const workspaceScopes = new Set(); + const globalScopes = new Set(); + if (workspace) { + for (const s of stack) { + if (s.id.startsWith("workspace_") || s.id.startsWith("user_workspace_")) { + workspaceScopes.add(s.id); + } else { + globalScopes.add(s.id); + } + } + } + return AsyncResult.match(sources, { onInitial: () => (
@@ -157,52 +248,94 @@ function SourceList(props: { pathname: string; onNavigate?: () => void }) { onFailure: () => (
No sources yet
), - onSuccess: ({ value }) => - value.length === 0 ? ( -
- No sources yet -
- ) : ( + onSuccess: ({ value }) => { + const all = value as readonly SidebarSource[]; + if (all.length === 0) { + return ( +
+ No sources yet +
+ ); + } + + // Global context — flat list, no buckets. + if (!workspace) { + return ( +
+ {all.map((s) => ( + + ))} +
+ ); + } + + // Workspace context — split into Workspace + Global buckets, with + // shadowed global sources rendered as `Overridden` (still listed so + // the user can see what's inherited and where the override comes + // from). + const ws: SidebarSource[] = []; + const global: SidebarSource[] = []; + for (const s of all) { + if (s.scopeId && workspaceScopes.has(s.scopeId)) { + ws.push(s); + } else if (s.scopeId && globalScopes.has(s.scopeId)) { + global.push(s); + } else { + // Static sources (no scopeId) and rows from scopes outside this + // request's stack land in the global bucket — they're not owned + // by the workspace. + global.push(s); + } + } + + return (
- {value.map((s) => { - const detailPath = workspace - ? `/${orgHandle}/${workspace.workspaceSlug}/sources/${s.id}` - : `/${orgHandle}/sources/${s.id}`; - const active = - props.pathname === detailPath || props.pathname.startsWith(`${detailPath}/`); - const to = workspace - ? "/$org/$workspace/sources/$namespace" - : "/$org/sources/$namespace"; - const params: Record = workspace - ? { - org: orgHandle, - workspace: workspace.workspaceSlug, - namespace: s.id, - } - : { org: orgHandle, namespace: s.id }; - return ( - - - {s.name} - - {s.kind} - - - ); - })} + Workspace + {ws.length === 0 ? ( +
+ No workspace sources +
+ ) : ( + ws.map((s) => ( + + )) + )} + Global + {global.length === 0 ? ( +
+ No global sources +
+ ) : ( + global.map((s) => ( + + )) + )}
- ), + ); + }, }); } diff --git a/packages/core/api/src/handlers/sources.ts b/packages/core/api/src/handlers/sources.ts index a8e52403f..58127a8bc 100644 --- a/packages/core/api/src/handlers/sources.ts +++ b/packages/core/api/src/handlers/sources.ts @@ -22,6 +22,9 @@ export const SourcesHandlers = HttpApiBuilder.group(ExecutorApi, "sources", (han canRemove: s.canRemove, canRefresh: s.canRefresh, canEdit: s.canEdit, + overriddenBy: s.overriddenBy + ? ScopeId.make(s.overriddenBy) + : undefined, })); })), ) diff --git a/packages/core/api/src/sources/api.ts b/packages/core/api/src/sources/api.ts index 2acee2551..b81560047 100644 --- a/packages/core/api/src/sources/api.ts +++ b/packages/core/api/src/sources/api.ts @@ -29,6 +29,9 @@ const SourceResponse = Schema.Struct({ canRemove: Schema.optional(Schema.Boolean), canRefresh: Schema.optional(Schema.Boolean), canEdit: Schema.optional(Schema.Boolean), + /** Set when an inner scope has another row with the same id. The UI + * renders this row as muted/disabled with an `Overridden` badge. */ + overriddenBy: Schema.optional(ScopeId), }); const SourceRemoveResponse = Schema.Struct({ diff --git a/packages/core/sdk/src/executor.test.ts b/packages/core/sdk/src/executor.test.ts index f9fc5b54d..b1c3ac8a7 100644 --- a/packages/core/sdk/src/executor.test.ts +++ b/packages/core/sdk/src/executor.test.ts @@ -1413,13 +1413,19 @@ describe("cross-scope write preservation (SDK)", () => { const outerSources = yield* execOuter.sources.list(); expect(outerSources.map((s) => s.id)).toContain("shared"); - // Inner executor's list is de-duplicated by id (innermost wins), - // so we only expect one entry for "shared" — pinned at the inner - // scope. The fact that it shows up at all (combined with the outer - // executor still seeing its own row above) proves no rows went - // missing. + // Inner executor sees BOTH the effective inner row and the + // shadowed outer row — the latter has `overriddenBy` set so the + // UI can render an `Overridden` badge. Tool invocation still + // resolves to the innermost row (covered by the + // `tools.invoke picks the innermost tool ...` test below). const innerSources = yield* execInner.sources.list(); - expect(innerSources.filter((s) => s.id === "shared")).toHaveLength(1); + const sharedRows = innerSources.filter((s) => s.id === "shared"); + expect(sharedRows).toHaveLength(2); + const effective = sharedRows.find((s) => s.overriddenBy === undefined); + const shadowed = sharedRows.find((s) => s.overriddenBy !== undefined); + expect(effective).toBeDefined(); + expect(shadowed).toBeDefined(); + expect(shadowed!.overriddenBy).toBe(effective!.scopeId); }), ); @@ -1664,7 +1670,7 @@ describe("cross-scope read precedence + remove isolation (SDK)", () => { ); it.effect( - "sources.list dedupes by id, keeping the innermost row", + "sources.list returns shadowed outer rows with overriddenBy set; effective row wins", () => Effect.gen(function* () { const { execOuter, execInner } = yield* makeMarkerExecutors(); @@ -1674,8 +1680,16 @@ describe("cross-scope read precedence + remove isolation (SDK)", () => { const sources = yield* execInner.sources.list(); const shared = sources.filter((s) => s.id === "shared"); - expect(shared).toHaveLength(1); - expect(shared[0]?.name).toBe("inner-name"); + // Both rows surface so the UI can render an `Overridden` badge + // for the shadowed outer row. Only the innermost row is + // "effective" (no `overriddenBy`); tool invocation continues to + // pick that effective row. + expect(shared).toHaveLength(2); + const effective = shared.find((s) => s.overriddenBy === undefined); + const shadowed = shared.find((s) => s.overriddenBy !== undefined); + expect(effective?.name).toBe("inner-name"); + expect(shadowed?.name).toBe("outer-name"); + expect(shadowed?.overriddenBy).toBe(effective?.scopeId); }), ); diff --git a/packages/core/sdk/src/executor.ts b/packages/core/sdk/src/executor.ts index 69588c743..67a593429 100644 --- a/packages/core/sdk/src/executor.ts +++ b/packages/core/sdk/src/executor.ts @@ -1963,30 +1963,41 @@ export const createExecutor = < const listSources = () => Effect.gen(function* () { const dynamic = yield* core.findMany({ model: "source" }); - // Dedup by id with innermost scope winning. Without this, a user - // who shadowed an org-wide source at their inner scope would see - // two rows — their override and the outer default — which is - // inconsistent with how `secrets.list` and every other list - // surface dedup shadowed entries. - const byId = new Map(); - const byIdRank = new Map(); + // Group by id and pick the innermost row as the "effective" one. + // Outer rows under the same id stay in the list with `overriddenBy` + // set to the winning scope id so the UI can render an `Overridden` + // badge (cloud workspace context shows global sources shadowed by + // workspace sources). Tool invocation still uses the innermost + // row — only this list surface returns the duplicates. + const byIdInnermost = new Map(); + const byIdInnermostRank = new Map(); for (const row of dynamic) { const rank = scopeRank(row); - const existing = byIdRank.get(row.id); + const existing = byIdInnermostRank.get(row.id); if (existing === undefined || rank < existing) { - byId.set(row.id, row); - byIdRank.set(row.id, rank); + byIdInnermost.set(row.id, row); + byIdInnermostRank.set(row.id, rank); } } - const dynamicDeduped = [...byId.values()]; const staticList: Source[] = []; for (const { source, pluginId } of staticSources.values()) { staticList.push(staticDeclToSource(source, pluginId)); } - const merged = [...staticList, ...dynamicDeduped.map(rowToSource)]; + const projected: Source[] = dynamic.map((row) => { + const winner = byIdInnermost.get(row.id); + if (!winner || winner.scope_id === row.scope_id) { + return rowToSource(row); + } + return { + ...rowToSource(row), + overriddenBy: winner.scope_id as string, + }; + }); + const merged = [...staticList, ...projected]; yield* Effect.annotateCurrentSpan({ "executor.sources.static_count": staticList.length, - "executor.sources.dynamic_count": dynamicDeduped.length, + "executor.sources.dynamic_count": projected.length, + "executor.sources.dynamic_effective_count": byIdInnermost.size, }); return merged; }).pipe(Effect.withSpan("executor.sources.list")); diff --git a/packages/core/sdk/src/types.ts b/packages/core/sdk/src/types.ts index 9d6891282..aaa6be19c 100644 --- a/packages/core/sdk/src/types.ts +++ b/packages/core/sdk/src/types.ts @@ -35,6 +35,13 @@ export interface Source { * `ctx.core.sources.register(...)`. UI differentiates built-in vs * user-added with this. */ readonly runtime: boolean; + /** When the executor's scope stack contains another source row with the + * same id at an inner scope, that inner scope's id appears here. The + * current row is "shadowed" — it stays in the list so UI can render an + * `Overridden` badge, but tool invocation always picks the innermost + * row. Undefined for the effective row + every static source. See + * `executor.sources.list` for the rule. */ + readonly overriddenBy?: string; } export interface Tool { diff --git a/packages/react/src/pages/sources.tsx b/packages/react/src/pages/sources.tsx index 59cccb4ea..861ff18f0 100644 --- a/packages/react/src/pages/sources.tsx +++ b/packages/react/src/pages/sources.tsx @@ -11,7 +11,7 @@ import { } from "@executor-js/sdk/client"; import { detectSource } from "../api/atoms"; import { useSourcesWithPending } from "../api/optimistic"; -import { useActiveWriteScopeId } from "../hooks/use-scope"; +import { useActiveWriteScopeId, useScopeStack } from "../hooks/use-scope"; import { McpInstallCard } from "../components/mcp-install-card"; import { Button } from "../components/button"; import { Badge } from "../components/badge"; @@ -58,11 +58,38 @@ const bestDetection = ( // Page // --------------------------------------------------------------------------- +// A source row with the fields we care about for grid + bucketing. Mirrors +// the API response in `packages/core/api/src/sources/api.ts` with optional +// optimistic flags layered in by `useSourcesWithPending`. +type SourceRow = { + readonly id: string; + readonly name: string; + readonly kind: string; + readonly url?: string; + readonly runtime?: boolean; + readonly scopeId?: string; + readonly overriddenBy?: string; +}; + export function SourcesPage() { const scopeId = useActiveWriteScopeId(); + const stack = useScopeStack(); const sources = useSourcesWithPending(scopeId); const [connectOpen, setConnectOpen] = useState(false); + // Workspace context iff the stack contains a workspace-prefixed entry. + // Local CLI hosts get a single `org_*` stack and skip the bucket split. + const inWorkspaceContext = stack.some((s) => s.id.startsWith("workspace_")); + const workspaceScopes = useMemo(() => { + const out = new Set(); + for (const s of stack) { + if (s.id.startsWith("workspace_") || s.id.startsWith("user_workspace_")) { + out.add(s.id); + } + } + return out; + }, [stack]); + return (
@@ -85,20 +112,57 @@ export function SourcesPage() { onInitial: () => , onFailure: () =>

Failed to load sources

, onSuccess: ({ value }) => { - const connectedSources = ( - value as Array<{ - readonly id: string; - readonly name: string; - readonly kind: string; - readonly url?: string; - readonly runtime?: boolean; - }> - ).filter((source) => !source.runtime); + const connectedSources = (value as ReadonlyArray).filter( + (source) => !source.runtime, + ); if (connectedSources.length === 0) { return setConnectOpen(true)} />; } + // Workspace context — split into Workspace + Global buckets, + // with shadowed global rows rendered as `Overridden` so users + // can see which inherited sources their workspace replaces. + if (inWorkspaceContext) { + const workspaceSources: SourceRow[] = []; + const globalSources: SourceRow[] = []; + for (const s of connectedSources) { + if (s.scopeId && workspaceScopes.has(s.scopeId)) { + workspaceSources.push(s); + } else { + globalSources.push(s); + } + } + return ( +
+
+

+ Workspace +

+ {workspaceSources.length === 0 ? ( +

+ No workspace sources yet. +

+ ) : ( + + )} +
+
+

+ Global +

+ {globalSources.length === 0 ? ( +

+ No inherited global sources. +

+ ) : ( + + )} +
+
+ ); + } + return (
@@ -398,15 +462,7 @@ function PresetGrid(props: { // Source grid — flat list of connected sources, click-through to detail // --------------------------------------------------------------------------- -function SourceGrid(props: { - sources: readonly { - id: string; - name: string; - kind: string; - url?: string; - runtime?: boolean; - }[]; -}) { +function SourceGrid(props: { sources: readonly SourceRow[] }) { const sourcePlugins = useSourcePlugins(); const pluginByKind = useMemo(() => { const out = new Map(); @@ -421,8 +477,17 @@ function SourceGrid(props: { const pluginKey = KIND_TO_PLUGIN_KEY[s.kind] ?? s.kind; const plugin = pluginByKind.get(pluginKey); const SummaryComponent = plugin?.summary; + const overridden = Boolean(s.overriddenBy); + // A scope+id pair uniquely identifies a row even when the same + // source id appears at two scopes (effective + shadowed global). + const rowKey = `${s.id}-${s.scopeId ?? "static"}`; return ( - + @@ -438,7 +503,11 @@ function SourceGrid(props: { )} {s.runtime && built-in} - {s.kind} + {overridden ? ( + Overridden + ) : ( + {s.kind} + )}