Stage 2: cross-workspace tool dispatch + identity-bound sessions#272
Open
mgoldsborough wants to merge 29 commits into
Open
Stage 2: cross-workspace tool dispatch + identity-bound sessions#272mgoldsborough wants to merge 29 commits into
mgoldsborough wants to merge 29 commits into
Conversation
…-gated) Pins the Stage 2 load-bearing contract: a single chat invokes a tool from one workspace and a tool from another workspace, with each call landing in the originating workspace's bundle subprocess + audit. Test cases (all five from Tests Required in 001-cross-workspace-e2e.md): - Happy path — both calls complete, per-source counters each 1. - Topology guard — ws_a tool MUST NOT increment ws_b's counter. - Audit attribution — tool.done.workspaceId matches namespace prefix. - Strict invariant — un-namespaced tool name throws from orchestrator. - Unknown prefix — ws_doesnotexist surfaces structured error, no fallback. Gated behind a single grep-able SKIP_UNTIL_T006 constant via Bun's it.if(...) per case. T006 flips the constant to false to turn the contract live. Fixture is shape-clean for T011's smoke-tier reuse.
src/tools/namespace.ts is the single construction/parsing site for
ws_<id>/<name> cross-workspace tool names:
- namespacedToolName(wsId, name) validates wsId against WORKSPACE_ID_RE
(imported from workspace-store, single source of truth) and throws on
invalid input — no silent passthrough.
- parseNamespacedToolName(s) throws structured UnknownNamespacedToolName
on malformed input; no null/undefined fallback. First-/ split so tool
names may contain slashes if a future source emits them (documented +
asserted).
- InvalidNamespacedToolNameInput separates build-side bugs from
parse-side input errors so the orchestrator (T004) can distinguish.
scripts/check-tool-namespace.ts is the AST lint, modeled on
check-conversation-paths.ts. Flags string-literal/template-literal/
binary-plus construction of the ws_X/Y shape and .split("/") on
namespaced bindings. Allowlist: src/tools/namespace.ts itself. Honors
// lint-ok:tool-namespace marker.
Wired into verify:static between check:personal-workspace-id and
check:code-style. CI fails on drift.
Constraint enforced: SPEC_REFERENCE.md § Constraints item 6 — single
construction site, lint-enforced.
Moves user-scoped credentials from users/<userId>/credentials/... to workspaces/ws_user_<userId>/credentials/... so credentials live with the personal workspace they belong to. Required carryover for T008's oauthScope: "user" deprecation — code path assumes credentials are already at the personal-workspace path. Operator surface mirrors Stage 1's migrate-tenant-files.ts: - --dry-run (default) / --apply / --work-dir <path>. - Lock-guarded via acquireMigrationLock (both dry and apply). - Same-FS pre-flight (NB_MIGRATE_FORCE_EXDEV=1 test hook simulates EXDEV; no portable cross-mount setup in CI). - Partial-rename heal: source-missing + destination-present is a no-op. - Idempotent: re-run on a fully-migrated workdir = zero moves, exit 0. - Conflict refusal: both sides exist with different contents = hard error, no auto-merge. - Personal workspace id sourced only via personalWorkspaceIdFor (check:personal-workspace-id passes). Tests (12, integration tier via spawnSync): happy / idempotent / partial-heal / EXDEV / conflict-refusal / lock-contention / stale-lock-takeover / dry-run-read-only / dry-run-on-migrated / helper-only-id-construction / --help / binary-content safety. package.json: migrate:user-creds alias added (was already in place from concurrent T002 work that included this line; commit consolidates).
…outing Introduces the per-call workspace-routing primitive every Stage 2 tool dispatch flows through. Given a namespaced tool name and an identity, the orchestrator parses the namespace via T002's primitive, confirms the workspace exists, authorizes the identity against workspace membership, constructs a fresh WorkspaceContext from the parsed wsId, and resolves the dispatch handle. Per SPEC_REFERENCE.md § Constraints item 3, the context is constructed from the parsed namespace — never from session-level state. Per Constraints item 5 and Stage 1 lesson 3, every failure mode throws a structured error: no `?? "ws_default"` fallbacks, no fall-back to "current workspace." Files added (each disclosed per Group A audit precedent on T002): - src/orchestrator/route.ts — routeToolCall + the structured error taxonomy (UnknownWorkspace, WorkspaceAccessDenied, UnknownToolSource) + the narrow OrchestratorRuntime structural type that lets unit tests stub without booting Runtime. Re-exports UnknownNamespacedToolName from T002 so callers get the full error surface in one place. - src/orchestrator/index.ts — public surface of the orchestrator module. Re-exports routeToolCall + error classes + types only. Internal helpers stay unexported. Coordinates with T005: that task is explicitly instructed NOT to edit this file; Group B integration folds T005's exports in. - test/unit/orchestrator/route.test.ts — 8 cases mapping 1:1 to the task spec's Tests Required (happy / strict invariant / non-member / personal workspace auth / context isolation / no ambient state / unknown workspace) plus one bonus case pinning UnknownToolSource. Each case names the failure mode it pins. Error taxonomy disclosure: the task spec lists three distinct error classes (WorkspaceAccessDenied, UnknownWorkspace, UnknownNamespacedToolName). A fourth, UnknownToolSource, surfaces when the inner toolName's source prefix isn't registered in the target workspace's registry — the alternative is throwing a bare Error from a load-bearing routing path, which the HTTP/audit layer can't distinguish from "tool execution failed." Added rather than dropped or conflated; documented in route.ts module doc-comment. Verification: bun run check / lint / check:cycles / check:tool-namespace / check:personal-workspace-id all green. test/unit/orchestrator/route.test.ts all 8 cases pass.
Files in this change: - src/orchestrator/tool-list-cache.ts (new): watcher-driven per-workspace tool cache + per-identity union cache. Modeled on src/bundles/conversations/src/index-cache.ts — lazy populate, fs.watch invalidation, debounce. Default 100ms debounce; configurable. - src/orchestrator/tool-list-aggregator.ts (new): public surface `createToolListAggregator(...)` returning aggregateToolList / invalidateIdentity / activeWatcherCount / dispose. Every entry built through namespacedToolName() from src/tools/namespace.ts. - test/unit/orchestrator/tool-list-aggregator.test.ts (new): 8 tests — happy path, collision, cache hit, perf-regression guard (N=50 calls, lister fires once per workspace), concurrent enumeration (~100ms, not 500ms), per-identity isolation (disjoint sets), membership-change invalidation, namespace primitive enforcement. - test/integration/tool-list-aggregator-watch.test.ts (new): 3 tests — fs.watch invalidation through workspace.json writes, workspace removal, watcher-leak-free dispose (closes every watcher; post-dispose calls throw). Stage 1 lesson 5 pin: re-scan-per-call (the getUserConversationStore perf footgun) is rejected by the perf-regression guard test. A naive impl would call the per-workspace lister 100× for 50 aggregator calls across 2 workspaces; the cached impl calls it exactly 2×. No touches to src/orchestrator/index.ts (T004's territory) — this PR adds only the two new files in src/orchestrator/ and their tests. Verification: bun run check, lint, check:cycles, check:tool-namespace, check:personal-workspace-id all green. Test suites pass clean (8 unit + 3 integration).
T004 owned index.ts during parallel Group B execution and reserved T005's surface for post-integration. This commit folds T005's public exports (createToolListAggregator + types, ToolListCache + ToolListCacheOptions) into the barrel so external callers reach the aggregator without deep imports. Files modified: - src/orchestrator/index.ts — added T005 re-exports; removed the "Group B coordination" note (now obsolete).
…uting
Stage 2 (delegation-model refactor) Task 007. Hard-cut `workspaceId`
from `SessionMeta` and both registry implementations per Q4 locked.
`/mcp` sessions are now identity-bound: `tools/list` returns the
cross-workspace union via T005's aggregator, `tools/call` parses the
namespaced name via T002's primitive and routes via T004's
orchestrator. The `X-Workspace-Id` header on `/mcp` is no longer
consulted — logged once per session at debug (`NB_DEBUG=mcp`) for
operator visibility. Per Q3 the bridge survives a workspace switch in
the web shell: the switcher cannot influence routing because every
tool call derives its target workspace from the parsed namespace.
Files modified:
src/api/session-store/types.ts — drop `workspaceId` from
`SessionMeta` (Q4 hard cut).
Header doc updated.
src/api/session-store/memory.ts — already minimal; the type
cut alone removes the field.
src/api/session-store/redis.ts — stop writing `workspaceId`
in `HSET`; `parseHash`
ignores any legacy field
rather than failing on it.
src/api/mcp-server.ts — major refactor:
- `McpServerHostOptions`
gains `runtime?: Runtime`.
- `handle()` signature drops
`toolRegistry` + workspace
context; takes
`McpSessionContext { identity }`.
- `TransportEntry` carries
`identityId` instead of
`workspaceId`.
- `tools/list` consumes
`runtime.getToolListAggregator()`.
- `tools/call` parses via
`parseNamespacedToolName`
and routes via
`routeToolCall`. All 4
orchestrator errors map
to distinct JSON-RPC
responses with structured
`error.data.reason`:
-32602 invalid_tool_name
-32602 unknown_workspace
-32602 workspace_access_denied
-32601 unknown_tool_source
- `resources/list` +
`resources/read` aggregate
across every workspace the
identity can access.
- `X-Workspace-Id` is read
once per session for a
debug log line; never used
for routing.
- `fmtSessionContext` drops
the `workspace=` field.
src/api/mcp-task-store.ts — task store re-keyed by
(identityId, taskId) so a
single identity-bound session
can hold tasks across multiple
workspaces. `ownerContext`
retains per-task workspace for
McpSource's downstream
authorization.
src/api/routes/mcp.ts — drop `resolveWorkspace` call;
pass an identity-only
`McpSessionContext` to the host.
src/api/server.ts — wire `runtime` into
`McpServerHost` constructor.
src/runtime/runtime.ts — add `getToolListAggregator()`
accessor (lazy, single
instance; disposed on
`shutdown()`). Production
satisfies `OrchestratorRuntime`
shape via existing accessors.
Tests modified:
test/unit/api/session-store/conformance.ts — drop `workspaceId` from
`sample()`; assert
round-trip carries no
legacy field. Idempotent
re-create test now keys
on `identityId`.
test/unit/api/session-store/redis.test.ts — drop `workspaceId` from
fixture create call.
test/unit/api/mcp-server-host.test.ts — update `handle()` signature
to the new identity-only
context; drop `ToolRegistry`
import (no longer required
by the session-miss surface).
test/unit/api/mcp-server-reclamation.test.ts — same signature update;
uses a shared `SESSION_CTX`
const for brevity.
test/integration/mcp-server-endpoint.test.ts — namespace tool names
(`ws_test/fake__echo`).
Session-miss log assertions
updated for the dropped
`workspace=` field. Drop
`X-Workspace-Id` from the
tests that no longer need
it for routing.
test/integration/mcp-resources.test.ts — `resources/list` now
aggregates across both
workspaces the identity is
a member of; the
cross-workspace test is
rewritten to assert the
new aggregation contract.
test/integration/mcp-tasks.test.ts — namespace every tool name
for tools/call. Reword the
cross-workspace task
isolation test as
cross-session.
test/integration/api/mcp-workspace-scope.test.ts — namespace tool
names; the "denied bundle"
test now asserts the
orchestrator's
`unknown_tool_source` →
-32601 JSON-RPC envelope
instead of `isError: true`.
test/integration/shell-integration.test.ts — namespace
(`ws_test/nb__*`) the
three MCP-client e2e tests.
Tests added:
test/integration/mcp-server-identity-bound.test.ts — Stage 2 T007
contract. Eight tests
pinning every failure mode
on the task spec's
"Tests Required":
- identity-only session
- cross-workspace call
(per-source counters 1,1)
- workspace-switcher header
does not invalidate session
- synapse iframe auto-prefix
routes to host workspace
- strict invariant: bare
name → -32602
- UnknownWorkspace mapping
- WorkspaceAccessDenied
mapping
- UnknownToolSource mapping
test/integration/session-store-legacy-compat.test.ts — round-trip
on both
`InMemorySessionRegistry`
and `RedisSessionRegistry`
surfaces no `workspaceId`;
a legacy Redis hash with
a stray `workspaceId`
loads cleanly and the
parser ignores the field.
Stage 2 (delegation-model refactor) Task 006. Hard-cut `workspaceId`
from `ChatRequest` and `ChatResult` per Q1 / Q4 locked. `runtime.chat()`
is now identity-bound: the session workspace is the identity's personal
workspace (`personalWorkspaceIdFor(identity.id)`), tools come from the
cross-workspace aggregator (`Runtime.getToolListAggregator()` — the same
instance T007 consumes), and each tool call routes through the
orchestrator (`routeToolCall`) which derives a fresh `WorkspaceContext`
from the parsed namespace. T001's cross-workspace E2E is unblocked: the
5 contract cases plus the self-check pass.
Files modified:
src/runtime/types.ts — drop `workspaceId` from
`ChatRequest` and `ChatResult`. Doc
comments explain the new
identity-bound contract +
per-tool-call attribution on each
`tool.done` event.
src/runtime/runtime.ts — major refactor:
- `_chatInner` reworked to hard-error
on missing identity when an
identity provider is configured,
fall back to `DEV_IDENTITY` in
dev mode (no `?? "anonymous"`
sentinel), and derive the session
workspace from
`personalWorkspaceIdFor(...)`.
- `ensureUserWorkspace` is called
before `ensureWorkspaceRegistry`
so embedded/CLI/test paths
auto-provision the personal
workspace, matching the production
login posture.
- `_buildIdentityToolRouter` builds
a `ToolRouter` whose
`availableTools()` returns the
aggregated cross-workspace list
(via the runtime's single
aggregator instance) and whose
`execute(call, ...)` routes via
`routeToolCall` → the bare tool
name after stripping the
`<source>__` prefix (mirroring
T007's `/mcp` dispatch contract).
- `_wrapSinkWithWorkspaceAttribution`
stamps `workspaceId` on
`tool.progress` / `tool.done`
events from a per-call dispatch
map, satisfying the audit
attribution criterion.
- Orchestrator errors map to
distinct `isError: true` tool
results with structured
`reason` values matching T007:
`invalid_tool_name`,
`unknown_workspace`,
`workspace_access_denied`,
`unknown_tool_source`.
- `loadBundleSkills` iterates over
every workspace the identity can
see so bundle-published
`skill://<name>/usage` resources
reach Layer 3 cross-workspace.
- `focusedApp` resolution now
searches every accessible
workspace's registry, not just
the request's.
- The aggregator is constructed
eagerly in `Runtime.start()`
(single instance shared with
T007's `/mcp` path) and disposed
in `shutdown()` — Group B audit
closeout follow-up.
src/runtime/tools.ts — `matchToolPattern` accepts both the
full namespaced (`ws_<id>/...`)
and the bare-inner form so legacy
`allowedTools` patterns from skill
manifests / chat composer keep
working unchanged.
src/skills/select.ts — same namespace-aware match in
`toolMatches` for the
`applies_to_tools` glob path —
skills with `appliesToTools:
["source__*"]` still light up
correctly under namespaced tool
names.
src/api/handlers.ts — `/v1/chat` body parser strips
`workspaceId` from the resulting
`ChatRequest` (accept-but-ignore on
the chat path), and chat-multipart
ingest writes to the identity's
personal workspace file store so
the chat reads from the same
location. Cross-workspace ingest
remains a Stage 6 concern.
src/api/routes/chat.ts — `/v1/chat` switches from
`requireWorkspace` to
`optionalWorkspace` middleware:
the header still validates against
membership when sent (400/403 on
malformed / cross-tenant) but the
route handler discards the value.
src/api/schemas/rest.ts — `ChatRequestBody.workspaceId`
description updated to flag it as
deprecated + accept-but-ignore.
test/helpers/two-workspace-fixture.ts
— fixture migrated to use
`namespacedToolName(wsId, name)`
from `src/tools/namespace.ts`
(Group A audit's reconciliation
criterion, option (a)).
`buildChatRequest` drops
`workspaceId`.
Tests modified:
test/integration/cross-workspace-chat.test.ts
— flip `SKIP_UNTIL_T006` to `false`.
5 contract cases + 1 self-check
pass.
test/integration/runtime/chat-identity-guard.test.ts
— rewrite to assert the new
identity-only guard (identity
required when auth provider
configured; DEV_IDENTITY fallback
in dev mode). The old
`workspaceId is required` cases
are deleted.
test/integration/runtime/workspace-conversations.test.ts
— rewrite to assert the
identity-bound conversation layout
(top-level path, ownerId on the
metadata, session workspace =
personal workspace breadcrumb).
test/integration/runtime/concurrent-chat.test.ts
— release-on-throw case now provokes
the throw via a foreign-owner
resume (`ConversationAccessDenied`)
instead of missing `workspaceId`.
test/integration/identity-wiring.test.ts
— `Chat with workspace context` and
`Chat without workspace
(workspaceId is now required)`
replaced with two cases asserting
the identity-bound contract.
test/integration/stage-1-cross-workspace-conversation.test.ts
— metadata `workspaceId` is now the
session (personal) workspace, not
the `X-Workspace-Id` of the
request — assertions updated.
test/integration/skills-read-tools.test.ts
— skill is pre-staged in the
identity's personal workspace dir
so the chat's session-workspace
read finds it. Read tools are now
called against the personal
workspace registry.
test/integration/chat-file-visibility.test.ts
— `X-Workspace-Id` on file-tool
calls switched to the personal
workspace (where chat-multipart
ingest now lands).
test/integration/files-mcp-resources.test.ts
— same: read resources from the
personal workspace.
Tests added:
test/integration/runtime/t006-acceptance.test.ts
— 6 tests pinning the two
audit-added acceptance criteria:
- aggregator's
`activeWatcherCount() === 0`
after `runtime.shutdown()`
- idempotent shutdown
- each of the four orchestrator
errors (`UnknownNamespacedToolName`,
`UnknownWorkspace`,
`WorkspaceAccessDenied`,
`UnknownToolSource`) surfaces a
distinct `isError: true` tool
result via `runtime.chat()`.
Schema cut + hard-error on legacy disk records for the deleted
`BundleRef.oauthScope: "user"` literal. The Stage 2 deploy runbook
(`nimblebrain-ops/research/delegation-model/STAGE_2_DEPLOY.md`) is the
operator contract: stop the platform → run `bun run migrate:user-creds`
→ deploy. The runtime is strict — no boot-time normalization, no in-
process re-implementations of T003, no backwards-compat shims. A
skipped migration is operator error and surfaces as a loud boot
failure naming the offending record.
Removed surface area (delete-don't-deprecate per Group A audit):
- src/users/user-connector-store.ts — DELETED. The class was kept
"to detect and migrate legacy user.json files on subsequent boots."
That is the T003 migration re-implemented in the binary. Gone.
- src/runtime/runtime.ts — boot-time migration block deleted (the
walk over `users/<userId>/user.json` that wrote into personal
workspaces). The `_userConnectorStore` field + `getUserConnectorStore`
method gone too. Stale comment referencing `oauthScope: "user"`
bundles in `getAllowInsecureRemotes` doc refreshed.
- src/bundles/lifecycle.ts — `normalizeBundleRefAtLoad` and
`LegacyUserScopeWithoutOwnerError` REPLACED with
`assertBundleRefIsPostStage2` + `LegacyOAuthScopeError`. No
translation. The new helper throws on encounter, naming the
bundle's serverName + url and pointing operators at
`bun run migrate:user-creds` and the deploy runbook. Removed the
unused `personalWorkspaceIdFor` import. `seedInstance`'s doc
refreshed to point at the disk-read boundary, not the deleted
normalization helper.
- src/runtime/workspace-runtime.ts — `buildProcessInventory` is now
the disk-read boundary: every ref goes through
`assertBundleRefIsPostStage2` before reaching the inventory.
- src/engine/types.ts — `ToolSource.execute`'s `principalId?` arg
REMOVED from the interface (no in-tree consumer existed; identity
flows via AsyncLocalStorage / `runWithRequestContext`).
`EngineConfig.principalId` field deleted too.
- src/tools/types.ts, src/tools/registry.ts, src/tools/mcp-source.ts,
src/tools/upjack-source.ts — `principalId` arg removed from every
implementation and the SharedSourceRef wrapper.
- src/engine/engine.ts — call site no longer passes `config.principalId`.
- src/runtime/runtime.ts — `_buildIdentityToolRouter`'s execute signature
drops the unused `principalId` arg; the chat-config builder no
longer threads `principalId` into the engine config.
- src/api/handlers.ts — REST `/v1/tools/call` path no longer extracts
and forwards a `principalId`; identity is already in the
AsyncLocalStorage `reqCtx`.
- src/api/types.ts, src/api/server.ts — `RouteContext.userConnectorStore`
field deleted from the `AppContext` and the construction site.
- src/bundles/types.ts — deletion-comment tombstones referencing
the deleted `"user"` literal removed from BundleRef.oauthScope and
BundleInstance.oauthScope doc blocks; the doc states what
`"workspace"` means today.
Catalog field rename — option (a) `defaultScope` → `defaultBinding`:
- src/connectors/catalog.yaml — every entry renamed; values
`workspace` (active workspace) / `personal` (caller's personal
workspace). 19 entries touched.
- src/connectors/server-detail.ts — `NimbleBrainConnectorMeta.defaultScope`
renamed to `defaultBinding: "workspace" | "personal"`; doc states
it's a UX hint that selects the wsId, NOT a per-ref `oauthScope`
literal.
- src/registries/types.ts — `DirectoryEntry.defaultBinding`.
- src/registries/projection.ts — `ConnectorCatalogEntry.defaultBinding`;
both projection functions thread the new field.
- src/tools/connector-tools.ts — every dispatch site reads
`defaultBinding`; composio gate compares against `"workspace"`.
- src/tools/system-tools.ts — doc refreshed.
- web/src/api/client.ts — both projected interfaces renamed.
- web/src/components/connectors/{OperatorOAuthSection,ConnectorStatusHero}.tsx
— DirectoryEntry construction sites use `defaultBinding`.
- web/src/pages/settings/ConnectorBrowsePage.tsx — page-scope `"user"`
/ `"workspace"` (UI mode) is mapped to `"personal"` / `"workspace"`
(binding) at the filter boundary inside the memo.
Why (a) not (b): catalog entries inherently know their install
target (Granola = always personal, Slack = workspace shared). Removing
the field would force the UI to maintain that knowledge separately —
worse architecture. The rename gives the field a name that means what
it does (selects the binding workspace) without referencing the
deleted oauthScope literal.
Tests:
- test/unit/bundles/legacy-oauth-scope-rejected.test.ts — NEW. 6
cases pinning the throw-on-load contract: URL bundle with legacy
`oauthScope: "user"` throws `LegacyOAuthScopeError`; error carries
serverName + url + operator-actionable message naming the migration
command and deploy runbook; clean URL bundles + non-URL refs
pass through; missing serverName falls back to `"(unknown)"`.
- test/integration/legacy-oauth-scope-boot-error.test.ts — NEW. 1
case pinning the disk-read boundary: a `workspace.json` with a
legacy bundle ref causes `Runtime.start()` to throw
`LegacyOAuthScopeError` at `buildProcessInventory` — the operator
sees a hard boot failure with the offending record named.
- test/unit/bundles/normalize-legacy-oauth-scope.test.ts — DELETED.
Was guarding the deleted normalization helper.
- test/integration/oauth-scope-user-loader-migration.test.ts — DELETED.
Was exercising the deleted boot-time loader migration.
- test/unit/connector-tools.test.ts — `defaultScope` → `defaultBinding`
in every helper + test case (3 catalog fixtures, 1 test name
rewrite). Mock harness drops `getUserConnectorStore`.
- test/unit/connector-tools-composio-install.test.ts — mock harness
drops `getUserConnectorStore`.
- test/unit/composio-auth.test.ts, test/unit/connector-directory.test.ts,
test/unit/directory-entry-projection.test.ts,
test/unit/connectors-catalog.test.ts — `defaultScope`/`defaultBinding`
rename through fixtures and assertions.
- web/src/__tests__/connector-sections.test.tsx — same rename.
Verification: `bun run verify` is clean — 3059 unit + 258 web-unit +
605 integration + 17 smoke, 0 fail, 12 skip. Cross-workspace E2E
(`cross-workspace-chat.test.ts`, `ambient-context-cross-workspace.test.ts`)
8/8. T007's `mcp-server-identity-bound.test.ts` + `mcp-server-endpoint.test.ts`
18/18. Greps (per the cleanup task spec): zero matches for
`normalizeBundleRefAtLoad`, `userConnectorStore`/`UserConnectorStore`,
`principalId\??:` in `src/engine/`+`src/tools/`, `"user"` in
`src/bundles/types.ts`, `defaultScope: user` in `src/bundles/` +
`src/connectors/`.
Operator action (unchanged from prior commit): run
`bun run migrate:user-creds` before deploying Stage 2. The runbook at
`nimblebrain-ops/research/delegation-model/STAGE_2_DEPLOY.md` is the
operator contract; the runtime no longer migrates or normalizes at
boot. T012's CHANGELOG entry carries the migration ordering.
…t, narrow scope
Stage 2 Group E, Task 009. Implements three things the locked Q1 + Q3
design decisions (2026-05-22) and the Group D audit call for:
1. Header workspace switcher (`WorkspaceSelector` — was historically the
"header switcher"; the actual file in this codebase) is DELETED
entirely. T013 fills the slot in the sidebar with the workspace+apps
navigator; T009 leaves the slot empty so the shell never briefly
hosts two workspace surfaces.
2. Q3 bridge-lifecycle unwiring. `setActiveWorkspaceId` no longer fires
the `resetMcpBridgeClient` lifecycle handler — the `/mcp` session is
identity-bound, so workspace switches reuse the same session and
carry context via the per-request `X-Workspace-Id` header. The setter
itself is preserved (T013's sidebar will call it). `setAuthToken`
continues to fire `resetMcpBridgeClient` on logout (identity boundary).
Both setters keep their equality guards.
3. Dead-union cleanup per the Group D audit ("no bandaids" directive).
`InstalledConnector.scope: "workspace" | "user"` narrowed to
`"workspace"` on both sides of the boundary — the `"user"` arm was
dead surface after T008 (every population site emits `"workspace"`
only). Three downstream return shapes (disconnect / uninstall /
install / list_tools_with_permissions / set_permissions) narrowed
the same way; their `scope` arg parameters tightened to `"workspace"`.
4. UI prop rename per the Group D audit: the prop `scope: "user" |
"workspace"` on 4 components is a page/route mode indicator (which
settings page is rendering), not an oauthScope. Renamed to
`mode: "personal" | "workspace"` across ConnectorBrowsePage,
ConnectorDetailPage, ConnectorList, ToolPermissionsTable, and every
call site. The REST helpers underneath always read/write
workspace-scope permissions; the `mode` is purely UI.
Files
-----
src/tools/connector-tools.ts — narrow `InstalledEntry.scope` to
`"workspace"` literal (the `"user"` arm is dead surface; every
population site already emits `"workspace"`).
web/src/api/client.ts — split lifecycle hook contract:
`setAuthToken` fires `onAuthLifecycleChange` on real change;
`setActiveWorkspaceId` no longer does (Q3). Both keep equality
guards. `InstalledConnector.scope` + every helper return / arg type
narrowed to `"workspace"`.
web/src/mcp-bridge-client.ts — doc comment updated to match the new
lifecycle contract. No behavior change to the function itself; the
`setAuthLifecycleHandler(resetMcpBridgeClient)` registration is
unchanged and now only fires on auth-token transitions.
web/src/components/WorkspaceSelector.tsx — DELETED.
web/src/components/ShellLayout.tsx — drop the `WorkspaceSelector`
import and the two JSX call sites (desktop + mobile). Empty
comment-anchored slot remains for T013 to fill.
web/src/components/UserMenu.tsx — comments updated to remove stale
references to the deleted component.
web/src/pages/settings/ConnectorBrowsePage.tsx
web/src/pages/settings/ConnectorDetailPage.tsx
web/src/components/connectors/ConnectorList.tsx
web/src/components/connectors/ToolPermissionsTable.tsx — prop renamed
from `scope: "user" | "workspace"` to `mode: "personal" |
"workspace"`. Internal logic now maps `mode` directly to catalog
entry's `defaultBinding` (the legacy "user → personal" mapping
collapses to identity). REST helper calls underneath always pass
`"workspace"` (Stage 2: personal connectors live in the user's
personal workspace).
web/src/App.tsx — `<ConnectorBrowsePage scope="workspace" />` →
`mode="workspace"`. Same for `ConnectorDetailPage`.
web/src/pages/settings/WorkspaceConnectorsTab.tsx — `scope="workspace"`
→ `mode="workspace"` on `<ConnectorList>`.
AGENTS.md — `## MCP App Bridge Rules` bullet on lifecycle handler
updated: `setAuthToken` fires the handler; `setActiveWorkspaceId`
does not. Same content as the doc comment in api/client.ts.
Tests
-----
web/src/__tests__/a-t009-acceptance.test.ts — NEW. Pins the
`ChatRequest` wire shape (mutual-extends type guard catches future
widening or narrowing), confirms `setActiveWorkspaceId` is still a
callable export (T013 dependency), and walks `web/src/` to assert no
source file imports or names `WorkspaceSwitcher` / `WorkspaceSelector`.
Filename is `a-` prefixed to load early — `connector-sections.test.tsx`
later in the suite installs a `mock.module("../api/client", ...)`
stub that would otherwise blow up any file importing additional
exports from api/client. Same pattern `api-client-lifecycle.test.ts`
uses.
web/src/__tests__/api-client-lifecycle.test.ts — flipped the
`setActiveWorkspaceId fires the handler` test to "does NOT fire
(Q3)". The equality-guard tests for both setters remain; the
contract narrative at the top of the file is updated to reflect the
split lifecycle.
web/src/__tests__/mcp-bridge-client.test.ts — added three tests under
a new `bridge session lifecycle vs auth/workspace setters` describe:
(1) workspace switch reuses the same bridge client (Q3
regression test — required by the task spec).
(2) logout (`setAuthToken(null)`) drops the bridge client
(Q3 boundary — auth-token branch must remain wired).
(3) next bridge fetch after switch carries the new
`X-Workspace-Id` header (topology guard for T013's plumbing).
Verification
------------
bun run check OK
bun run check:web OK
bun run lint OK
bun run check:cycles OK
bun run check:tool-namespace OK
bun run check:personal-workspace-id OK
bun run check:codegen OK
bun run verify:static OK
bun run test:unit 3059 pass / 0 fail
bun run test:web 264 pass / 0 fail
bun run test:integration 605 pass / 0 fail / 12 skip
…d wsId
Replaces the connector install "scope" dropdown with an explicit
WorkspaceTargetPicker dialog and tightens `manage_connectors.install`
to require an explicit `wsId` argument with no default-to-personal
fallback inside the tool. Honors the no-bandaids directive (Mat's
T008 closeout note): the tool refuses if `wsId` is missing; the UI
picks the target. The legacy `oauthScope: "user"` literal — already
gone after T008 — stays gone.
The dialog presents every workspace the caller has admin role in
(filtering member-only rows, including the personal workspace which
is always sole-owner-by-design per Stage 1 invariants). Personal-
typical connectors (`defaultBinding: "personal"`) preselect the
personal workspace. Installing into any non-personal workspace
requires the user to type the workspace's display name (case-
insensitive after trimming — friction, not key-entry precision) to
enable Install. Closing and reopening the dialog clears the typed-
confirmation and selection so a stale "Helix" can't satisfy the gate
for a different workspace.
Server side: `manage_connectors.install` now takes a top-level `wsId`
arg in the input schema. `handleInstall` hard-errors on missing /
empty `wsId`, validates the workspace exists, gates on admin role,
checks `connectorsAllowList`, then dispatches. The previous two-
branch flow (workspace vs personal, keyed on `entry.defaultBinding`)
collapsed into one — `defaultBinding` is now only a UX preselection
hint the picker may consult, never read by the tool. The composio
"shared-only" restriction now reads `ws.isPersonal` (source of
truth) instead of `entry.defaultBinding` (UX hint). Personal-
workspace ids stay sourced from `personalWorkspaceIdFor` —
`check:personal-workspace-id` lint passes.
Files:
- src/tools/connector-tools.ts — install action schema gains `wsId`;
`handleInstall` requires it (hard-error, Stage 1 precedent); two-
branch dispatcher collapsed into one parameterized by the picked
wsId; composio personal-target gate now inspects `ws.isPersonal`.
- src/tools/workspace-mgmt-tools.ts — `manage_workspaces.list`
surfaces `isPersonal` per workspace so the bridge-list fallback
can preselect the personal workspace.
- web/src/api/client.ts — `installConnector(entry, wsId)` — wsId
required; new return includes `wsId` for caller-side audit.
- web/src/context/WorkspaceContext.tsx — `WorkspaceInfo` gains
`isPersonal?: boolean`.
- web/src/lib/bootstrap.ts — both mappers propagate `isPersonal`
from bootstrap and `manage_workspaces.list` contracts.
- web/src/components/connectors/WorkspaceTargetPicker.tsx (CREATE) —
radiogroup of installable workspaces, controlled component;
`workspacesEligibleForInstall` filters to admin-role rows.
- web/src/components/connectors/InstallConnectorDialog.tsx (CREATE)
— dialog with preselection + typed-confirmation gate + state
reset on close. Exports `preselectWorkspaceId` +
`typedConfirmationMatches` for unit-test coverage.
- web/src/pages/settings/ConnectorBrowsePage.tsx — onInstall opens
the dialog instead of immediately calling `installConnector`;
the dialog hands back `{serverName, wsId}` and the page routes
to OAuth initiate / Configure as before.
- test/unit/connector-tools.test.ts — install tests tightened to
pass `wsId` explicitly; new adversarial coverage for missing /
empty / unknown wsId; personal-workspace test uses
`personalWorkspaceIdFor` helper + pins `oauthScope: "workspace"`
on the persisted ref; audit-attribution test pins "picked wsId
reaches the dispatcher, not the session header".
- test/unit/connector-tools-composio-install.test.ts — every
install call site updated to pass `wsId: h.wsId`.
- test/integration/connector-tools-install.test.ts (CREATE) — pins
persisted on-disk shape (`oauthScope: "workspace"`, `wsId:
<picked>`, no legacy `"user"` literal anywhere), personal install
records `wsId === personalWorkspaceIdFor(userId)`, hard-error +
zero on-disk writes when `wsId` is missing, and audit attribution
(install lands in picked wsId, not session header).
- web/src/__tests__/WorkspaceTargetPicker.test.tsx (CREATE) — picker
is fully controlled; admin-role filter hides member-only rows;
Personal/Shared badge contract.
- web/src/__tests__/InstallConnectorDialog.test.tsx (CREATE) —
preselection heuristic, typed-confirmation gate (case-insensitive
decision documented), install call uses picked wsId (no ambient
leak), and the adversarial state-reset-on-close walk (open for
Helix + type "Helix" → close → reopen for Acme → typed "Helix"
still disables Install; only "Acme" unlocks).
Verification:
- bun run check / check:web / lint / check:personal-workspace-id /
check:tool-namespace — all clean.
- bun run test:unit — 3062/3062 pass.
- bun run test:web — 276/276 pass (incl. 7 picker + 11 dialog).
- bun run test:integration — 605/605 pass.
- cross-workspace-chat: 6/6 (load-bearing E2E unchanged).
- mcp-server-identity-bound: 8/8 (load-bearing E2E unchanged).
No bandaids, no backwards compatibility: a buggy client that omits
wsId hard-errors with a message naming the missing arg; the tool
never silently picks a target. Pre-T010 single-step install via
`installConnector(entry)` no longer compiles — every caller must
pass a workspace id picked by the UI.
…crumbs + tool-call provenance
The user-visible payoff of Stage 2 — the sidebar mockup Mat locked Q1
against (2026-05-22). Replaces the deleted header switcher (T009) with
a sidebar WORKSPACES section, adds a composer footer with viewing +
TOOLS FROM breadcrumbs, and renders namespaced tool calls as friendly
`tool · Workspace` rows in the transcript.
Sidebar navigator (new)
- web/src/components/shell/Sidebar.tsx (new) — spec-named entry point;
re-exports SidebarWorkspaceNav so ShellLayout has one mount point.
- web/src/components/shell/SidebarWorkspaceNav.tsx (new) — WORKSPACES
heading + `+` affordance (routes to the existing
/settings/org/workspaces create flow — no new UX invented). Reads
workspaces from useWorkspaceContext; orders personal-first then
shared alphabetical via lib/workspace-order. Active workspace
auto-expands; last-viewed app restoration on mount.
- web/src/components/shell/WorkspaceRow.tsx (new) — workspace row with
expand chevron + role badge. Per-row expansion state owned by the
parent so toggling one row doesn't collapse another.
- web/src/components/shell/WorkspaceAppList.tsx (new) — indented app
list. Click handler:
1. Equality-guards setActiveWorkspace at the React layer
(double-guards api/client's setActiveWorkspaceId — T009
invariant). Cross-workspace click fires the setter once;
re-click on the same app is a no-op.
2. Persists `<wsId>:<appRoute>` to localStorage key
`nb:last-viewed-app` so a reload restores the selection.
3. Navigates to `/w/<slug>/app/<bundleName>` (existing convention).
Composer footer + tool provenance (new)
- web/src/components/chat/ComposerFooter.tsx (new) — two-line footer.
- Viewing line: "You're viewing <app> in <workspace>" derived from
URL + WorkspaceContext + ShellContext placement label. Falls back
to "Not viewing an app" when no app matches.
- TOOLS FROM line: badges driven by useToolWorkspaces (NOT by the
active workspace). Pinning to active would pass single-workspace
tests and silently break the multi-workspace case the spec
explicitly asserts.
- web/src/components/chat/Composer.tsx (new) — wraps MessageInput +
ComposerFooter. `hideFooter` prop for compact/popover compositions.
- web/src/components/chat/ToolCallProvenance.tsx (new) — renders
`<friendly-tool> · <workspace-badge> [status-pill]`. Parses through
parseNamespacedToolName (no .split). Q2 fallback: missing workspace
in user's list renders raw `ws_<id>/<tool>` — never falls back to
personal-workspace display name. Personal workspace gets a fixed
badge variant so it's visually distinct across sidebar + provenance
+ composer footer.
- web/src/context/ToolWorkspacesContext.tsx (new) — separate context
for the TOOLS FROM set so tests can stub multi-workspace + watcher-
invalidation topology independently of identity. Default derives
from useWorkspaceContext.
Primitives + helpers (new)
- web/src/lib/namespaced-tool.ts (new) — web-side mirror of
src/tools/namespace.ts::parseNamespacedToolName. Web cannot import
from `src/` (tsconfig + vite alias scope), so we keep a separate
primitive with identical contract. Differs only in error handling:
returns null instead of throwing (transcript renderer degrades to
raw rather than crash on a removed workspace).
- web/src/lib/workspace-order.ts (new) — orderWorkspacesForSidebar.
Personal-first then shared alphabetical (case-insensitive),
id-stable tie-break.
Integration
- web/src/components/ShellLayout.tsx (modified) — mounts
WorkspaceNavSidebar in both the desktop sidebar and the mobile
drawer. Removes the "left empty by T009" placeholder comments.
- web/src/components/ChatPanel.tsx (modified) — swaps MessageInput
for Composer. Compact (popover/embedded) ChatPanel hides the
footer (`hideFooter={compact}`) — the breadcrumb strip is sized
for the full chat surface, not the embedded composer.
Tests (new)
- web/test/namespaced-tool.test.ts — parser contract (9 tests):
well-formed parse, first `/` as separator, raw fallback (not
fail-loud) for missing separator / empty components / invalid
wsId. Pins the Q2 invariant against silent-fallback regressions.
- web/test/workspace-order.test.ts — ordering rule (5 tests):
personal-first, case-insensitive alphabetical, id-stable
tie-break, legacy `isPersonal=undefined` treated as shared, no
input mutation.
- web/test/ToolCallProvenance.test.tsx — provenance contract (8
tests): friendly name + workspace badge, prefix stripping (`gmail__`
→ `send_message`), Q2 fallback to raw on missing workspace
(adversarial — Personal name MUST NOT bleed in), non-namespaced
input verbatim, status pills, personal-workspace variant pin, and
an audit grep that fails if any new component contains
`.split("/")`.
- web/test/ComposerFooter.test.tsx — footer contract (7 tests):
multi-workspace TOOLS FROM (the load-bearing adversarial topology
pin), single-workspace + empty cases, watcher-add re-render with
three badges, viewing-line "no app" fallback, app+workspace
rendering, switch-workspace remount updates both lines.
- web/src/__tests__/b-t013-sidebar-nav.test.tsx — sidebar contract (8
tests). Prefixed `b-` to load after `a-t009-acceptance` and before
`connector-sections` (which installs a partial mock.module on
api/client). Mocks api/client minimally with an equality-guarded
spy mirroring the production setter. Pins: ordering + role badges,
`+` affordance present, per-row expansion (toggle does not affect
other rows), the equality-guard topology test (cross-workspace
click fires setter once, re-click is no-op), navigation contract
(`/w/<slug>/app/<route>`), persistence (write + restore on
remount).
Audit
- No `as unknown as T` casts in new components.
- No `?? "ws_<id>"` fallbacks anywhere.
- No raw `ws_<X>/<Y>` substitution-headed template literals in the
new component dirs.
- No `.split("/")` on tool-name bindings in new components (audit
test enforces).
- No `ChatContext` consumption from shell components.
- No new REST routes — `+` affordance reuses existing
/settings/org/workspaces flow; workspace+bundle data sourced from
the existing WorkspaceContext bootstrap (which already routes
through manage_workspaces.list per CLAUDE.md § API Surfaces:
prefer tool actions over new routes).
- No conversation schema change — Q1 follow-up "anchorAppId /
anchorWorkspaceId" deferred to Stage 4+ per task spec.
Verification
- bun run check / check:web / lint / format:check: clean.
- bun run check:cycles / check:tool-namespace /
check:personal-workspace-id / check:codegen / check:code-style:
all green.
- bun run test:unit: 3062 pass / 0 fail.
- bun run test:web: 319 pass / 0 fail.
- bun run test:integration: 609 pass / 12 skip / 0 fail.
- Stage 2 invariant suites still green: cross-workspace-chat 6/6,
mcp-server-identity-bound 8/8, t006-acceptance 6/6,
ambient-context-cross-workspace 2/2, connector-tools-install 4/4.
Proves SPEC_REFERENCE.md § Verification line 3 ("External MCP test:
Claude Desktop sees aggregated tool list, can invoke cross-workspace")
through the real `/mcp` Streamable HTTP transport — the same protocol
surface Claude Desktop / Claude Code / Cursor use.
test/smoke/external-mcp-cross-workspace.test.ts — new smoke-tier file.
Constructs the MCP SDK `Client` with `StreamableHTTPClientTransport`
against a `startServer()`-backed `/mcp` endpoint and pins 7 failure
modes (one per case): no-X-Workspace-Id init, aggregated tools/list,
cross-workspace dispatch (per-source counters at 1/1), session
survives without a switcher, un-namespaced strict invariant rejects
with -32602 invalid_tool_name and does NOT silently route anywhere,
audit attribution via RequestContext.workspaceId (read directly from
handler-execution, NOT from a server envelope — Stage 1 lesson 2),
and the T008 connector-from-personal carryover proof.
test/helpers/two-workspace-fixture.ts — extended. Adds an
`auditTrail()` / `resetAuditTrail()` probe per workspace handle that
records the `RequestContext.workspaceId` value the counter source's
handler saw at execution time. Independent observation channel from
`callCount()`: a misattribution that still routes to the right source
(e.g. session-default workspace id stamped onto a correctly-routed
dispatch) is invisible to the counter but a smoking gun in the audit
trail. The probe is fixture-level so both the existing cross-workspace
chat E2E and the new external-MCP smoke can use it without
duplication.
Smoke tier per CLAUDE.md § Testing classification: real subprocess +
network. Runs under `bun run smoke`, excluded from `bun run test`.
No flake retries, no setTimeout workarounds — green on first run.
…NTS/CHANGELOG
Closes Stage 2 of the delegation-model refactor with the same kind of
structural enforcement Stage 1 ended with, plus the bandaid cleanup
Mat's no-backwards-compatibility directive surfaced.
Part 1 — credential-path AST lint
- scripts/check-credential-paths.ts: AST-scans src/**/*.ts for
`join(..., "users", X, "credentials", ...)`, equivalent template
literals, and string-literal substrings. Modeled on
check-conversation-paths.ts / check-personal-workspace-id.ts —
same allow-marker convention (// lint-ok:credential-path), same
allowed-files mechanism (empty by design; T008 already deleted the
pre-Stage-2 consumers).
- test/unit/scripts/check-credential-paths.test.ts: 13 unit tests
covering positive + negative fixtures and clean-tree subprocess
invocation. Same shape as the Stage 1 predicate tests.
- package.json: check:credential-paths script + verify:static wiring.
Stage 2 closeout brings the count of delegation-model structural
lints to four (conversation-paths, personal-workspace-id,
tool-namespace, credential-paths).
Part 2 — web-side WORKSPACE_ID_RE divergence fix
- Pre-Stage-2 the web parser used /^ws_[a-zA-Z0-9_-]+$/ while the
server used /^ws_[a-z0-9_]{1,64}$/i. Web was strictly more permissive
— server-produced names always parsed, but a future server-side
tightening wouldn't have propagated.
- src/workspace/workspace-id-pattern.ts: new shared module exporting
WORKSPACE_ID_PATTERN (literal string), WORKSPACE_ID_FLAGS (literal
flags), and the compiled WORKSPACE_ID_RE. src/workspace/workspace-store.ts
re-exports the regex so existing call sites keep working.
- scripts/codegen-web-platform-schemas.ts: emits
web/src/_generated/workspace-id-pattern.ts from the server source.
Web cannot import from src/ (tsconfig + vite alias both scoped to
./src), so codegen is the only mechanism that keeps the literal in
lockstep. CI's check:codegen catches drift.
- web/src/lib/namespaced-tool.ts: imports the generated literal and
builds its own RegExp locally.
- web/test/namespaced-tool.test.ts: regex-equality contract test —
pins WORKSPACE_ID_PATTERN / WORKSPACE_ID_FLAGS against the literal
embedded in the source, asserts the constructed RegExp's .source /
.flags match, and pins rejection of hyphens / no-prefix / path
traversal / length-overflow shapes.
Part 3 — dead-union audit grep
Ran `grep -rnE '"workspace"\s*\|\s*"user"|"user"\s*\|\s*"workspace"' src/`.
Hits: only src/tools/platform/skills.ts (layered-skill scope
"org"|"workspace"|"user"|"bundle") and src/permissions/permission-store.ts
(PermissionOwner discriminator). Both confirmed-not-dead by Group E
audit — separate concepts (layered skills, permissions storage layer).
Zero new narrowings. Documented as the audit signal per spec.
Part 4 — AGENTS.md updates
- Workspace Isolation section: credentials now live in
workspaces/<wsId>/credentials/..., not users/<userId>/credentials/...
- New "Cross-workspace tool namespacing (Stage 2)" subsection:
ws_<id>/<tool> canonical, namespacedToolName / parseNamespacedToolName
primitives, check:tool-namespace lint enforces, web tier mirrored via
codegen, BundleRef.oauthScope: "user" deletion + LegacyOAuthScopeError,
dev-mode parity noted.
- New "Stage 2 follow-ups — tenant migration order" subsection: cross-
links to nimblebrain-ops/research/delegation-model/STAGE_2_DEPLOY.md.
- CLAUDE.md symlink intact (verified `readlink CLAUDE.md` → AGENTS.md).
- MCP App Bridge Rules section already updated by T009 for Q3 — left
alone.
Part 5 — CHANGELOG.md Stage 2 entry
Added Highlight + Added + Changed + Breaking + Removed bullets covering
every audit-mandated entry (cross-workspace dispatch, identity-bound
sessions, sidebar navigator, oauthScope deletion, no-bandaids stance,
installConnector / ToolSource.execute / ChatRequest / ChatResult /
SessionMeta signature changes, WorkspaceSelector / UserConnectorStore /
UserPoolSource removal). Required operator action line:
"run `bun run migrate:user-creds` BEFORE deploying" with cross-link to
STAGE_2_DEPLOY.md, plus the smoke-test gating reference.
Stage 2 word count is over the 250-350 target (~460 words) because the
audit criteria mandate 23+ specific bullets; further trimming would
drop required items.
Verification
- bun run verify: green end-to-end (verify:static + test:unit +
test:web + test:integration + smoke).
- 3075 unit tests pass (+13 new from check-credential-paths.test.ts).
- 322 web tests pass (+3 new regex-equality contract tests).
- 6/6 cross-workspace-chat integration tests; 8/8 mcp-server-identity-
bound integration tests; 24/24 smoke tests.
- All four delegation-model lints clean against src/.
Audit criteria
- Lint script structure matches Stage 1 templates exactly (allowlist,
// lint-ok: marker, AST predicates exported for unit-test exercise,
import.meta.main gating).
- CHANGELOG names migrate-user-creds-to-personal-workspace.ts and
references STAGE_2_DEPLOY.md.
- AGENTS.md is the source, CLAUDE.md is the symlink.
- Three Stage 2 lints (tool-namespace, credential-paths, conversation-
paths, personal-workspace-id) all in verify:static.
- No `as unknown as T` casts in check-credential-paths.ts.
- Dev-mode parity noted in AGENTS.md.
…weep
The cross-workspace separator moved from `/` to `-` (LLM provider tool-name
validators reject `/` against ^[a-zA-Z0-9_-]{1,128}$). Three read-side
sites that parse/detect namespaced names by hand were missed by that
sweep, and `check:tool-namespace` didn't catch them (they used
`indexOf("/")`, not `.split("/")`):
- surfaceTools: detected system tools via `name.startsWith("nb__")`,
but Stage 2 namespaces them to `ws_<id>-nb__<tool>`. In Tier 2
(tool union > maxDirect) the direct list IS the system-tool list,
so it came back EMPTY — the model got zero tools and hallucinated
tool calls as plain text instead of invoking anything.
- skills/select toolMatches: `/`-split left bare allowedTools globs
(`crm__*`) unable to match namespaced tools, breaking tool-affined
Layer 3 skill selection.
- engine removeTool guard: `startsWith("nb__")` no longer recognized
namespaced system tools, so they could be released.
Fix: add `bareToolName()` to tools/namespace.ts — one non-throwing
read-side helper built on the canonical parser, so the separator and the
workspace-id boundary stay defined in exactly one place. Apply it at all
three sites.
surfaceTools/filterTools/matchToolPattern now need namespace parsing, and
the runtime↛tools layering rule (correctly) flagged the import — the
signal that these tool-name-aware shaping functions belong in the tools/
layer. Moved src/runtime/tools.ts → src/tools/surfacing.ts, which also
removed two pre-existing tools/→runtime/ imports (delegate, compose).
Adds a surfaceTools regression block exercising namespaced names through
the real builder (the gap that let this ship green: every prior case used
bare names). Fixes the bundle-skills integration test.
Iframe MCP-App tool calls arrive with bare qualified names (`<source>__<tool>`). The bridge now namespaces the wire name via `namespacedToolName(getActiveWorkspaceId(), name)` before dispatch, so a widget's tool call lands in the host workspace's bundle subprocess and audit — the Q3 contract. Returns a structured JSON-RPC error if no active workspace is in scope rather than guessing. Today the iframe is always mounted under its host workspace, so active == host; if cross-workspace iframes land later (Stage 6), capture the host at bridge construction instead.
…es, chat on every route
Workspaces are siblings of Conversations/Automations/Files (all
identity-bound), not parents — so they live in a labelled WORKSPACES
section in the sidebar, single click navigates + activates. Replaces the
T013 inline-app-list nav (the "two actions at once" row plus a data path
that lied with "No apps installed"). Deterministic avatars from the
workspace id; role in aria-label, not a pill.
New surfaces:
- GlobalHomePage at `/` — greeting + workspaces grid.
- WorkspaceOverviewPage at `/w/:slug/` — app discovery sourced from the
placements registry (single source of truth), not the empty bundles[].
Chat lifted to ShellLayout via ChatChrome, so it's one click from every
route (was only mounted by AppWithChat → invisible on /, overview,
settings). <main> pushes over with transition timing matched to the panel
slide.
Removes the obsolete T013 components (SidebarWorkspaceNav, WorkspaceRow,
WorkspaceAppList, the rail variant, HomeAppRoute, WorkspaceRedirect) and
their tests; adds b-workspace-section coverage.
…llback When bootstrap falls back from a stale active-workspace id (a workspace the user was removed from or that was deleted), the in-memory api/client state reflects the server-resolved fallback but localStorage still held the stale id — the drift surfaced as a workspace_error on reload. Keep localStorage in lockstep with the resolved workspace; guard for unavailable storage (private mode, quota).
…isted active workspace
Two sidebar items showed active at once — e.g. on Home (`/`), the
persisted active workspace's row stayed highlighted. `activeWorkspace`
is always set (it scopes cross-workspace tool dispatch), so keying the
row highlight off it lit a workspace even on global, identity-bound
routes (`/`, `/conversations`).
Derive the highlight from the URL via `matchPath("/w/:slug", { end:
false })` instead, mirroring how the core NavLinks compute active state.
A workspace row is now active only on `/w/<slug>/...`; global routes
show zero active workspace rows. Selection still sets the active
workspace for tool scoping — the two concerns are now decoupled.
Updates the active-state test to be route-driven and adds a regression
pin for the global-route (zero-active) case.
The no-inline-type-imports check globbed src/**/*.ts and walked any node_modules nested under src/ — notably src/bundles/<name>/ui/node_modules, which a local bundle-UI install creates (gitignored, absent in CI). That made the check pass in CI but fail locally with 1700+ violations in third-party .d.ts files. Skip any path with a node_modules segment; matches the script's documented "src source only" scope.
…<tool>
Foundation for identity-scoped ("personal") apps. The Stage 2 namespace
had one axis — the workspace. Generalize the leading segment to a SCOPE:
a workspace id (`ws_<id>`) or the identity sentinel `me`.
ws_acme-crm__search → { kind: "workspace", wsId: "ws_acme" }
me-conversations__search → { kind: "identity" }
`parseNamespacedToolName` now returns `{ scope, toolName }` where scope is
a discriminated union; `me` is collision-safe (a ws id must match
`^ws_[a-z0-9_]{1,64}$`, which `me` can't). Adds `identityToolName(name)`
builder + `IDENTITY_SCOPE` const, mirrored in the web parser. `me-` tools
authorize by identity, carry no workspace.
The orchestrator routes the workspace scope exactly as before; the
identity branch fails closed via `IdentityScopeNotRoutable` until the
`IdentityContext` dispatch lands (next commit, A2). No surface emits a
`me-` name yet — the aggregator still namespaces by workspace — so the
branch is unreachable in normal flow; a crafted /mcp call fails closed.
ToolCallProvenance renders identity-scoped tools without a workspace
badge. Tests updated to the scope shape + identity coverage. Behavior
for existing workspace-scoped dispatch is unchanged.
…l union Cross-workspace bug: the aggregator namespaces every tool per workspace, including the system tools — so nb__search is duplicated across every workspace the identity can see (ws_mat-nb__search, ws_user_*-nb__search, …). Each copy searched only its OWN workspace's registry. The model picked the personal workspace's copy, which can't see a CRM installed in ws_mat, and reported "no CRM tools installed" — even though synapse-crm was running there. (Confirmed in a conversation log: the call was ws_user_<id>-nb__search.) nb__search is an identity-level discovery surface. Add Runtime.listDiscoverableTools() — returns the identity's aggregated cross-workspace union when an identity is in scope, falling back to the current workspace's registry otherwise. Point scope:tools at it. Results are namespaced (ws_<id>-<tool>), so the model promotes via manage_tools and dispatches through the orchestrator unchanged. This is the per-workspace mis-scoping of system tools the identity-app surface work fixes structurally (system tools → me-scoped); this is the targeted fix for the search surface.
…arch) Regression for the nb__search-finds-nothing bug. From the personal workspace's request context, listDiscoverableTools() must still surface a tool installed in another workspace (the "CRM in ws_mat" case), and the union must be identity-stable regardless of which workspace is in context. Would have failed before the fix: the old per-workspace search returned only the calling workspace's registry.
Identity counterpart to WorkspaceContext: a typed access path to per-user
data at {workDir}/users/{userId}/... (files, skills — the North Star
user-owned layout). The orchestrator will construct one when routing a
`me-<tool>` name so identity-scoped tools dispatch with identity authority
and no ambient workspace.
Deliberately credential-free (personal credentials live in the personal
workspace per Stage 2) and conversation-free (conversations are top-level,
ownership-authorized). Validates userId against path traversal + rejects
`/` so an id can't escape the users/ tree.
…inel)
Revises A1 per the design review. Two-axis model: a tool is either
workspace-replicated or a singleton. Namespace encodes only the former.
ws_<id>-<tool> → { kind: "workspace", wsId }
bare <tool> → { kind: "global" } (whole name; no prefix to strip)
The workspace prefix means "this specific workspace"; its absence means
global. Drops the `me`/IDENTITY_SCOPE sentinel + identityToolName builder
(global tools have no builder — the bare name IS the wire name). Platform
tools (`nb__*`) and identity-owned app tools (`conversations__*`) are
global singletons.
parseNamespacedToolName now returns `{ scope, toolName }` where a bare
name parses to global (the whole name); only empty input or a malformed
`ws_<id>-` prefix throws (a workspace attempt, surfaced not globalized).
Stage 2 safety preserved: a bare WORKSPACE-app name is still refused (no
silent current-workspace routing).
Orchestrator routes the global branch fail-closed via GlobalScopeNotRoutable
until W3 wires global dispatch (IdentityContext + kernel global-source
validation) — no surface emits bare global names yet. Both error mappers
(chat + /mcp) map it to reason `global_not_routable`. Web mirror + tests
updated; ToolCallProvenance renders global tools with no workspace badge.
…st degradation (review) Two review fixes, applied from a worktree to avoid colliding with the parallel W1 work in the main checkout. MF1 — delete the dead pre-Stage-2 dispatch chain in runtime.ts: the `workspaceToolRouter` (workspace-bound `execute()` with no membership check), the `engine` built from it, and the unused `_engine` constructor param + its `new Runtime(...)` arg. After T006, chat() builds its own identity-bound engine per request (`_buildIdentityToolRouter`); the constructor engine was never assigned or read. Removing it deletes the exact auth-bypass pattern from the hot file and makes the dispatch architecture read as one router, not two. S3 — `getUnionForIdentity` uses `Promise.allSettled` instead of `Promise.all`: a single workspace whose listing rejects (e.g. its registry can't be constructed) no longer nukes the identity's entire aggregated tool list. Degrade gracefully, skip the failed workspace, and log it under the `mcp` namespace — matching the per-source containment posture the surrounding code already documents.
Exercises the allSettled branch added in a536451: N workspaces where one lister rejects → the union returns exactly the healthy workspaces' tools, the failed workspace absent. Without it a refactor back to all-or-nothing (Promise.all) stays green.
- Separator `/` → `-` in docstrings that predated the late sweep: namespace.ts, route.ts, tool-list-cache.ts, tool-list-aggregator.ts, and CLAUDE.md's cross-workspace-namespacing section. - CLAUDE.md + identity/context.ts updated to the bare-global model: drop the stale `me-<tool>` sentinel; workspace tools are `ws_<id>-…`, platform + identity-owned tools are bare global singletons. - tool-list-aggregator.ts dispose comment → past tense (dispose is wired at runtime.ts shutdown, not "eventually").
…watchers, log skipped sources Addresses the QA pass on PR #272. C2 (AGENTS.md accuracy) — the namespace section overstated bare-global as shipped. Corrected to describe what ACTUALLY ships: every tool is namespaced per workspace (`ws_<id>-<source>__<tool>`, incl. the platform `nb` source); the bare/global scope is parsed but dispatch fail-closes (`GlobalScopeNotRoutable`), deferred to a later stage. (The prior doc edit went through the `CLAUDE.md` symlink and never staged the real `AGENTS.md` — fixed here by staging `AGENTS.md` directly.) C3 (finish the / → - separator sweep) — stale `ws_<id>/<tool>` doc comments the earlier sweep missed: mcp-server.ts, session-store/types.ts, runtime.ts (×3), runtime/types.ts, skills/select.ts, workspace-id-pattern.ts, bridge.ts, ToolCallProvenance.tsx, and AGENTS.md. (Left the legitimate `skill://…/usage` and path/URI splits untouched.) S1 (watcher fd leak) — `ToolListCache.invalidateIdentity` now reaps a workspace's `fs.watch` handle when its last subscriber leaves (membership change / workspace delete), instead of leaking it for the process lifetime. Lazily re-created on next listing; shared workspaces keep their watcher. Placed in invalidateIdentity (not invalidateWorkspace, which fires on every workspace.json change → would churn). Test asserts the reap via activeWatcherCount. S2 (silent source swallow) — the cross-workspace lister now emits a `log.debug("mcp", …)` when a source's `tools()` throws, so "my tool disappeared from the list" is diagnosable (gated behind NB_DEBUG=mcp).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stage 2 — identity-bound sessions + cross-workspace tool routing
Reframes the runtime around identity-bound sessions: a user's chat /
/mcpsession aggregates tools across every workspace they belong to, with per-call routing throughsrc/orchestrator/. Builds toward the delegation-model North Star (User / Workspace / Conversation / Automation as siblings).See
nimblebrain-ops/research/delegation-model/for the spec line (NORTH_STAR.md,STAGE_2_DESIGN_DECISIONS.md,IDENTITY_APP_SURFACE.md).What's in here (29 commits)
Stage 2 core (T001–T013) — the namespaced cross-workspace routing stack:
ws_<id>-<tool>namespace primitive + AST lint, user-cred migration script, thesrc/orchestrator/skeleton (per-call workspace routing), and the watcher-cached cross-workspace tool-list aggregator./mcpsessions with namespaced tool routing.oauthScope:"user", web shell teardown (deleted the workspace switcher), connector install workspace picker, sidebar workspace+apps navigator / tool-call provenance, credential-path lint + codegen.Post-T012 — live-bug fixes (both confirmed in use):
5368ef2Tools reach the model again —surfaceToolsTier-2 was returning an empty direct list because namespacednb__*tools failed a bare-prefix check (the/→-sweep missedsrc/tools/surfacing.ts).5ccec27nb__search scope:toolssearches the identity's full cross-workspace union (Runtime.listDiscoverableTools()) — was per-workspace, so a tool installed in another workspace (e.g. a CRM) was invisible. Regression test in841d72e.Post-T012 — web shell IA + bridge:
c542213WORKSPACES sidebar section + global/overview pages + chat on every route.431e013sidebar highlight follows the route.2fc13eflocalStorage sync on bootstrap fallback.4b0212abridge auto-prefixes the active workspace ontools/call.68e80e2code-style lint skips vendored node_modules.Post-T012 — identity-app-surface foundation:
a65696e→cf08bbeTool-name scope model: generalized to<scope>-<tool>, then settled on bare-global grammar (ws_<id>-<tool>= workspace; a bare name parses to{ kind: "global" }; nome-sentinel).IdentityContextprimitive added. NOTE: bare/global dispatch is parsed but not yet wired —routeToolCallfail-closes a bare name withGlobalScopeNotRoutable(deferred). Everything that ships today is workspace-namespaced.Post-T012 — review fixes:
a536451Removed the dead pre-Stage-2 workspace-bound dispatch path (workspaceToolRouter/_engine);getUnionForIdentityusesPromise.allSettledso one failing workspace doesn't nuke the union.416231dtest for that.6525859doc-drift sweep (separator + scope).d006781QA-review fixes: corrected AGENTS.md to describe shipped per-workspace namespacing (bare-global is grammar-only / W3); finished the/→-doc-comment sweep across ~10 files; reap orphanedfs.watchhandles ininvalidateIdentity(fd-leak fix + test);log.debug("mcp", …)when a source'stools()throws.Tests
bun run verify(full CI parity) green.Remaining pre-merge gates
origin/main— branch is currentlyCONFLICTING;git merge-treeshows conflicts insrc/bundles/lifecycle.tsandsrc/tools/connector-tools.ts(both heavily refactored here and by#223on main). Do this as the last step (one rebase) and re-review the conflict resolution.nb__manage_appremoval (W1) — conversational install/configure removal, in flight on a parallel branch; lands on top of this before the rebase.Known follow-ups (tracked, not blocking)
IdentityContext, Conversations as a bare identity app, root routes, Files/Automations migrations) — own PRs.instructions-reworkintegration cases (Layer-2 overlay injection incompose.ts) — pre-existing, tracked separately.nb__searchsurfaces once per workspace) — addressed when global dispatch (W3) lands.