From d1b921f66192fde8a9bc562d2561d068b9bf6cba Mon Sep 17 00:00:00 2001 From: mroops0111 Date: Mon, 14 Sep 2026 11:20:47 +0800 Subject: [PATCH] feat: name what a run hands to a person, and gate it as one resource A proposal and a clarification were never one thing in the type system, yet every layer treated them as one: the same `domain/hitl/` directory, the same service, the same queue, the same owner fields, the same filter copied twice. This names that concept `Handoff`, a point a run reached that only a person can settle, and hangs the permission model off it. The capability catalog goes from ten ids to eight, and the verb set closes to read, write, manage, and run. A verb now says only whether a call changes the resource, never what the call is called, so `history.write` covers restoring rather than a `restore` verb that every new feature would want a sibling of. `manage` means the part of a resource that reaches other people, which is a wider scope rather than a stronger write, so `workspace.manage` covers members and roles and also whether one person sees everybody else's work. Three ids went because nothing was deciding anything with them. The check behind `workspace.read` was `effectiveRole !== null`, identical to the membership gate it always sat behind, so it passed everyone it was shown. `proposal.read` and `clarification.read` were never mounted on a route at all, which left the Studio nav enforcing them on its own. Four holes close with it: - `/batch` had no permission gate, so any member could start, stop, resume, or archive a batch. It now takes `skill.run` against the per-unit skill the ontology declares, since a batch is that skill run many times over. - `GET /agui/runs/:runId` replayed any run's whole event log. The visibility check lived on the `/runs` router, and `/agui` is a different prefix. - `GET /proposals/:id` and `GET /clarifications/:id` had no owner check, so the list filter was queue hygiene rather than a boundary. - `/coverage` carried each stage's pending ids to everybody, so a reader who could not open the queue could still count it. Two behaviours change on purpose. A maintainer now sees what the reactor handed over, which `includeServiceOwned: isOwner` had backwards, and which is the whole point of the role. And a settled handoff follows whether it is still private rather than whether it is still pending, so an applied proposal stays public as the graph's provenance while a rejected one stays with whoever raised it. The Documents surface lands on the same rule. Reading a document asks nothing, since a document is a reading of the graph and every member reads the graph. Writing one is the form's generator skill run on a projected subject, so `POST /views` takes `skill.run` against that skill, and the two generators inherit the owner-and-maintainer default from their manifests without declaring anything. The gate resolves the form before it decides, so an unknown kind or form is still answered as a bad request rather than as a refusal. Studio gets one answer to which surfaces a viewer may open, because the sidebar hiding a surface while the command palette still reached it was the same drift twice. Chords collapse to the six surfaces the sidebar keeps a row for, which is what lets every one of them be its own first letter. Several printed shortcuts had no handler behind them, and `G B` went somewhere other than its label said. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NGKCWu7ACm78YVGmPAywnx --- packages/core/src/application/ViewService.ts | 30 +- .../core/src/domain/hitl/handoffVisibility.ts | 26 ++ packages/core/src/index.ts | 1 + .../InMemoryClarificationRepository.ts | 6 +- .../in-memory/InMemoryProposalRepository.ts | 6 +- .../domain/hitl/handoffVisibility.test.ts | 31 ++ packages/schema/src/capability.ts | 33 ++- packages/schema/src/clarification.ts | 21 +- packages/schema/src/handoff.ts | 51 ++++ packages/schema/src/index.ts | 1 + packages/schema/src/proposal.ts | 21 +- packages/server/src/app.ts | 18 +- .../hitl/FsClarificationRepository.ts | 10 +- .../hitl/FsProposalRepository.ts | 11 +- packages/server/src/policy/checks.ts | 31 +- packages/server/src/routes/admin.ts | 2 +- packages/server/src/routes/agui.ts | 4 + packages/server/src/routes/batch.ts | 20 +- packages/server/src/routes/clarifications.ts | 61 +++- packages/server/src/routes/coverage.ts | 30 +- packages/server/src/routes/helpers.ts | 15 +- packages/server/src/routes/proposals.ts | 59 ++-- packages/server/src/routes/runVisibility.ts | 57 ++++ packages/server/src/routes/runs.ts | 52 +--- packages/server/src/routes/skills.ts | 9 +- .../server/src/routes/sourceConnection.ts | 5 +- packages/server/src/routes/views.ts | 20 +- .../server/src/routes/workspaceMembers.ts | 4 +- packages/server/src/routes/workspaces.ts | 6 +- .../hitl/FsProposalRepository.test.ts | 11 +- packages/server/test/policy/policy.test.ts | 46 +-- .../server/test/routes/handoffAccess.test.ts | 197 +++++++++++++ packages/server/test/routes/views.test.ts | 10 +- packages/studio/src/App.tsx | 31 +- .../studio/src/components/CommandPalette.tsx | 78 +++-- packages/studio/src/components/Sidebar.tsx | 45 ++- .../src/components/WorkspaceDetailsSheet.tsx | 45 ++- .../handoff/ClarificationDetail.tsx} | 266 +----------------- .../handoff/ProposalDetail.tsx} | 236 +--------------- packages/studio/src/lib/landingSurface.ts | 68 +++++ packages/studio/src/locales/en/inbox.ts | 5 + packages/studio/src/locales/zh-Hant/inbox.ts | 5 + packages/studio/src/pages/Documents.tsx | 10 +- packages/studio/src/pages/Inbox.tsx | 67 ++++- packages/studio/src/policy/checks.ts | 31 +- .../handoff/ClarificationDetail.test.ts} | 2 +- 46 files changed, 959 insertions(+), 835 deletions(-) create mode 100644 packages/core/src/domain/hitl/handoffVisibility.ts create mode 100644 packages/core/test/domain/hitl/handoffVisibility.test.ts create mode 100644 packages/schema/src/handoff.ts create mode 100644 packages/server/src/routes/runVisibility.ts create mode 100644 packages/server/test/routes/handoffAccess.test.ts rename packages/studio/src/{pages/Clarification.tsx => components/handoff/ClarificationDetail.tsx} (74%) rename packages/studio/src/{pages/Proposals.tsx => components/handoff/ProposalDetail.tsx} (74%) rename packages/studio/test/{pages/Clarification.test.ts => components/handoff/ClarificationDetail.test.ts} (97%) diff --git a/packages/core/src/application/ViewService.ts b/packages/core/src/application/ViewService.ts index 2cf6357b..cd9d94a0 100644 --- a/packages/core/src/application/ViewService.ts +++ b/packages/core/src/application/ViewService.ts @@ -10,6 +10,7 @@ import type { ViewArtifact, ViewArtifactFormat, ViewContent, + ViewFormDescriptor, ViewFormId, ViewKind, ViewKindDescriptor, @@ -140,6 +141,29 @@ export class ViewService { return generator === undefined ? undefined : formOfId(generator.forms, form)?.format } + /** + * The skill a form runs to write its document. + * + * Exposed so a caller can ask who may run it before anything is projected, + * since writing a document is that skill run, + * and permission to write one is permission to run it. + * Refuses an unknown kind or form the way `generate` does, + * so a bad request is answered as a bad request rather than as a refusal. + */ + skillIdFor(kind: ViewKind, form: ViewFormId): SkillId { + const generator = this.deps.pluginRegistry.requireViewGenerator(kind) + return skillFor(generator, this.requireForm(generator, kind, form).id) + } + + private requireForm(generator: ViewGeneratorPlugin, kind: ViewKind, form: ViewFormId): ViewFormDescriptor { + const match = formOfId(generator.forms, form) + if (match === undefined) { + const named = generator.forms.map(one => one.id).join(', ') + throw new ValidationError(`A "${kind}" view is written in one of ${named}`) + } + return match + } + /** * Project the subject, then set a form writing it out. * @@ -148,11 +172,7 @@ export class ViewService { */ async generate(input: GenerateViewInput): Promise { const generator = this.deps.pluginRegistry.requireViewGenerator(input.kind) - const form = formOfId(generator.forms, input.form) - if (form === undefined) { - const named = generator.forms.map(one => one.id).join(', ') - throw new ValidationError(`A "${input.kind}" view is written in one of ${named}`) - } + const form = this.requireForm(generator, input.kind, input.form) const environment = this.deps.environment ?? {} const unset = unsetRequirements(form, environment) diff --git a/packages/core/src/domain/hitl/handoffVisibility.ts b/packages/core/src/domain/hitl/handoffVisibility.ts new file mode 100644 index 00000000..ffbc7383 --- /dev/null +++ b/packages/core/src/domain/hitl/handoffVisibility.ts @@ -0,0 +1,26 @@ +import type { Actor, ClarificationStatus, ProposalStatus, UserId, UserKind } from '@braidhq/schema' + +/** The part of a handoff that decides who may see it. */ +export interface HandoffVisibility { + readonly status: ProposalStatus | ClarificationStatus + readonly owner: Actor + readonly ownerKind?: UserKind | undefined +} + +/** + * Whether one person may see a handoff that somebody else may have made. + * + * An applied handoff is public, because it is part of why the graph says + * what it says, and hiding it would hide the model's own provenance. + * Everything else stays with whoever handed it over, + * plus whatever a service handed over, + * which belongs to the workspace rather than to any one person. + * + * Callers holding `workspace.manage` skip this by passing no viewer at all, + * which is the one way to see another person's unsettled work. + */ +export function handoffVisibleTo(handoff: HandoffVisibility, viewerId: UserId): boolean { + return handoff.status === 'applied' + || handoff.owner === viewerId + || handoff.ownerKind === 'service' +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index c06840b6..5fea14b6 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -37,6 +37,7 @@ export * from './domain/history/WorkspaceHistory.js' export * from './domain/hitl/Clarification.js' export * from './domain/hitl/clarificationOutcome.js' export * from './domain/hitl/ClarificationRepository.js' +export * from './domain/hitl/handoffVisibility.js' export * from './domain/hitl/Proposal.js' export * from './domain/hitl/ProposalRepository.js' export * from './domain/hitl/runSubmission.js' diff --git a/packages/core/src/infrastructure/in-memory/InMemoryClarificationRepository.ts b/packages/core/src/infrastructure/in-memory/InMemoryClarificationRepository.ts index d5c6775c..fefead81 100644 --- a/packages/core/src/infrastructure/in-memory/InMemoryClarificationRepository.ts +++ b/packages/core/src/infrastructure/in-memory/InMemoryClarificationRepository.ts @@ -1,6 +1,7 @@ import type { ClarificationFilter, ClarificationId } from '@braidhq/schema' import type { Clarification } from '../../domain/hitl/Clarification.js' import type { ClarificationRepository } from '../../domain/hitl/ClarificationRepository.js' +import { handoffVisibleTo } from '../../domain/hitl/handoffVisibility.js' import { paginate } from '../../domain/paginate.js' import { InMemoryKeyedStore } from './InMemoryKeyedStore.js' @@ -19,10 +20,7 @@ export class InMemoryClarificationRepository implements ClarificationRepository } if (filter?.viewerId !== undefined) { const viewerId = filter.viewerId - const includeServiceOwned = filter.includeServiceOwned ?? false - clarifications = clarifications.filter(clarification => - clarification.status !== 'pending' || clarification.owner === viewerId || (includeServiceOwned && clarification.ownerKind === 'service'), - ) + clarifications = clarifications.filter(clarification => handoffVisibleTo(clarification, viewerId)) } return paginate(clarifications, filter?.limit, filter?.offset) } diff --git a/packages/core/src/infrastructure/in-memory/InMemoryProposalRepository.ts b/packages/core/src/infrastructure/in-memory/InMemoryProposalRepository.ts index 79213ea6..98321c23 100644 --- a/packages/core/src/infrastructure/in-memory/InMemoryProposalRepository.ts +++ b/packages/core/src/infrastructure/in-memory/InMemoryProposalRepository.ts @@ -1,6 +1,7 @@ import type { ProposalFilter, ProposalId } from '@braidhq/schema' import type { Proposal } from '../../domain/hitl/Proposal.js' import type { ProposalRepository } from '../../domain/hitl/ProposalRepository.js' +import { handoffVisibleTo } from '../../domain/hitl/handoffVisibility.js' import { paginate } from '../../domain/paginate.js' import { InMemoryKeyedStore } from './InMemoryKeyedStore.js' @@ -23,10 +24,7 @@ export class InMemoryProposalRepository implements ProposalRepository { } if (filter?.viewerId !== undefined) { const viewerId = filter.viewerId - const includeServiceOwned = filter.includeServiceOwned ?? false - proposals = proposals.filter(proposal => - proposal.status !== 'pending' || proposal.owner === viewerId || (includeServiceOwned && proposal.ownerKind === 'service'), - ) + proposals = proposals.filter(proposal => handoffVisibleTo(proposal, viewerId)) } return paginate(proposals, filter?.limit, filter?.offset) } diff --git a/packages/core/test/domain/hitl/handoffVisibility.test.ts b/packages/core/test/domain/hitl/handoffVisibility.test.ts new file mode 100644 index 00000000..22f19a28 --- /dev/null +++ b/packages/core/test/domain/hitl/handoffVisibility.test.ts @@ -0,0 +1,31 @@ +import type { Actor, UserId } from '@braidhq/schema' +import { describe, expect, it } from 'vitest' +import { handoffVisibleTo } from '../../../src/domain/hitl/handoffVisibility.js' + +const alice = 'usr-alice' as UserId +const bob = 'usr-bob' as UserId + +describe('handoffVisibleTo', () => { + it('shows a viewer what they handed over themselves', () => { + expect(handoffVisibleTo({ status: 'pending', owner: alice as Actor }, alice)).toBe(true) + }) + + it('hides what somebody else handed over and has not settled', () => { + expect(handoffVisibleTo({ status: 'pending', owner: bob as Actor }, alice)).toBe(false) + }) + + it('shows what a service handed over, since it belongs to the workspace', () => { + const autonomous = { status: 'pending' as const, owner: 'system' as Actor, ownerKind: 'service' as const } + expect(handoffVisibleTo(autonomous, alice)).toBe(true) + }) + + it('shows an applied handoff to everybody, since it is now the graph\'s provenance', () => { + expect(handoffVisibleTo({ status: 'applied', owner: bob as Actor }, alice)).toBe(true) + }) + + it('keeps a rejected or skipped handoff with the person who raised it', () => { + expect(handoffVisibleTo({ status: 'rejected', owner: bob as Actor }, alice)).toBe(false) + expect(handoffVisibleTo({ status: 'skipped', owner: bob as Actor }, alice)).toBe(false) + expect(handoffVisibleTo({ status: 'answered', owner: bob as Actor }, alice)).toBe(false) + }) +}) diff --git a/packages/schema/src/capability.ts b/packages/schema/src/capability.ts index 1b4916e1..3fe7535f 100644 --- a/packages/schema/src/capability.ts +++ b/packages/schema/src/capability.ts @@ -3,26 +3,33 @@ import { z } from 'zod' /** * The closed catalog of capability ids the platform gates on. The server is the * authoritative check, Studio reads the same list to show locked affordances. - * Each resource has at most one read and one write, write means change anything - * mutable in that resource. Plugins register their own checks under custom string - * ids, this list is the first-party set. + * + * A capability is named for its resource, never for the surface that shows it, + * so a page that moves or merges costs no rename here. + * The verb is one of four, and it says only whether the call changes the + * resource, never what the call is called. + * `manage` is the part of a resource that reaches other people, + * which is a wider scope rather than a stronger write. + * + * A resource every member may read needs no id at all, + * since the membership gate upstream has already answered that. + * Plugins register their own checks under custom string ids, + * this list is the first-party set. */ export const Capability = z.enum([ // Server scope, no workspace member required. - 'workspace.create', // scaffold a new workspace or register a path - 'server.admin', // manage users and invites, requires the admin serverRole + 'server.write', // scaffold a workspace + 'server.manage', // manage users and invites, requires the admin serverRole // Workspace scope. - 'workspace.read', // open the workspace, see settings, sources, members, graph, history - 'workspace.write', // edit PRODUCT.md, sources, plugins, ontology, members, or delete - 'proposal.read', // list and read pending or decided proposals - 'proposal.write', // submit, apply, reject - 'clarification.read', // list and read clarifications - 'clarification.write', // answer, skip + 'handoff.read', // list and read what runs have handed over for a person to settle + 'handoff.write', // apply, reject, answer, skip, defer + 'workspace.write', // edit PRODUCT.md, sources, mcp servers, or delete the workspace + 'workspace.manage', // members and roles, plus everyone's handoffs and conversations 'history.write', // restore a past commit, manage tags - // Skill run carries a per-member override matrix on top of the role default, - // so it stays its own verb rather than a read or write pair. + // Running carries a per-member override matrix on top of the role default, + // so it keeps its own verb rather than folding into a write. 'skill.run', ]) export type Capability = z.infer diff --git a/packages/schema/src/clarification.ts b/packages/schema/src/clarification.ts index 5bae651c..932f2b96 100644 --- a/packages/schema/src/clarification.ts +++ b/packages/schema/src/clarification.ts @@ -1,6 +1,5 @@ import { z } from 'zod' import { - Actor, ClarificationCandidateId, ClarificationId, ExternalReference, @@ -11,8 +10,8 @@ import { UserId, WorkspaceId, } from './common.js' +import { HandoffFilter, HandoffOwner } from './handoff.js' import { GraphOperation } from './proposal.js' -import { UserKind } from './user.js' // Only the hard contract here. Authoring rules (length, tone, language) live in the skill layer. const clarificationQuestion = z.string().min(1).max(400).describe('The single question shown to the reviewer.') @@ -48,7 +47,7 @@ export type ClarificationCandidate = z.infer export const ClarificationAnswerMode = z.enum(['resumes', 'standing']) export type ClarificationAnswerMode = z.infer -export const Clarification = z.object({ +export const Clarification = HandoffOwner.extend({ id: ClarificationId, workspaceId: WorkspaceId, question: clarificationQuestion, @@ -57,13 +56,6 @@ export const Clarification = z.object({ answeredBy: UserId.optional(), selectedCandidateId: ClarificationCandidateId.optional(), resolution: z.array(GraphOperation).optional(), - // The user who filed it, or 'system' for autonomous ones. Pending is owner-only. - owner: Actor, - // Display-name snapshot at submit time. Absent for the 'system' owner. - ownerDisplayName: z.string().min(1).optional(), - // Owner's kind snapshotted at submit time. - // Absent means a human's private clarification, 'service' is autonomous and owner-visible. - ownerKind: UserKind.optional(), // Set when the resolution becomes a Proposal, so the UI can link the two. proposalId: ProposalId.optional(), externalReferences: z.array(ExternalReference).optional(), @@ -118,14 +110,7 @@ export const ClarificationCreateBody = ClarificationCreate .extend({ candidates: z.array(ClarificationCandidate.partial({ id: true })) }) export type ClarificationCreateBody = z.infer -export const ClarificationFilter = z.object({ - workspaceId: WorkspaceId.optional(), +export const ClarificationFilter = HandoffFilter.extend({ statuses: z.array(ClarificationStatus).optional(), - limit: z.number().int().positive().optional(), - offset: z.number().int().nonnegative().optional(), - // When set, hides others' pending clarifications. Non-pending stay visible, absent shows all. - viewerId: UserId.optional(), - // Owner-only, also shows service-owned (autonomous) pending to this viewer. - includeServiceOwned: z.boolean().optional(), }) export type ClarificationFilter = z.infer diff --git a/packages/schema/src/handoff.ts b/packages/schema/src/handoff.ts new file mode 100644 index 00000000..d35294ed --- /dev/null +++ b/packages/schema/src/handoff.ts @@ -0,0 +1,51 @@ +import { z } from 'zod' +import { Actor, UserId, WorkspaceId } from './common.js' +import { UserKind } from './user.js' + +/** + * A run reached a point only a person can settle, and handed it over. + * + * Always from a run to a person, never the other way, + * and never between two runs, which is what the word means elsewhere. + * Answering a clarification carries its run on, + * but that is the answer travelling back, not a second handoff. + * + * A proposal and a clarification are the two shapes this takes. + * What makes both one kind is who must act next, + * rather than whether a run is parked waiting on it, + * since a proposal ends its run and a resuming clarification suspends one. + */ +export const HandoffKind = z.enum(['proposal', 'clarification']) +export type HandoffKind = z.infer + +/** + * Who handed this over, snapshotted at the moment they did. + * + * The display name survives a rename, because it records who it was then. + * A 'service' kind marks an autonomous handoff, + * which belongs to the workspace rather than to any one person, + * so everybody who may act on handoffs sees it. + */ +export const HandoffOwner = z.object({ + owner: Actor, + ownerDisplayName: z.string().min(1).optional(), + ownerKind: UserKind.optional(), +}) +export type HandoffOwner = z.infer + +/** + * The query shape every handoff listing shares. + * + * A present `viewerId` narrows the result to what that person may see, + * which is their own plus anything a service handed over. + * Absent applies no narrowing, which is what `workspace.manage` grants. + * Each kind extends this with its own statuses, + * since the two do not share a status enum. + */ +export const HandoffFilter = z.object({ + workspaceId: WorkspaceId.optional(), + limit: z.number().int().positive().optional(), + offset: z.number().int().nonnegative().optional(), + viewerId: UserId.optional(), +}) +export type HandoffFilter = z.infer diff --git a/packages/schema/src/index.ts b/packages/schema/src/index.ts index b81b16ac..1d591b69 100644 --- a/packages/schema/src/index.ts +++ b/packages/schema/src/index.ts @@ -12,6 +12,7 @@ export * from './embedding.js' export * from './error.js' export * from './event.js' export * from './graph-validation.js' +export * from './handoff.js' export * from './history.js' export * from './locale.js' export * from './mcp.js' diff --git a/packages/schema/src/proposal.ts b/packages/schema/src/proposal.ts index 4cb061a5..9a426d97 100644 --- a/packages/schema/src/proposal.ts +++ b/packages/schema/src/proposal.ts @@ -1,6 +1,5 @@ import { z } from 'zod' import { - Actor, ClarificationId, EdgeId, ExternalReference, @@ -12,6 +11,7 @@ import { UserId, WorkspaceId, } from './common.js' +import { HandoffFilter, HandoffOwner } from './handoff.js' import { GraphEdge, GraphEdgeCreate, @@ -21,7 +21,6 @@ import { GraphNodeUpdate, } from './model.js' import { SourceUnit } from './source-unit.js' -import { UserKind } from './user.js' // Only the hard contract here. Authoring rules (length, tone, language) live in the skill layer. const proposalRationale = z.string().min(1).max(1500).describe('One-paragraph plain-text summary of what changed and why.') @@ -45,7 +44,7 @@ export type GraphOperation = z.infer export const ProposalStatus = z.enum(['pending', 'applied', 'rejected']) export type ProposalStatus = z.infer -export const Proposal = z.object({ +export const Proposal = HandoffOwner.extend({ id: ProposalId, workspaceId: WorkspaceId, status: ProposalStatus, @@ -82,13 +81,6 @@ export const Proposal = z.object({ * while the model reflected none of it. */ sourceUnits: z.array(SourceUnit).optional(), - // The user who created it, or 'system' for autonomous ones. Pending is owner-only. - owner: Actor, - // Name at submit time, survives renames. Absent for the 'system' owner. - ownerDisplayName: z.string().min(1).optional(), - // Owner's kind snapshotted at submit time. - // Absent means a human's private draft, 'service' is autonomous and owner-visible. - ownerKind: UserKind.optional(), }) export type Proposal = z.infer @@ -104,15 +96,8 @@ export const ProposalCreate = z.object({ }) export type ProposalCreate = z.infer -export const ProposalFilter = z.object({ - workspaceId: WorkspaceId.optional(), +export const ProposalFilter = HandoffFilter.extend({ statuses: z.array(ProposalStatus).optional(), generatedBy: z.array(SkillId).optional(), - limit: z.number().int().positive().optional(), - offset: z.number().int().nonnegative().optional(), - // When set, hides others' pending proposals. Non-pending stay visible, absent shows all. - viewerId: UserId.optional(), - // Owner-only, also shows service-owned (autonomous) pending to this viewer. - includeServiceOwned: z.boolean().optional(), }) export type ProposalFilter = z.infer diff --git a/packages/server/src/app.ts b/packages/server/src/app.ts index 40c43ec5..2a23509a 100644 --- a/packages/server/src/app.ts +++ b/packages/server/src/app.ts @@ -335,11 +335,21 @@ export function createApp(deps: AppDependencies, options: AppOptions = {}): Open if (deps.historyService) { workspaceScoped.route('/history', createHistoryRouter({ historyService: deps.historyService })) } - if (deps.batchService) { - workspaceScoped.route('/batch', createBatchRouter({ batchService: deps.batchService })) + if (deps.batchService && deps.skillRegistry) { + workspaceScoped.route('/batch', createBatchRouter({ + batchService: deps.batchService, + workspaceRepository: deps.workspaceRepository, + skillRegistry: deps.skillRegistry, + pluginRegistry: deps.pluginRegistry, + })) + } + if (deps.viewService && deps.skillRegistry) { + workspaceScoped.route('/views', createViewsRouter({ + viewService: deps.viewService, + skillRegistry: deps.skillRegistry, + workspaceRepository: deps.workspaceRepository, + })) } - if (deps.viewService) - workspaceScoped.route('/views', createViewsRouter({ viewService: deps.viewService })) workspaceScoped.route('/reactor-cycles', createReactorCyclesRouter({ reactorCycleRepository: deps.reactorCycleRepository, })) diff --git a/packages/server/src/infrastructure/hitl/FsClarificationRepository.ts b/packages/server/src/infrastructure/hitl/FsClarificationRepository.ts index 39802279..25eb0e4f 100644 --- a/packages/server/src/infrastructure/hitl/FsClarificationRepository.ts +++ b/packages/server/src/infrastructure/hitl/FsClarificationRepository.ts @@ -4,7 +4,7 @@ import type { ClarificationId, WorkspaceId, } from '@braidhq/schema' -import { Clarification, type ClarificationRepository, paginate } from '@braidhq/core' +import { Clarification, type ClarificationRepository, handoffVisibleTo, paginate } from '@braidhq/core' import { Clarification as ClarificationSchema } from '@braidhq/schema' import { clarificationDir, CLARIFY_STATUSES } from '../_shared/paths.js' import { StatusedJsonStore } from './StatusedJsonStore.js' @@ -37,14 +37,10 @@ export class FsClarificationRepository implements ClarificationRepository { ...(filter?.workspaceId !== undefined ? { workspaceId: filter.workspaceId } : {}), ...(filter?.statuses !== undefined ? { statuses: filter.statuses } : {}), }) - // Pending clarifications are personal, only the owner sees them. - // Answered, applied, and skipped clarifications stay workspace-shared. + // Absent viewerId means no narrowing, which is what `workspace.manage` grants. if (filter?.viewerId !== undefined) { const viewerId = filter.viewerId - const includeServiceOwned = filter.includeServiceOwned ?? false - clarifications = clarifications.filter(clarification => - clarification.status !== 'pending' || clarification.owner === viewerId || (includeServiceOwned && clarification.ownerKind === 'service'), - ) + clarifications = clarifications.filter(clarification => handoffVisibleTo(clarification, viewerId)) } return paginate(clarifications, filter?.limit, filter?.offset) } diff --git a/packages/server/src/infrastructure/hitl/FsProposalRepository.ts b/packages/server/src/infrastructure/hitl/FsProposalRepository.ts index 371f68bd..18ea71af 100644 --- a/packages/server/src/infrastructure/hitl/FsProposalRepository.ts +++ b/packages/server/src/infrastructure/hitl/FsProposalRepository.ts @@ -1,5 +1,5 @@ import type { AbsolutePath, ProposalFilter, ProposalId, WorkspaceId } from '@braidhq/schema' -import { paginate, Proposal, type ProposalRepository } from '@braidhq/core' +import { handoffVisibleTo, paginate, Proposal, type ProposalRepository } from '@braidhq/core' import { Proposal as ProposalSchema } from '@braidhq/schema' import { PROPOSAL_STATUSES, proposalsDir } from '../_shared/paths.js' import { StatusedJsonStore } from './StatusedJsonStore.js' @@ -36,15 +36,10 @@ export class FsProposalRepository implements ProposalRepository { const skills = filter.generatedBy proposals = proposals.filter(proposal => skills.includes(proposal.generatedBy)) } - // Pending proposals are personal, only the owner sees them. - // Applied and rejected stay as workspace-shared audit history. - // Absent viewerId means no filter, for Owner Show All and legacy callers. + // Absent viewerId means no narrowing, which is what `workspace.manage` grants. if (filter?.viewerId !== undefined) { const viewerId = filter.viewerId - const includeServiceOwned = filter.includeServiceOwned ?? false - proposals = proposals.filter(proposal => - proposal.status !== 'pending' || proposal.owner === viewerId || (includeServiceOwned && proposal.ownerKind === 'service'), - ) + proposals = proposals.filter(proposal => handoffVisibleTo(proposal, viewerId)) } return paginate(proposals, filter?.limit, filter?.offset) } diff --git a/packages/server/src/policy/checks.ts b/packages/server/src/policy/checks.ts index a0b40431..4df8dcc9 100644 --- a/packages/server/src/policy/checks.ts +++ b/packages/server/src/policy/checks.ts @@ -6,35 +6,30 @@ import type { CapabilityCheck } from './CapabilityCheck.js' * The server is the authoritative gate. * The client copy lets Studio render locked affordances, without hitting a 403. * - * Workspace-scope verbs collapse to read and write per resource. - * `workspace.create` is server-scope, resolved with no member. + * Server-scope checks read `serverRole` rather than `effectiveRole`, + * since they resolve with no member and a workspace owner is an owner too. + * + * A resource every member may read has no check here at all. + * `workspaceAccessMiddleware` already refused everybody else, + * so a check that only repeats it would never decide anything. + * * `skill.run` keeps its own verb for its three-step resolution, * covering owner short-circuit, per-member override, allowedRoles, * which does not fit a read or write pair. */ export const checks: readonly CapabilityCheck[] = [ - { id: 'workspace.create', evaluate: v => v.effectiveRole === 'owner' }, - // Server admin reads serverRole directly, not effectiveRole, - // since a workspace owner also resolves to an owner effectiveRole. - { id: 'server.admin', evaluate: v => v.user.serverRole === 'admin' }, - { id: 'workspace.read', evaluate: v => v.effectiveRole !== null }, - { id: 'workspace.write', evaluate: v => v.effectiveRole === 'owner' }, - { - id: 'proposal.read', - evaluate: v => v.effectiveRole === 'owner' || v.effectiveRole === 'maintainer', - }, + { id: 'server.write', evaluate: v => v.user.serverRole === 'admin' }, + { id: 'server.manage', evaluate: v => v.user.serverRole === 'admin' }, { - id: 'proposal.write', + id: 'handoff.read', evaluate: v => v.effectiveRole === 'owner' || v.effectiveRole === 'maintainer', }, { - id: 'clarification.read', - evaluate: v => v.effectiveRole === 'owner' || v.effectiveRole === 'maintainer', - }, - { - id: 'clarification.write', + id: 'handoff.write', evaluate: v => v.effectiveRole === 'owner' || v.effectiveRole === 'maintainer', }, + { id: 'workspace.write', evaluate: v => v.effectiveRole === 'owner' }, + { id: 'workspace.manage', evaluate: v => v.effectiveRole === 'owner' }, { id: 'history.write', evaluate: v => v.effectiveRole === 'owner' }, { id: 'skill.run', diff --git a/packages/server/src/routes/admin.ts b/packages/server/src/routes/admin.ts index fd335bc6..225388e1 100644 --- a/packages/server/src/routes/admin.ts +++ b/packages/server/src/routes/admin.ts @@ -185,7 +185,7 @@ function serviceAccountRefusal(userId: string): string { export function createAdminRouter(deps: AdminRouterDeps): OpenAPIHono { const router = new OpenAPIHono() - router.use('*', requireServerCapability('server.admin', deps.userRegistry)) + router.use('*', requireServerCapability('server.manage', deps.userRegistry)) router.openapi(listInvitesRoute, async (context) => { const items = await deps.accessPolicy.listInvites() diff --git a/packages/server/src/routes/agui.ts b/packages/server/src/routes/agui.ts index f234a657..be272f12 100644 --- a/packages/server/src/routes/agui.ts +++ b/packages/server/src/routes/agui.ts @@ -12,6 +12,7 @@ import { createAsyncQueue } from '../infrastructure/skill/asyncQueue.js' import { extractBearerToken, getUserId } from '../middleware/auth.js' import { getWorkspaceId } from '../middleware/workspaceId.js' import { loadWorkspaceById } from './helpers.js' +import { requireVisibleRun } from './runVisibility.js' export interface AguiRouterDeps { readonly skillRunner: SkillRunner @@ -257,6 +258,9 @@ export function createAguiRouter(deps: AguiRouterDeps): Hono { router.get('/runs/:runId', async (context) => { const workspace = await loadWorkspaceById(getWorkspaceId(context), deps.workspaceRepository) const runId = SkillRunId.parse(context.req.param('runId')) + // The events are the run, so replaying them answers as fully as the run + // endpoints do, and has to refuse the same runs they refuse. + await requireVisibleRun(context, workspace, runId, deps.runRepository) const threadId = context.req.query('threadId') ?? runId const encoder = sseEncoder() const translator = new AguiTranslator(threadId, runId) diff --git a/packages/server/src/routes/batch.ts b/packages/server/src/routes/batch.ts index ebba6361..d7fb02ed 100644 --- a/packages/server/src/routes/batch.ts +++ b/packages/server/src/routes/batch.ts @@ -1,10 +1,12 @@ -import type { BatchService } from '@braidhq/core' +import type { BatchService, PluginRegistry, SkillRegistry, WorkspaceRepository } from '@braidhq/core' import { NotFoundError } from '@braidhq/core' import { zValidator } from '@hono/zod-validator' import { Hono } from 'hono' import { z } from 'zod' import { extractBearerToken, getUserId } from '../middleware/auth.js' +import { requirePermission } from '../middleware/workspaceAccess.js' import { getWorkspaceId } from '../middleware/workspaceId.js' +import { loadWorkspaceById, resolvePerUnitSkillId } from './helpers.js' const StartBody = z.object({ autoApply: z.boolean(), @@ -15,11 +17,27 @@ const StartBody = z.object({ export interface BatchRouterDeps { batchService: BatchService + workspaceRepository: WorkspaceRepository + skillRegistry: SkillRegistry + pluginRegistry: PluginRegistry } export function createBatchRouter(deps: BatchRouterDeps): Hono { const router = new Hono() + // A batch is the per-unit skill run many times over, + // so whoever may run that skill may drive the batch, and nobody else. + // Reading the plan is left open, since it is the same standing + // the coverage board already gives every member. + router.on('POST', ['/', '/stop', '/resume', '/archive'], requirePermission('skill.run', async (context) => { + const workspace = await loadWorkspaceById(getWorkspaceId(context), deps.workspaceRepository) + const skillId = resolvePerUnitSkillId(deps.pluginRegistry, workspace) + if (!skillId) + return undefined + const manifest = await deps.skillRegistry.get(workspace, skillId) + return { skill: manifest.toData().frontmatter, skillId } + })) + router.post('/', zValidator('json', StartBody), async (context) => { const workspaceId = getWorkspaceId(context) const { autoApply, scope } = context.req.valid('json') diff --git a/packages/server/src/routes/clarifications.ts b/packages/server/src/routes/clarifications.ts index 3804d6a9..cf0863a8 100644 --- a/packages/server/src/routes/clarifications.ts +++ b/packages/server/src/routes/clarifications.ts @@ -1,11 +1,13 @@ -import type { ClarificationRepository, HITLService } from '@braidhq/core' +import type { Clarification as ClarificationEntity, ClarificationRepository, HITLService } from '@braidhq/core' +import type { Context } from 'hono' import type { RunOutputGate } from '../infrastructure/skill/RunOutputGate.js' -import { newClarificationCandidateId } from '@braidhq/core' +import { handoffVisibleTo, newClarificationCandidateId, NotFoundError } from '@braidhq/core' import { Clarification, ClarificationCandidateId, ClarificationCreateBody, ClarificationId, ClarificationStatus, ProposalId, UserId } from '@braidhq/schema' import { createRoute, OpenAPIHono, z } from '@hono/zod-openapi' import { getSkillRunId, getUserId } from '../middleware/auth.js' import { getViewerContext, requirePermission } from '../middleware/workspaceAccess.js' import { getWorkspaceId } from '../middleware/workspaceId.js' +import { defaultPermissionRegistry } from '../policy/index.js' import { forRuns, NotFoundResponse, ValidationFailureResponse, WorkspaceIdParam } from './_shared.js' import { assertEntityInWorkspace } from './helpers.js' @@ -13,7 +15,7 @@ const ListQuery = z.object({ status: z.union([ClarificationStatus, z.array(ClarificationStatus)]).optional().openapi({ description: 'Filter by clarification status. Pass one or many.' }), limit: z.coerce.number().int().positive().optional(), offset: z.coerce.number().int().nonnegative().optional(), - showAll: z.coerce.boolean().optional().openapi({ description: 'Owner-only: bypass the personal-pending filter so every member\'s open questions are visible.' }), + showAll: z.coerce.boolean().optional().openapi({ description: 'Requires workspace.manage: drop the personal filter, so every member\'s open questions are visible.' }), }) // Reviewer-facing answer body. @@ -225,13 +227,36 @@ const reportNoClarificationRoute = createRoute(forRuns({ export function createClarificationRouter(deps: ClarificationRouterDeps): OpenAPIHono { const router = new OpenAPIHono() - // Answer, skip, and mark-applied are HITL decisions, + // Reading the queue is the Build surface's lower half, gated as one resource. + // Creating is a run handing a question over, carried by its own run token, + // so it stays outside the gate a person passes. + router.on('GET', ['/', '/:clarificationId'], requirePermission('handoff.read')) + // Answering, skipping, deferring, and marking applied all settle a handoff, // Owner and Maintainer only. - // Guests never see the tab, but a direct curl still 403s here. - router.use('/:clarificationId/answer', requirePermission('clarification.write')) - router.use('/:clarificationId/skip', requirePermission('clarification.write')) - router.use('/:clarificationId/defer', requirePermission('clarification.write')) - router.use('/:clarificationId', requirePermission('clarification.write')) + // Guests never see the queue, so this is what makes a direct curl 403. + router.on('PATCH', '/:clarificationId', requirePermission('handoff.write')) + router.use('/:clarificationId/answer', requirePermission('handoff.write')) + router.use('/:clarificationId/skip', requirePermission('handoff.write')) + router.use('/:clarificationId/defer', requirePermission('handoff.write')) + + /** Whether this caller sees every member's work, rather than only their own. */ + function seesEveryone(context: Context): boolean { + const viewer = getViewerContext(context) + return viewer !== undefined && defaultPermissionRegistry.can('workspace.manage', viewer) + } + + /** + * Refuse a clarification that belongs to somebody else. + * + * Reported as absent rather than forbidden, + * so the answer never confirms that another person's question exists. + */ + function requireVisible(context: Context, clarification: ClarificationEntity): void { + if (!getViewerContext(context) || seesEveryone(context)) + return + if (!handoffVisibleTo(clarification.toData(), getUserId(context))) + throw new NotFoundError(`Clarification "${clarification.id}" not found`) + } router.openapi(reportNoClarificationRoute, async (context) => { deps.outputGate?.declareNothingToClarify(getSkillRunId(context)) @@ -261,17 +286,18 @@ export function createClarificationRouter(deps: ClarificationRouterDeps): OpenAP const workspaceId = getWorkspaceId(context) const { status, limit, offset, showAll } = context.req.valid('query') const statuses = status === undefined ? undefined : Array.isArray(status) ? status : [status] - const viewer = getViewerContext(context) - const isOwner = viewer?.effectiveRole === 'owner' - // No viewer is an open composition (in-memory), which applies no personal filter. - const viewerId = (!viewer || (showAll && isOwner)) ? undefined : getUserId(context) - // Owners also see service-owned (autonomous) pending, since only they can act on it. + // Show All is the one way to read another person's unsettled work, + // so everyone else is narrowed whatever they send. + // No viewer is an open composition (in-memory), which narrows nothing. + const viewerId = (!getViewerContext(context) || (showAll && seesEveryone(context))) + ? undefined + : getUserId(context) const clarifications = await deps.clarificationRepository.list({ workspaceId, statuses, limit, offset, - ...(viewerId ? { viewerId, includeServiceOwned: isOwner } : {}), + ...(viewerId ? { viewerId } : {}), }) return context.json({ items: clarifications.map(clarification => clarification.toData()) }, 200) }) @@ -281,6 +307,7 @@ export function createClarificationRouter(deps: ClarificationRouterDeps): OpenAP const { clarificationId } = context.req.valid('param') const clarification = await deps.clarificationRepository.load(clarificationId) assertEntityInWorkspace(workspaceId, clarification.workspaceId, 'Clarification', clarificationId) + requireVisible(context, clarification) return context.json(clarification.toData(), 200) }) @@ -291,6 +318,7 @@ export function createClarificationRouter(deps: ClarificationRouterDeps): OpenAP const userId = body.userId ?? getUserId(context) const clarification = await deps.clarificationRepository.load(clarificationId) assertEntityInWorkspace(workspaceId, clarification.workspaceId, 'Clarification', clarificationId) + requireVisible(context, clarification) const selection = body.candidateId ? { kind: 'existing' as const, candidateId: body.candidateId } : { kind: 'custom' as const, description: body.customCandidate!.description } @@ -310,6 +338,7 @@ export function createClarificationRouter(deps: ClarificationRouterDeps): OpenAP const userId = bodyUserId ?? getUserId(context) const clarification = await deps.clarificationRepository.load(clarificationId) assertEntityInWorkspace(workspaceId, clarification.workspaceId, 'Clarification', clarificationId) + requireVisible(context, clarification) const applied = await deps.hitlService.markClarificationApplied(clarificationId, userId, proposalId) return context.json(applied.toData(), 200) }) @@ -319,6 +348,7 @@ export function createClarificationRouter(deps: ClarificationRouterDeps): OpenAP const { clarificationId } = context.req.valid('param') const clarification = await deps.clarificationRepository.load(clarificationId) assertEntityInWorkspace(workspaceId, clarification.workspaceId, 'Clarification', clarificationId) + requireVisible(context, clarification) const deferred = await deps.hitlService.deferClarification(clarificationId, getUserId(context)) return context.json(deferred.toData(), 200) }) @@ -330,6 +360,7 @@ export function createClarificationRouter(deps: ClarificationRouterDeps): OpenAP const userId = bodyUserId ?? getUserId(context) const clarification = await deps.clarificationRepository.load(clarificationId) assertEntityInWorkspace(workspaceId, clarification.workspaceId, 'Clarification', clarificationId) + requireVisible(context, clarification) const skipped = await deps.hitlService.skipClarification(clarificationId, reason, userId) return context.json(skipped.toData(), 200) }) diff --git a/packages/server/src/routes/coverage.ts b/packages/server/src/routes/coverage.ts index b7c73d7d..a0b27b8c 100644 --- a/packages/server/src/routes/coverage.ts +++ b/packages/server/src/routes/coverage.ts @@ -1,7 +1,10 @@ import type { CoverageProjection, WorkspaceRepository } from '@braidhq/core' +import type { CoverageBoard as CoverageBoardType } from '@braidhq/schema' import { CoverageBoard } from '@braidhq/schema' import { createRoute, OpenAPIHono } from '@hono/zod-openapi' +import { getViewerContext } from '../middleware/workspaceAccess.js' import { getWorkspaceId } from '../middleware/workspaceId.js' +import { defaultPermissionRegistry } from '../policy/index.js' import { WorkspaceIdParam } from './_shared.js' import { loadWorkspaceById } from './helpers.js' @@ -31,8 +34,33 @@ export function createCoverageRouter(deps: CoverageRouterDeps): OpenAPIHono { router.openapi(boardRoute, async (context) => { const workspace = await loadWorkspaceById(getWorkspaceId(context), deps.workspaceRepository) - return context.json(await deps.coverageProjection.board(workspace), 200) + const board = await deps.coverageProjection.board(workspace) + const viewer = getViewerContext(context) + const readsHandoffs = !viewer || defaultPermissionRegistry.can('handoff.read', viewer) + return context.json(readsHandoffs ? board : withoutHandoffs(board), 200) }) return router } + +/** + * The board as somebody who may not read the queue sees it. + * + * Counting what is waiting is still reading the queue, + * so the ids come off here rather than the surface being trusted + * not to render them. + * Every state stays, since what a document is still waiting on + * is the board's whole point, and it names nobody's work. + */ +function withoutHandoffs(board: CoverageBoardType): CoverageBoardType { + return { + ...board, + cards: board.cards.map(card => ({ ...card, proposalIds: [], clarificationIds: [] })), + stages: board.stages.map(stage => ({ + ...stage, + proposalIds: [], + clarificationIds: [], + answeredIds: [], + })), + } +} diff --git a/packages/server/src/routes/helpers.ts b/packages/server/src/routes/helpers.ts index 46396ad1..2ac2e5b9 100644 --- a/packages/server/src/routes/helpers.ts +++ b/packages/server/src/routes/helpers.ts @@ -1,5 +1,5 @@ -import type { Workspace, WorkspaceRepository } from '@braidhq/core' -import type { WorkspaceId } from '@braidhq/schema' +import type { PluginRegistry, Workspace, WorkspaceRepository } from '@braidhq/core' +import type { SkillId, WorkspaceId } from '@braidhq/schema' import { NotFoundError } from '@braidhq/core' export function assertEntityInWorkspace( @@ -23,3 +23,14 @@ export async function loadWorkspaceById( throw new NotFoundError(`Workspace "${workspaceId}" not registered`) return match } + +/** + * The skill a batch runs once per document, declared by the ontology. + * + * Absent means this ontology has no per-unit step, + * so there is nothing for a batch to run and nothing to authorise. + */ +export function resolvePerUnitSkillId(pluginRegistry: PluginRegistry, workspace: Workspace): SkillId | undefined { + const ontology = pluginRegistry.findOntology(workspace.productManifest.ontologyId) + return ontology?.batch?.perUnit?.skillId +} diff --git a/packages/server/src/routes/proposals.ts b/packages/server/src/routes/proposals.ts index 24169a3e..5894ce73 100644 --- a/packages/server/src/routes/proposals.ts +++ b/packages/server/src/routes/proposals.ts @@ -1,10 +1,13 @@ -import type { HITLService, ModelRepository, ModelValidationService, ProposalRepository, WorkspaceService } from '@braidhq/core' +import type { HITLService, ModelRepository, ModelValidationService, Proposal as ProposalEntity, ProposalRepository, WorkspaceService } from '@braidhq/core' +import type { Context } from 'hono' import type { RunOutputGate } from '../infrastructure/skill/RunOutputGate.js' +import { handoffVisibleTo, NotFoundError } from '@braidhq/core' import { Proposal, ProposalCreate, ProposalId, ProposalStatus, UserId, ValidationResult } from '@braidhq/schema' import { createRoute, OpenAPIHono, z } from '@hono/zod-openapi' import { getSkillRunId, getUserId } from '../middleware/auth.js' import { getViewerContext, requirePermission } from '../middleware/workspaceAccess.js' import { getWorkspaceId } from '../middleware/workspaceId.js' +import { defaultPermissionRegistry } from '../policy/index.js' import { forRuns, NotFoundResponse, ValidationFailureResponse, WorkspaceIdParam } from './_shared.js' import { assertEntityInWorkspace } from './helpers.js' @@ -12,7 +15,7 @@ const ListQuery = z.object({ status: z.union([ProposalStatus, z.array(ProposalStatus)]).optional().openapi({ description: 'Filter by proposal status. Pass one or many.' }), limit: z.coerce.number().int().positive().optional(), offset: z.coerce.number().int().nonnegative().optional(), - showAll: z.coerce.boolean().optional().openapi({ description: 'Owner-only: bypass the personal-pending filter so every member\'s drafts are visible.' }), + showAll: z.coerce.boolean().optional().openapi({ description: 'Requires workspace.manage: drop the personal filter, so every member\'s unsettled work is visible.' }), }) // Body `userId` is a back-compat shim. @@ -172,11 +175,33 @@ const rejectProposalRoute = createRoute(forRuns({ export function createProposalsRouter(deps: ProposalsRouterDeps): OpenAPIHono { const router = new OpenAPIHono() - // Apply and reject are HITL decisions, Owner and Maintainer only. - // Guests never see the buttons, the UI hides the tab. - // Defence-in-depth here means a direct curl from a Guest still 403s. - router.use('/:proposalId/apply', requirePermission('proposal.write')) - router.use('/:proposalId/reject', requirePermission('proposal.write')) + // Reading the queue is the Build surface's lower half, gated as one resource. + // Creating is a run handing something over, carried by its own run token, + // so it stays outside the gate a person passes. + router.on('GET', ['/', '/:proposalId', '/:proposalId/validate'], requirePermission('handoff.read')) + // Apply and reject settle a handoff, Owner and Maintainer only. + // Guests never see the buttons, so this is what makes a direct curl 403. + router.use('/:proposalId/apply', requirePermission('handoff.write')) + router.use('/:proposalId/reject', requirePermission('handoff.write')) + + /** Whether this caller sees every member's work, rather than only their own. */ + function seesEveryone(context: Context): boolean { + const viewer = getViewerContext(context) + return viewer !== undefined && defaultPermissionRegistry.can('workspace.manage', viewer) + } + + /** + * Refuse a proposal that belongs to somebody else. + * + * Reported as absent rather than forbidden, + * so the answer never confirms that another person's proposal exists. + */ + function requireVisible(context: Context, proposal: ProposalEntity): void { + if (!getViewerContext(context) || seesEveryone(context)) + return + if (!handoffVisibleTo(proposal.toData(), getUserId(context))) + throw new NotFoundError(`Proposal "${proposal.id}" not found`) + } router.openapi(createProposalRoute, async (context) => { const workspaceId = getWorkspaceId(context) @@ -197,20 +222,18 @@ export function createProposalsRouter(deps: ProposalsRouterDeps): OpenAPIHono { const workspaceId = getWorkspaceId(context) const { status, limit, offset, showAll } = context.req.valid('query') const statuses = status === undefined ? undefined : Array.isArray(status) ? status : [status] - // Show All bypass is gated to the workspace owner. - // Everyone else is forced through the personal-pending filter, - // whatever they send. - const viewer = getViewerContext(context) - const isOwner = viewer?.effectiveRole === 'owner' - // No viewer is an open composition (in-memory), which applies no personal filter. - const viewerId = (!viewer || (showAll && isOwner)) ? undefined : getUserId(context) - // Owners also see service-owned (autonomous) pending, since only they can apply it. + // Show All is the one way to read another person's unsettled work, + // so everyone else is narrowed whatever they send. + // No viewer is an open composition (in-memory), which narrows nothing. + const viewerId = (!getViewerContext(context) || (showAll && seesEveryone(context))) + ? undefined + : getUserId(context) const proposals = await deps.proposalRepository.list({ workspaceId, statuses, limit, offset, - ...(viewerId ? { viewerId, includeServiceOwned: isOwner } : {}), + ...(viewerId ? { viewerId } : {}), }) return context.json({ items: proposals.map(proposal => proposal.toData()) }, 200) }) @@ -220,6 +243,7 @@ export function createProposalsRouter(deps: ProposalsRouterDeps): OpenAPIHono { const { proposalId } = context.req.valid('param') const proposal = await deps.proposalRepository.load(proposalId) assertEntityInWorkspace(workspaceId, proposal.workspaceId, 'Proposal', proposalId) + requireVisible(context, proposal) return context.json(proposal.toData(), 200) }) @@ -228,6 +252,7 @@ export function createProposalsRouter(deps: ProposalsRouterDeps): OpenAPIHono { const { proposalId } = context.req.valid('param') const proposal = await deps.proposalRepository.load(proposalId) assertEntityInWorkspace(workspaceId, proposal.workspaceId, 'Proposal', proposalId) + requireVisible(context, proposal) const workspace = await deps.workspaceService.findById(workspaceId) const snapshot = await deps.modelRepository.load(workspaceId) const result = await deps.modelValidationService.validateOperations(snapshot, proposal.operations, workspace) @@ -241,6 +266,7 @@ export function createProposalsRouter(deps: ProposalsRouterDeps): OpenAPIHono { const userId = body.userId ?? getUserId(context) const proposal = await deps.proposalRepository.load(proposalId) assertEntityInWorkspace(workspaceId, proposal.workspaceId, 'Proposal', proposalId) + requireVisible(context, proposal) const applied = await deps.hitlService.applyProposal(proposalId, userId) return context.json(applied.toData(), 200) }) @@ -252,6 +278,7 @@ export function createProposalsRouter(deps: ProposalsRouterDeps): OpenAPIHono { const userId = bodyUserId ?? getUserId(context) const proposal = await deps.proposalRepository.load(proposalId) assertEntityInWorkspace(workspaceId, proposal.workspaceId, 'Proposal', proposalId) + requireVisible(context, proposal) const rejected = await deps.hitlService.rejectProposal(proposalId, reason, userId) return context.json(rejected.toData(), 200) }) diff --git a/packages/server/src/routes/runVisibility.ts b/packages/server/src/routes/runVisibility.ts new file mode 100644 index 00000000..5874dae3 --- /dev/null +++ b/packages/server/src/routes/runVisibility.ts @@ -0,0 +1,57 @@ +import type { RunRepository, Workspace } from '@braidhq/core' +import type { RunRecord, SkillRunId } from '@braidhq/schema' +import type { Context } from 'hono' +import { NotFoundError } from '@braidhq/core' +import { getUserId } from '../middleware/auth.js' +import { getViewerContext } from '../middleware/workspaceAccess.js' +import { defaultPermissionRegistry } from '../policy/index.js' + +/** + * Which runs this caller may read or act on. + * + * A conversation is personal, so it stays with whoever started it. + * `workspace.manage` is the one standing that reaches everybody else's, + * which is what makes a workspace answerable to whoever runs it. + */ +export function visibleToCaller(context: Context): (record: RunRecord) => boolean { + const viewer = getViewerContext(context) + // No viewer is an open composition (in-memory), which applies no filter. + if (!viewer || defaultPermissionRegistry.can('workspace.manage', viewer)) + return () => true + const userId = getUserId(context) + return record => record.startedBy === userId +} + +/** + * Refuse a run belonging to somebody else. + * + * Reported as absent rather than forbidden, + * so the answer never confirms that another person's run exists. + * An id with no record is left alone, + * since the endpoints already answer for a run they cannot find. + */ +export async function requireVisibleRun( + context: Context, + workspace: Workspace, + runId: SkillRunId, + runRepository: RunRepository, +): Promise { + const records = await runRepository.listRecords(workspace) + const record = records.find(candidate => candidate.runId === runId) + if (record && !visibleToCaller(context)(record)) + throw new NotFoundError(`Run "${runId}" not found`) +} + +/** The same rule for a session, which is visible through any run under it. */ +export async function requireVisibleSession( + context: Context, + workspace: Workspace, + sessionId: string, + runRepository: RunRepository, +): Promise { + const records = await runRepository.listRecords(workspace) + const under = records.filter(record => record.sessionId === sessionId) + const canSee = visibleToCaller(context) + if (under.length > 0 && !under.some(canSee)) + throw new NotFoundError(`Session "${sessionId}" not found`) +} diff --git a/packages/server/src/routes/runs.ts b/packages/server/src/routes/runs.ts index fe15a567..53443ace 100644 --- a/packages/server/src/routes/runs.ts +++ b/packages/server/src/routes/runs.ts @@ -4,7 +4,7 @@ import type { Workspace, WorkspaceRepository, } from '@braidhq/core' -import type { RunRecord, SessionMetadata, SkillEvent, SkillRunId as SkillRunIdType } from '@braidhq/schema' +import type { SessionMetadata, SkillEvent, SkillRunId as SkillRunIdType } from '@braidhq/schema' import type { Context } from 'hono' import { ConflictError, NotFoundError, ValidationError } from '@braidhq/core' import { SkillRunId } from '@braidhq/schema' @@ -12,10 +12,13 @@ import { Hono } from 'hono' import { streamSSE } from 'hono/streaming' import { z } from 'zod' import { createAsyncQueue } from '../infrastructure/skill/asyncQueue.js' -import { getUserId } from '../middleware/auth.js' -import { getViewerContext } from '../middleware/workspaceAccess.js' import { getWorkspaceId } from '../middleware/workspaceId.js' import { loadWorkspaceById } from './helpers.js' +import { + requireVisibleRun as requireRunVisible, + requireVisibleSession as requireSessionVisible, + visibleToCaller, +} from './runVisibility.js' export interface RunsRouterDeps { readonly runRepository: RunRepository @@ -25,45 +28,10 @@ export interface RunsRouterDeps { export function createRunsRouter(deps: RunsRouterDeps): Hono { const router = new Hono() - - /** - * Which runs this caller may read or act on. - * - * An owner reviews the workspace as a whole, so every run is theirs. - * Everyone else sees only what they asked for. - */ - function visibleToCaller(context: Context): (record: RunRecord) => boolean { - const viewer = getViewerContext(context) - // No viewer is an open composition (in-memory), which applies no filter. - if (!viewer || viewer.effectiveRole === 'owner') - return () => true - const userId = getUserId(context) - return record => record.startedBy === userId - } - - /** - * Refuse a run belonging to someone else. - * - * Reported as absent rather than forbidden, - * so the answer never confirms that another person's run exists. - * An id with no record is left alone, - * since the endpoints below already answer for a run they cannot find. - */ - async function requireVisibleRun(context: Context, workspace: Workspace, runId: SkillRunIdType): Promise { - const records = await deps.runRepository.listRecords(workspace) - const record = records.find(candidate => candidate.runId === runId) - if (record && !visibleToCaller(context)(record)) - throw new NotFoundError(`Run "${runId}" not found`) - } - - /** The same rule for a session, which is visible through any run under it. */ - async function requireVisibleSession(context: Context, workspace: Workspace, sessionId: string): Promise { - const records = await deps.runRepository.listRecords(workspace) - const under = records.filter(record => record.sessionId === sessionId) - const canSee = visibleToCaller(context) - if (under.length > 0 && !under.some(canSee)) - throw new NotFoundError(`Session "${sessionId}" not found`) - } + const requireVisibleRun = (context: Context, workspace: Workspace, runId: SkillRunIdType): Promise => + requireRunVisible(context, workspace, runId, deps.runRepository) + const requireVisibleSession = (context: Context, workspace: Workspace, sessionId: string): Promise => + requireSessionVisible(context, workspace, sessionId, deps.runRepository) router.get('/', async (context) => { const workspace = await loadWorkspaceById(getWorkspaceId(context), deps.workspaceRepository) diff --git a/packages/server/src/routes/skills.ts b/packages/server/src/routes/skills.ts index ebc7fdf9..70dbd845 100644 --- a/packages/server/src/routes/skills.ts +++ b/packages/server/src/routes/skills.ts @@ -8,7 +8,7 @@ import type { Workspace, WorkspaceRepository, } from '@braidhq/core' -import type { SkillEvent, SkillId, SkillRunId as SkillRunIdType } from '@braidhq/schema' +import type { SkillEvent, SkillRunId as SkillRunIdType } from '@braidhq/schema' import { createLogger, unitBearingRoleIds, ValidationError } from '@braidhq/core' import { SkillId as SkillIdSchema, SkillManifest, SkillRunId, SourceId } from '@braidhq/schema' import { createRoute, OpenAPIHono, z } from '@hono/zod-openapi' @@ -16,7 +16,7 @@ import { extractBearerToken, getUserId } from '../middleware/auth.js' import { requirePermission } from '../middleware/workspaceAccess.js' import { getWorkspaceId } from '../middleware/workspaceId.js' import { NotFoundResponse, WorkspaceIdParam } from './_shared.js' -import { loadWorkspaceById } from './helpers.js' +import { loadWorkspaceById, resolvePerUnitSkillId } from './helpers.js' // Long enough for a cold shallow fetch across a handful of repos, // short enough that an unresponsive remote does not look like a hung submit. @@ -231,11 +231,6 @@ export function createSkillsRouter(deps: SkillsRouterDeps): OpenAPIHono { return router } -function resolvePerUnitSkillId(pluginRegistry: PluginRegistry, workspace: Workspace): SkillId | undefined { - const ontology = pluginRegistry.findOntology(workspace.productManifest.ontologyId) - return ontology?.batch?.perUnit?.skillId -} - const recordLogger = createLogger('skills.recordObservation') // Stop waiting after this many ms, even if no terminal event arrives. diff --git a/packages/server/src/routes/sourceConnection.ts b/packages/server/src/routes/sourceConnection.ts index 09dd8a21..53f3b9c2 100644 --- a/packages/server/src/routes/sourceConnection.ts +++ b/packages/server/src/routes/sourceConnection.ts @@ -4,7 +4,6 @@ import type { SecretStore } from '../infrastructure/secrets/SecretStore.js' import { SourceId, WorkspaceId } from '@braidhq/schema' import { Hono } from 'hono' import { OAUTH_PROVIDERS } from '../infrastructure/oauth/providers.js' -import { requirePermission } from '../middleware/workspaceAccess.js' import { getWorkspaceId } from '../middleware/workspaceId.js' // OAuth namespaces a source credential can live under, @@ -46,7 +45,7 @@ export interface SourceConnectionRouterDeps { export function createSourceConnectionRouter(deps: SourceConnectionRouterDeps): HonoType { const router = new Hono() - router.get('/', requirePermission('workspace.read'), async (context) => { + router.get('/', async (context) => { const workspaceId = getWorkspaceId(context) const workspace = await deps.workspaceService.findById(WorkspaceId.parse(workspaceId)) const connections: SourceConnectionSummary[] = [] @@ -58,7 +57,7 @@ export function createSourceConnectionRouter(deps: SourceConnectionRouterDeps): return context.json({ connections }) }) - router.get('/:sourceId', requirePermission('workspace.read'), async (context) => { + router.get('/:sourceId', async (context) => { return context.json(await readStatus(deps.secretStore, getWorkspaceId(context), SourceId.parse(context.req.param('sourceId')))) }) diff --git a/packages/server/src/routes/views.ts b/packages/server/src/routes/views.ts index acdd6b5b..c5b72d6e 100644 --- a/packages/server/src/routes/views.ts +++ b/packages/server/src/routes/views.ts @@ -1,12 +1,16 @@ -import type { ViewService } from '@braidhq/core' +import type { SkillRegistry, ViewService, WorkspaceRepository } from '@braidhq/core' import { GeneratedView, GenerateViewRequest, GenerateViewResponse, ListViewsResponse, ViewContent } from '@braidhq/schema' import { createRoute, OpenAPIHono, z } from '@hono/zod-openapi' import { extractBearerToken, getUserId } from '../middleware/auth.js' +import { requirePermission } from '../middleware/workspaceAccess.js' import { getWorkspaceId } from '../middleware/workspaceId.js' import { ConflictResponse, NotFoundResponse, ValidationFailureResponse, WorkspaceIdParam } from './_shared.js' +import { loadWorkspaceById } from './helpers.js' export interface ViewsRouterDeps { viewService: ViewService + skillRegistry: SkillRegistry + workspaceRepository: WorkspaceRepository } /** @@ -83,6 +87,20 @@ const generateRoute = createRoute({ export function createViewsRouter(deps: ViewsRouterDeps): OpenAPIHono { const router = new OpenAPIHono() + // Writing a document is the form's skill run on a projected subject, + // so whoever may run that skill may write one, and nobody else. + // Reading is left open, since a document is a reading of the graph, + // and anybody who may read the graph may read what was written out of it. + // The resource builder refuses an unknown kind or form the way `generate` + // does, so a bad request is still answered as a bad request. + router.on('POST', '/', requirePermission('skill.run', async (context) => { + const workspace = await loadWorkspaceById(getWorkspaceId(context), deps.workspaceRepository) + const body = GenerateViewRequest.parse(await context.req.json()) + const skillId = deps.viewService.skillIdFor(body.kind, body.form) + const manifest = await deps.skillRegistry.get(workspace, skillId) + return { skill: manifest.toData().frontmatter, skillId } + })) + router.openapi(listRoute, async (context) => { const items = await deps.viewService.list(getWorkspaceId(context)) return context.json({ items: [...items] }, 200) diff --git a/packages/server/src/routes/workspaceMembers.ts b/packages/server/src/routes/workspaceMembers.ts index 3f8650c1..5326ff2b 100644 --- a/packages/server/src/routes/workspaceMembers.ts +++ b/packages/server/src/routes/workspaceMembers.ts @@ -33,7 +33,7 @@ export interface WorkspaceMembersRouterDeps { export function createWorkspaceMembersRouter(deps: WorkspaceMembersRouterDeps): Hono { const router = new Hono() - const ownerOnly = requirePermission('workspace.write') + const ownerOnly = requirePermission('workspace.manage') // List is open to every member of the workspace. // The access middleware upstream already enforced membership. @@ -87,7 +87,7 @@ export function createWorkspaceMembersRouter(deps: WorkspaceMembersRouterDeps): export function createTransferOwnershipRouter(deps: WorkspaceMembersRouterDeps): Hono { const router = new Hono() - router.post('/', requirePermission('workspace.write'), zValidator('json', TransferBody), async (context) => { + router.post('/', requirePermission('workspace.manage'), zValidator('json', TransferBody), async (context) => { const workspaceId = getWorkspaceId(context) const { newOwnerId } = context.req.valid('json') const workspace = await deps.workspaceService.findById(workspaceId) diff --git a/packages/server/src/routes/workspaces.ts b/packages/server/src/routes/workspaces.ts index b5cc4de5..3c970c23 100644 --- a/packages/server/src/routes/workspaces.ts +++ b/packages/server/src/routes/workspaces.ts @@ -160,7 +160,7 @@ export function createWorkspacesRouter(deps: WorkspacesRouterDeps): OpenAPIHono // Server-scope gate for creation, admin-only. // Skips without userRegistry, so in-memory tests stay open. - const serverCreate = requireServerCapability('workspace.create', deps.userRegistry) + const serverCreate = requireServerCapability('server.write', deps.userRegistry) // Workspace-scope gate composed inline per :workspaceId route below. // `workspaceIdMiddleware` resolves the path param onto the context, @@ -195,7 +195,7 @@ export function createWorkspacesRouter(deps: WorkspacesRouterDeps): OpenAPIHono return context.json({ items: visible.map(workspace => workspace.toData()) }, 200) }) - router.get('/:workspaceId', workspaceIdMiddleware, wsAccess, requirePermission('workspace.read'), async (context) => { + router.get('/:workspaceId', workspaceIdMiddleware, wsAccess, async (context) => { const workspaceId = getWorkspaceId(context) const workspace = await deps.workspaceService.findById(workspaceId) return context.json(workspace.toData()) @@ -392,7 +392,7 @@ export function createWorkspacesRouter(deps: WorkspacesRouterDeps): OpenAPIHono // so Studio can show a stale or failing mirror without opening each source, // and monitoring can alert on one that has not succeeded in a while. // Read-only, so any member sees it, matching the source-connection route. - router.get('/:workspaceId/source-sync-states', workspaceIdMiddleware, wsAccess, requirePermission('workspace.read'), async (context) => { + router.get('/:workspaceId/source-sync-states', workspaceIdMiddleware, wsAccess, async (context) => { const states = await deps.syncStateRepository.listByWorkspace(getWorkspaceId(context)) return context.json({ states }) }) diff --git a/packages/server/test/infrastructure/hitl/FsProposalRepository.test.ts b/packages/server/test/infrastructure/hitl/FsProposalRepository.test.ts index 9f9eee0a..6c255d2f 100644 --- a/packages/server/test/infrastructure/hitl/FsProposalRepository.test.ts +++ b/packages/server/test/infrastructure/hitl/FsProposalRepository.test.ts @@ -66,7 +66,7 @@ describe('FsProposalRepository', () => { expect(pending.map(p => p.id)).toEqual(['p-1']) }) - it('shows service-owned pending only when includeServiceOwned is set (owner view)', async () => { + it('narrows to the viewer\'s own plus whatever a service handed over', async () => { const root = await makeWorkspaceRoot() const workspaceId = 'ws-1' as WorkspaceId const repository = new FsProposalRepository({ @@ -77,11 +77,12 @@ describe('FsProposalRepository', () => { await repository.save(makeProposal('p-reactor', workspaceId, 'pending', 'reactor' as UserId, 'service')) await repository.save(makeProposal('p-bob', workspaceId, 'pending', 'bob' as UserId)) - const personal = await repository.list({ workspaceId, viewerId: alice }) - expect(personal.map(p => p.id).sort()).toEqual(['p-mine']) + const narrowed = await repository.list({ workspaceId, viewerId: alice }) + expect(narrowed.map(p => p.id).sort()).toEqual(['p-mine', 'p-reactor']) - const asOwner = await repository.list({ workspaceId, viewerId: alice, includeServiceOwned: true }) - expect(asOwner.map(p => p.id).sort()).toEqual(['p-mine', 'p-reactor']) + // No viewer is what `workspace.manage` grants, and it reaches Bob's too. + const everyone = await repository.list({ workspaceId }) + expect(everyone.map(p => p.id).sort()).toEqual(['p-bob', 'p-mine', 'p-reactor']) }) it('load throws NotFoundError when proposal missing', async () => { diff --git a/packages/server/test/policy/policy.test.ts b/packages/server/test/policy/policy.test.ts index 6690f239..4d191b45 100644 --- a/packages/server/test/policy/policy.test.ts +++ b/packages/server/test/policy/policy.test.ts @@ -74,45 +74,53 @@ describe('resolveViewer', () => { describe('PermissionRegistry (default)', () => { const registry = buildDefaultPermissionRegistry() - it('owners get read + write on everything', () => { + it('owners get every workspace-scope capability', () => { const v = resolveViewer(makeUser('user'), makeMember('owner')) - const all: Capability[] = ['workspace.read', 'workspace.write', 'proposal.read', 'proposal.write', 'clarification.read', 'clarification.write', 'history.write'] + const all: Capability[] = ['workspace.write', 'workspace.manage', 'handoff.read', 'handoff.write', 'history.write'] for (const cap of all) expect(registry.can(cap, v)).toBe(true) }) - it('maintainers get reads + proposal/clarifications write, no workspace.write or history.write', () => { + it('maintainers settle handoffs, but govern nothing', () => { const v = resolveViewer(makeUser('user'), makeMember('maintainer')) - expect(registry.can('workspace.read', v)).toBe(true) - expect(registry.can('proposal.read', v)).toBe(true) - expect(registry.can('proposal.write', v)).toBe(true) - expect(registry.can('clarification.read', v)).toBe(true) - expect(registry.can('clarification.write', v)).toBe(true) + expect(registry.can('handoff.read', v)).toBe(true) + expect(registry.can('handoff.write', v)).toBe(true) expect(registry.can('workspace.write', v)).toBe(false) + expect(registry.can('workspace.manage', v)).toBe(false) expect(registry.can('history.write', v)).toBe(false) }) - it('guests get workspace.read but no proposal/clarifications access', () => { + it('guests read the workspace, but never reach the handoff queue', () => { const v = resolveViewer(makeUser('user'), makeMember('guest')) - expect(registry.can('workspace.read', v)).toBe(true) - expect(registry.can('proposal.read', v)).toBe(false) - expect(registry.can('proposal.write', v)).toBe(false) - expect(registry.can('clarification.read', v)).toBe(false) - expect(registry.can('clarification.write', v)).toBe(false) + expect(v.effectiveRole).toBe('guest') + expect(registry.can('handoff.read', v)).toBe(false) + expect(registry.can('handoff.write', v)).toBe(false) expect(registry.can('workspace.write', v)).toBe(false) + expect(registry.can('workspace.manage', v)).toBe(false) expect(registry.can('history.write', v)).toBe(false) }) it('outsiders (no member, not admin) get nothing', () => { const v = resolveViewer(makeUser('user'), undefined) - expect(registry.can('workspace.read', v)).toBe(false) - expect(registry.can('proposal.write', v)).toBe(false) + expect(registry.can('handoff.read', v)).toBe(false) + expect(registry.can('handoff.write', v)).toBe(false) + expect(registry.can('workspace.write', v)).toBe(false) + }) + + it('server scope reads serverRole, so a workspace owner is not a server admin', () => { + const owner = resolveViewer(makeUser('user'), makeMember('owner')) + expect(registry.can('server.write', owner)).toBe(false) + expect(registry.can('server.manage', owner)).toBe(false) + const admin = resolveViewer(makeUser('admin'), undefined) + expect(registry.can('server.write', admin)).toBe(true) + expect(registry.can('server.manage', admin)).toBe(true) }) it('admin who joined as guest still gets owner-level permissions', () => { const v = resolveViewer(makeUser('admin'), makeMember('guest')) expect(registry.can('workspace.write', v)).toBe(true) - expect(registry.can('proposal.write', v)).toBe(true) + expect(registry.can('workspace.manage', v)).toBe(true) + expect(registry.can('handoff.write', v)).toBe(true) expect(registry.can('history.write', v)).toBe(true) }) }) @@ -175,7 +183,7 @@ describe('PermissionRegistry', () => { it('register returns the registry for chaining', () => { const r = new PermissionRegistry() - expect(r.register({ id: 'workspace.read', evaluate: () => true })).toBe(r) - expect(r.has('workspace.read')).toBe(true) + expect(r.register({ id: 'workspace.write', evaluate: () => true })).toBe(r) + expect(r.has('workspace.write')).toBe(true) }) }) diff --git a/packages/server/test/routes/handoffAccess.test.ts b/packages/server/test/routes/handoffAccess.test.ts new file mode 100644 index 00000000..33aa7623 --- /dev/null +++ b/packages/server/test/routes/handoffAccess.test.ts @@ -0,0 +1,197 @@ +import type { User, WorkspaceId } from '@braidhq/schema' +import type { OpenAPIHono } from '@hono/zod-openapi' +import { mkdir, writeFile } from 'node:fs/promises' +import { join } from 'node:path' +import { REACTOR_USER_ID } from '@braidhq/core' +import { describe, expect, it } from 'vitest' +import { asUser, asUserJson, buildMultiUserApp } from '../helpers/multiUser.js' + +const COMMAND = 'command' + +interface IdList { + items: ReadonlyArray<{ id: string }> +} + +async function submitProposal( + app: OpenAPIHono, + workspaceId: WorkspaceId, + submitter: User, + nodeId: string, +): Promise { + const response = await app.request( + `/workspaces/${workspaceId}/proposals`, + asUserJson(submitter.id, 'POST', { + operations: [{ + operation: 'addNode', + payload: { + type: COMMAND, + name: nodeId, + id: nodeId, + metadata: { sourceReferences: [], missingRoles: ['code'] }, + }, + }], + generatedBy: 'extract', + rationale: `submitted by ${submitter.displayName}`, + }), + ) + expect(response.status).toBe(201) + return (await response.json() as { id: string }).id +} + +describe('handoff.read gates the queue', () => { + it('refuses a guest the proposal and clarification lists', async () => { + const { app, workspaceId, users } = await buildMultiUserApp() + + expect((await app.request(`/workspaces/${workspaceId}/proposals`, asUser(users.guest.id))).status).toBe(403) + expect((await app.request(`/workspaces/${workspaceId}/clarifications`, asUser(users.guest.id))).status).toBe(403) + }) + + it('lets a maintainer read the queue', async () => { + const { app, workspaceId, users } = await buildMultiUserApp() + + expect((await app.request(`/workspaces/${workspaceId}/proposals`, asUser(users.maintainer.id))).status).toBe(200) + }) +}) + +describe('one member\'s unsettled work stays theirs', () => { + it('reports another member\'s pending proposal as absent, rather than forbidden', async () => { + const { app, workspaceId, users } = await buildMultiUserApp() + const ownersProposal = await submitProposal(app, workspaceId, users.owner, 'owner-node') + + const asMaintainer = await app.request( + `/workspaces/${workspaceId}/proposals/${ownersProposal}`, + asUser(users.maintainer.id), + ) + expect(asMaintainer.status).toBe(404) + }) + + it('keeps it out of the maintainer\'s list too', async () => { + const { app, workspaceId, users } = await buildMultiUserApp() + await submitProposal(app, workspaceId, users.owner, 'owner-node') + const mine = await submitProposal(app, workspaceId, users.maintainer, 'maintainer-node') + + const response = await app.request(`/workspaces/${workspaceId}/proposals`, asUser(users.maintainer.id)) + const body = await response.json() as IdList + expect(body.items.map(item => item.id)).toEqual([mine]) + }) + + it('shows a maintainer what the reactor handed over, which is nobody\'s in particular', async () => { + const { app, workspaceId, users } = await buildMultiUserApp() + await submitProposal(app, workspaceId, users.owner, 'owner-node') + const autonomous = await submitProposal( + app, + workspaceId, + { ...users.owner, id: REACTOR_USER_ID, displayName: 'Reactor' }, + 'reactor-node', + ) + + const response = await app.request(`/workspaces/${workspaceId}/proposals`, asUser(users.maintainer.id)) + const body = await response.json() as IdList + expect(body.items.map(item => item.id)).toEqual([autonomous]) + }) + + it('lets whoever governs the workspace read it by id', async () => { + const { app, workspaceId, users } = await buildMultiUserApp() + const maintainersProposal = await submitProposal(app, workspaceId, users.maintainer, 'maintainer-node') + + const asOwner = await app.request( + `/workspaces/${workspaceId}/proposals/${maintainersProposal}`, + asUser(users.owner.id), + ) + expect(asOwner.status).toBe(200) + }) +}) + +describe('the coverage board never counts a queue its reader may not open', () => { + it('strips handoff ids for a guest, and keeps them for a maintainer', async () => { + const { app, workspaceId, users } = await buildMultiUserApp() + + const forGuest = await app.request(`/workspaces/${workspaceId}/coverage`, asUser(users.guest.id)) + expect(forGuest.status).toBe(200) + const guestBoard = await forGuest.json() as { + cards: ReadonlyArray<{ proposalIds: string[], clarificationIds: string[] }> + stages: ReadonlyArray<{ proposalIds: string[], clarificationIds: string[], answeredIds: string[] }> + } + for (const card of guestBoard.cards) { + expect(card.proposalIds).toEqual([]) + expect(card.clarificationIds).toEqual([]) + } + for (const stage of guestBoard.stages) { + expect(stage.proposalIds).toEqual([]) + expect(stage.clarificationIds).toEqual([]) + expect(stage.answeredIds).toEqual([]) + } + + expect((await app.request(`/workspaces/${workspaceId}/coverage`, asUser(users.maintainer.id))).status).toBe(200) + }) +}) + +describe('a batch is the per-unit skill run many times over', () => { + it('refuses a guest, who may run no skill here', async () => { + const { app, workspaceId, users } = await buildMultiUserApp() + + const response = await app.request( + `/workspaces/${workspaceId}/batch`, + asUserJson(users.guest.id, 'POST', { autoApply: false }), + ) + expect(response.status).toBe(403) + expect((await app.request( + `/workspaces/${workspaceId}/batch/stop`, + asUserJson(users.guest.id, 'POST'), + )).status).toBe(403) + }) +}) + +describe('writing a document is the generator skill run', () => { + it('lets every role read what was written', async () => { + const { app, workspaceId, users } = await buildMultiUserApp() + + for (const user of [users.owner, users.maintainer, users.guest]) + expect((await app.request(`/workspaces/${workspaceId}/views`, asUser(user.id))).status).toBe(200) + }) + + it('refuses a guest the write, who may run no generator here', async () => { + const { app, workspaceId, users } = await buildMultiUserApp() + + const response = await app.request( + `/workspaces/${workspaceId}/views`, + asUserJson(users.guest.id, 'POST', { kind: 'doc', form: 'reference', subject: 'ctx.any' }), + ) + expect(response.status).toBe(403) + }) + + it('lets a maintainer past the gate, so the subject is what answers next', async () => { + const { app, workspaceId, users } = await buildMultiUserApp() + + // The node does not exist, so a 404 about the subject is the point: + // it is the handler answering rather than the gate. + const response = await app.request( + `/workspaces/${workspaceId}/views`, + asUserJson(users.maintainer.id, 'POST', { kind: 'doc', form: 'reference', subject: 'ctx.any' }), + ) + expect(response.status).toBe(404) + }) +}) + +describe('replaying a run answers as fully as reading it', () => { + it('refuses a maintainer another member\'s transcript', async () => { + const { app, workspaceId, workspaceRootPath, users } = await buildMultiUserApp() + const dir = join(workspaceRootPath, 'artifacts', 'runs') + await mkdir(dir, { recursive: true }) + await writeFile(join(dir, 'index.jsonl'), `${JSON.stringify({ + runId: 'run-owner', + workspaceId, + skillId: 'braid:ask', + args: 'a question', + resumed: false, + startedAt: '2026-05-21T10:00:00.000Z', + startedBy: users.owner.id, + })}\n`, 'utf-8') + + const response = await app.request( + `/workspaces/${workspaceId}/agui/runs/run-owner`, + asUser(users.maintainer.id), + ) + expect(response.status).toBe(404) + }) +}) diff --git a/packages/server/test/routes/views.test.ts b/packages/server/test/routes/views.test.ts index a3447c40..620a314f 100644 --- a/packages/server/test/routes/views.test.ts +++ b/packages/server/test/routes/views.test.ts @@ -1,4 +1,4 @@ -import type { GenerateViewInput, ViewService } from '@braidhq/core' +import type { GenerateViewInput, SkillRegistry, ViewService, WorkspaceRepository } from '@braidhq/core' import type { EmittedBlock, GeneratedView, GenerateViewResponse, ViewContent } from '@braidhq/schema' import { NotFoundError, ValidationError } from '@braidhq/core' import { FIXTURE_FORM, FIXTURE_FORMAT, FIXTURE_KIND, makeGeneratedView, makeViewForm, makeViewKind } from '@braidhq/test-utils' @@ -40,7 +40,13 @@ function viewService(overrides: Partial = {}): ViewService { function app(service: ViewService) { const scoped = new OpenAPIHono() scoped.use('*', workspaceIdMiddleware) - scoped.route('/views', createViewsRouter({ viewService: service })) + // No membership middleware here, so `requirePermission` skips its gate, + // and the resource builder these two would serve is never called. + scoped.route('/views', createViewsRouter({ + viewService: service, + skillRegistry: {} as unknown as SkillRegistry, + workspaceRepository: {} as unknown as WorkspaceRepository, + })) const root = new OpenAPIHono() root.onError(errorHandler) root.route('/workspaces/:workspaceId', scoped) diff --git a/packages/studio/src/App.tsx b/packages/studio/src/App.tsx index 850c07a5..57265165 100644 --- a/packages/studio/src/App.tsx +++ b/packages/studio/src/App.tsx @@ -22,7 +22,7 @@ import { TooltipProvider } from './components/ui/tooltip' import { UserPicker } from './components/UserPicker' import { WorkspaceDetailsSheet } from './components/WorkspaceDetailsSheet' import { asNodeId } from './lib/brands' -import { useLandingSurface } from './lib/landingSurface' +import { useLandingSurface, useSurfaceReach } from './lib/landingSurface' import { useBatchStatus, useReactorCycles, useWorkspaces } from './lib/queries' import { useAuthGate } from './lib/useAuthGate' import { GraphNavigationContext } from './lib/useGraphNavigation' @@ -35,13 +35,11 @@ import { ActivityPage } from './pages/Activity' import { AskPage } from './pages/Ask' import { BatchPage } from './pages/Batch' import { BuildPage } from './pages/Build' -import { ClarificationPage } from './pages/Clarification' import { DocumentsPage } from './pages/Documents' import { GraphSurface, GraphSurfaceActions, useGraphSurfaceState } from './pages/GraphSurface' import { HistoryPage } from './pages/History' import { InboxPage } from './pages/Inbox' import { LoginPage } from './pages/Login' -import { ProposalsPage } from './pages/Proposals' import { SettingsPage } from './pages/Settings' const NO_ARRIVAL: readonly NodeId[] = Object.freeze([]) @@ -101,6 +99,7 @@ function AppInner() { // Waiting for the answer beats landing on Graph and jumping a moment later, // and the reader keeps whatever they choose from here, including Graph. const landing = useLandingSurface(activeId) + const reaches = useSurfaceReach(activeId) useEffect(() => { if (activeSurface === null && landing !== undefined) setActiveSurface(landing) @@ -209,7 +208,7 @@ function AppInner() { setActiveSurface('batch')} + {...(reaches('batch') ? { onStartBootstrap: () => setActiveSurface('batch') } : {})} onOpenSearch={() => setPaletteOpen(true)} /> )} @@ -232,16 +231,6 @@ function AppInner() { {activeSurface === 'documents' && ( )} - {activeSurface === 'clarifications' && ( - - )} - {activeSurface === 'proposals' && ( - setFocusedProposalId(null)} - /> - )} {activeSurface === 'activity' && ( )} @@ -305,7 +294,7 @@ function AppInner() { function GraphHomeView({ workspaceId, state, onStartBootstrap, onOpenSearch }: { workspaceId: string state: ReturnType - onStartBootstrap: () => void + onStartBootstrap?: (() => void) | undefined onOpenSearch: () => void }) { const { t } = useTranslation() @@ -365,7 +354,7 @@ function GraphHomeView({ workspaceId, state, onStartBootstrap, onOpenSearch }: { focusMode={focusMode} centerRequest={centerRequest} onOpenSearch={onOpenSearch} - onStartBootstrap={onStartBootstrap} + {...(onStartBootstrap ? { onStartBootstrap } : {})} /> @@ -387,13 +376,9 @@ function WorkspaceHeader({ workspaceId, activeSurface, onOpenDetails }: { ? t('shell.surfaces.documents') : activeSurface === 'actions' ? t('shell.surfaces.actions') - : activeSurface === 'clarifications' - ? t('shell.surfaces.clarifications') - : activeSurface === 'proposals' - ? t('shell.surfaces.proposals') - : activeSurface === 'history' - ? t('shell.surfaces.history') - : null + : activeSurface === 'history' + ? t('shell.surfaces.history') + : null return (
diff --git a/packages/studio/src/components/CommandPalette.tsx b/packages/studio/src/components/CommandPalette.tsx index c21f8e4f..293cee79 100644 --- a/packages/studio/src/components/CommandPalette.tsx +++ b/packages/studio/src/components/CommandPalette.tsx @@ -1,5 +1,5 @@ import type { NodeId, SkillManifest, Workspace } from '@braidhq/schema' -import { Activity, Boxes, ClipboardCheck, FileText, GitGraph, HelpCircle, Inbox, MessageCircleQuestion, Network, Settings, Settings2, Sparkles } from 'lucide-react' +import { Activity, Boxes, FileText, GitGraph, Inbox, MessageCircleQuestion, Network, Settings, Settings2, Sparkles } from 'lucide-react' import { useEffect, useRef, useState } from 'react' import { useTranslation } from 'react-i18next' import { @@ -11,6 +11,7 @@ import { CommandList, CommandShortcut, } from '@/components/ui/command' +import { useSurfaceReach } from '@/lib/landingSurface' import { useNodeSearch, useSkills } from '@/lib/queries' import { useDebounced } from '@/lib/useDebounced' import { useEmbeddingProgress } from '@/lib/useEmbeddingProgress' @@ -44,12 +45,10 @@ export const SURFACES = [ 'ask', 'batch', 'build', - 'clarifications', 'documents', 'graph', 'history', 'inbox', - 'proposals', 'settings', ] as const @@ -57,12 +56,23 @@ export type Surface = typeof SURFACES[number] type ChordTarget = { kind: 'surface', surface: Surface | null } | { kind: 'workspace-details' } +/** + * A chord goes to a surface the sidebar keeps a row for, and to nothing else. + * + * Held to that, every one of them is its own first letter, + * which is the only mapping a reader can guess. + * Widening it past the sidebar would not survive the second letter, + * since Ask, Actions, and Activity all open with the same one, + * as do Build and Batch. + * Those three are reached by name in this palette, + * and each already has a way in of its own. + */ function chordSecondKey(key: string): ChordTarget | undefined { switch (key) { case 'g': return { kind: 'surface', surface: 'graph' } - case 'a': return { kind: 'surface', surface: 'actions' } + case 'a': return { kind: 'surface', surface: 'ask' } + case 'b': return { kind: 'surface', surface: 'build' } case 'i': return { kind: 'surface', surface: 'inbox' } - case 'b': return { kind: 'surface', surface: 'activity' } case 'd': return { kind: 'surface', surface: 'documents' } case 'h': return { kind: 'surface', surface: 'history' } case 's': return { kind: 'surface', surface: 'settings' } @@ -72,24 +82,24 @@ function chordSecondKey(key: string): ChordTarget | undefined { } // `as const` keeps labelKey literal so t() validates each against the typed catalog. +// A shortcut is printed only where `chordSecondKey` answers it, +// so the hint never promises a key that does nothing. const SURFACE_ITEMS = [ { id: null, labelKey: 'shell.commandPalette.graphHome', Icon: Network, shortcut: 'G G' }, - { id: 'ask', labelKey: 'shell.surfaces.ask', Icon: MessageCircleQuestion, shortcut: 'G Q' }, + { id: 'ask', labelKey: 'shell.surfaces.ask', Icon: MessageCircleQuestion, shortcut: 'G A' }, { id: 'build', labelKey: 'shell.surfaces.build', Icon: Boxes, shortcut: 'G B' }, { id: 'inbox', labelKey: 'shell.surfaces.inbox', Icon: Inbox, shortcut: 'G I' }, { id: 'documents', labelKey: 'shell.surfaces.documents', Icon: FileText, shortcut: 'G D' }, + { id: 'history', labelKey: 'shell.surfaces.history', Icon: GitGraph, shortcut: 'G H' }, + { id: 'settings', labelKey: 'shell.surfaces.settings', Icon: Settings, shortcut: 'G S' }, // Everything below folds into a surface above. // Each reaches the same records once they are settled, // which is browsing rather than working, - // so they keep a way in without taking a sidebar row for it. - { id: 'actions', labelKey: 'shell.surfaces.actions', Icon: Sparkles, shortcut: 'G A' }, - { id: 'clarifications', labelKey: 'shell.surfaces.clarifications', Icon: HelpCircle, shortcut: 'G C' }, - { id: 'proposals', labelKey: 'shell.surfaces.proposals', Icon: ClipboardCheck, shortcut: 'G P' }, - { id: 'activity', labelKey: 'shell.surfaces.activity', Icon: Activity, shortcut: 'G R' }, - { id: 'batch', labelKey: 'shell.surfaces.batch', Icon: Boxes, shortcut: 'G T' }, - { id: 'history', labelKey: 'shell.surfaces.history', Icon: GitGraph, shortcut: 'G H' }, - { id: 'settings', labelKey: 'shell.surfaces.settings', Icon: Settings, shortcut: 'G S' }, -] as const satisfies readonly { id: Surface | null, labelKey: string, Icon: typeof Sparkles, shortcut: string }[] + // so they keep a way in without taking a sidebar row or a chord for it. + { id: 'actions', labelKey: 'shell.surfaces.actions', Icon: Sparkles }, + { id: 'activity', labelKey: 'shell.surfaces.activity', Icon: Activity }, + { id: 'batch', labelKey: 'shell.surfaces.batch', Icon: Boxes }, +] as const satisfies readonly { id: Surface | null, labelKey: string, Icon: typeof Sparkles, shortcut?: string }[] function isTypingTarget(target: EventTarget | null): boolean { if (!(target instanceof HTMLElement)) @@ -123,6 +133,7 @@ export function CommandPalette({ debouncedQuery, ) const { data: skillData } = useSkills(activeWorkspaceId ?? undefined) + const reaches = useSurfaceReach(activeWorkspaceId) const { rebuilding } = useEmbeddingProgress(open ? activeWorkspaceId : null) useEffect(() => { @@ -170,11 +181,14 @@ export function CommandPalette({ } if (target.surface !== 'settings' && !activeWorkspaceId) return + // A chord is a shortcut to a surface, never a way around it. + if (!reaches(target.surface)) + return onSelectSurface(target.surface) } window.addEventListener('keydown', onKey) return () => window.removeEventListener('keydown', onKey) - }, [activeWorkspaceId, onSelectSurface, onOpenWorkspaceDetails]) + }, [activeWorkspaceId, onSelectSurface, onOpenWorkspaceDetails, reaches]) const skills = (skillData?.items ?? []).filter((s: SkillManifest) => !s.frontmatter.braid.hidden) const nodes = nodeData?.items ?? [] @@ -219,20 +233,24 @@ export function CommandPalette({ )} - {SURFACE_ITEMS.map(({ id, labelKey, Icon, shortcut }) => ( - { - onSelectSurface(id) - setOpen(false) - }} - disabled={(id !== 'settings' && !activeWorkspaceId) || activeSurface === id} - > - - {t(labelKey)} - {shortcut} - - ))} + {SURFACE_ITEMS.filter(({ id }) => reaches(id)).map((item) => { + const { id, labelKey, Icon } = item + const shortcut = 'shortcut' in item ? item.shortcut : undefined + return ( + { + onSelectSurface(id) + setOpen(false) + }} + disabled={(id !== 'settings' && !activeWorkspaceId) || activeSurface === id} + > + + {t(labelKey)} + {shortcut && {shortcut}} + + ) + })} {activeWorkspaceId && ( - !s.frontmatter.braid.hidden && policy.can('skill.run', { skill: s.frontmatter, skillId: s.id }), - ) - const canAsk = (skills?.items ?? []).some(s => - s.frontmatter.braid.category === 'ask' - && !s.frontmatter.braid.hidden - && policy.can('skill.run', { skill: s.frontmatter, skillId: s.id }), - ) - const canSeeHistory = policy.effectiveRole !== null && policy.effectiveRole !== 'guest' + const reaches = useSurfaceReach(workspaceId) const inFlight = useRunsInFlight(workspaceId, skills?.items ?? []) return ( @@ -542,14 +531,14 @@ function HereSection({ )}
    - {canAsk && ( + {reaches('ask') && ( onSelectSurface('ask')} /> )} @@ -561,7 +550,7 @@ function HereSection({ shortcut="G G" onClick={onGoHome} /> - {canRunActions && ( + {reaches('build') && ( onSelectSurface('build')} /> )} - {(canSeeClarification || canSeeProposals) && ( + {reaches('inbox') && ( onSelectSurface('inbox')} /> )} - onSelectSurface('documents')} - /> - {canSeeHistory && ( + {reaches('documents') && ( + onSelectSurface('documents')} + /> + )} + {reaches('history') && ( { invalidate() onRenamed(newId) @@ -83,7 +84,7 @@ function Body({ workspaceId, onUnregistered, onRenamed }: { />
    - setAddSourceOpen(true)} addLabel={t('workspace.details.addSource')} /> + setAddSourceOpen(true)} addLabel={t('workspace.details.addSource')} disabled={!canWrite} /> {workspace.productManifest.mcpServers.map(server => ( - + ))}
)} @@ -132,6 +133,7 @@ function Body({ workspaceId, onUnregistered, onRenamed }: {
{ invalidate() @@ -190,12 +192,17 @@ function AutoRefreshSwitch({ workspaceId, enabled, canWrite, onChange }: { ) } -function SectionHeader({ title, onAdd, addLabel }: { title: string, onAdd?: (() => void) | undefined, addLabel?: string | undefined }) { +function SectionHeader({ title, onAdd, addLabel, disabled = false }: { + title: string + onAdd?: (() => void) | undefined + addLabel?: string | undefined + disabled?: boolean +}) { return (

{title}

{onAdd && ( - )} @@ -203,7 +210,7 @@ function SectionHeader({ title, onAdd, addLabel }: { title: string, onAdd?: (() ) } -function RenameSection({ workspace, onRenamed }: { workspace: Workspace, onRenamed: (newId: string) => void }) { +function RenameSection({ workspace, canWrite, onRenamed }: { workspace: Workspace, canWrite: boolean, onRenamed: (newId: string) => void }) { const { t } = useTranslation() const [name, setName] = useState(workspace.productManifest.name) const [description, setDescription] = useState(workspace.productManifest.description ?? '') @@ -223,7 +230,7 @@ function RenameSection({ workspace, onRenamed }: { workspace: Workspace, onRenam
- setName(e.target.value)} /> + setName(e.target.value)} />
{patch.error &&

{humaniseApiError(patch.error)}

}
-
@@ -296,13 +303,14 @@ function SourceRow({ workspaceId, source, paused, onChange }: { )} -

{detail}

{parts.length === 0 ? t('workspace.details.noChangeLabel') : parts.join(' ')} } -function McpRow({ workspaceId, server, onChange }: { +function McpRow({ workspaceId, server, canWrite, onChange }: { workspaceId: string server: McpServerConfig + canWrite: boolean onChange: () => void }) { const { t } = useTranslation() @@ -701,6 +710,7 @@ function McpRow({ workspaceId, server, onChange }: { {server.transport === 'stdio' ? `${server.command}${server.args ? ` ${server.args.join(' ')}` : ''}` : server.url}

+ {stored ?? {emptyHint}} +

+ ) + } return ( ) diff --git a/packages/studio/src/pages/Clarification.tsx b/packages/studio/src/components/handoff/ClarificationDetail.tsx similarity index 74% rename from packages/studio/src/pages/Clarification.tsx rename to packages/studio/src/components/handoff/ClarificationDetail.tsx index 646d61af..e468663f 100644 --- a/packages/studio/src/pages/Clarification.tsx +++ b/packages/studio/src/components/handoff/ClarificationDetail.tsx @@ -1,23 +1,16 @@ -import type { Clarification, ClarificationCandidate, ClarificationStatus, ExternalReference, GraphOperation, NodeId, ProposalId } from '@braidhq/schema' +import type { Clarification, ClarificationCandidate, ExternalReference, GraphOperation, NodeId, ProposalId } from '@braidhq/schema' import { useMutation, useQueryClient } from '@tanstack/react-query' -import { Check, Clock, ExternalLink, Inbox, Pencil, Plus, SkipForward, X } from 'lucide-react' -import { useEffect, useState } from 'react' +import { Check, Clock, ExternalLink, Pencil, SkipForward, X } from 'lucide-react' +import { useState } from 'react' import { useTranslation } from 'react-i18next' -import { EmptyState } from '@/components/EmptyState' -import { ListRow } from '@/components/ListRow' -import { PageActions } from '@/components/PageActions' import { MentionTextarea } from '@/components/references/MentionTextarea' import { NodeReferenceTag } from '@/components/references/ReferenceTag' import { ReferenceText } from '@/components/references/ReferenceText' import { StatusBadge } from '@/components/StatusBadge' -import { SubmitIssueForm } from '@/components/SubmitIssueForm' -import { SurfaceLayout } from '@/components/SurfaceLayout' import { Badge } from '@/components/ui/badge' import { Button } from '@/components/ui/button' -import { FILTER_TAB_TRIGGER, FILTER_TABS_LIST } from '@/components/ui/filterTabs' -import { Tabs, TabsList, TabsTrigger } from '@/components/ui/tabs' import { api } from '@/lib/api' -import { queryKeys, useClarificationByStatus, useClarificationDetail, usePendingClarification, useWorkspaceMembers } from '@/lib/queries' +import { queryKeys, useClarificationDetail } from '@/lib/queries' import { useTabNavigation } from '@/lib/useTabNavigation' import { useWorkspacePolicy } from '@/policy' @@ -105,255 +98,6 @@ export function questionExcerpt(question: string, max = 80): string { return `${trimmed.slice(0, max - 1)}…` } -interface ClarificationPageProps { - workspaceId: string -} - -type StatusFilter = ClarificationStatus - -export function ClarificationPage({ workspaceId }: ClarificationPageProps) { - const { t } = useTranslation() - // Same shape as Proposals. Status drives the list query, - // and the detail pane reads only from the selected ticket, - // so it cannot show an item that no longer matches the active filter. - const [status, setStatus] = useState('pending') - const [showAll, setShowAll] = useState(false) - const [selected, setSelected] = useState(null) - // When `true`, the detail pane renders the inline SubmitIssueForm, - // instead of the selected ticket. - // Mutually exclusive with `selected`, - // the compose surface fills the same area, - // so the reviewer is never doing two things at once. - const [composing, setComposing] = useState(false) - const { data, isLoading } = useClarificationByStatus(workspaceId, status, showAll) - - // Auto-select the first ticket when entering a list with no selection, - // on initial mount, after a status switch, - // or after answer or skip clears the detail pane. - // Saves the reviewer one click per ticket when working through a queue. - useEffect(() => { - if (composing || selected || isLoading || !data?.items.length) - return - setSelected(data.items[0]!) - }, [data, selected, isLoading, composing]) - - function changeStatus(next: StatusFilter): void { - setStatus(next) - setSelected(null) - setComposing(false) - } - - function startComposing(): void { - setSelected(null) - setComposing(true) - } - - // The "submit an issue" affordance is meaningful only on the Pending tab, - // the other statuses are post-resolution archives. - // Keeping it pending-only also reduces surprise, - // you would never expect to file a new issue while browsing rejected ones. - const canSubmitIssue = status === 'pending' - - return ( -
- - - - - - {isLoading - ? ( -
{t('common.loading')}
- ) - : ( - <> - {data && data.items.length > 0 && ( -
    - {data.items.map(ticket => ( - { - setComposing(false) - setSelected(ticket) - }} - /> - ))} -
- )} - {canSubmitIssue && ( - - )} - - )} - - )} - > -
- {composing - ? ( - setComposing(false)} - onSubmitted={(ticket) => { - // SSE clarification.created already invalidates the list. - // Selecting the new ticket also dismisses compose mode, - // so the reviewer lands on the freshly filed issue. - setComposing(false) - setStatus('pending') - setSelected(ticket) - }} - /> - ) - : selected - ? ( - setSelected(null)} - key={selected.id} - /> - ) - : ( - - )} -
- -
- ) -} - -/** - * Header strip: status filter with a live badge on pending. - * The "submit a question" affordance lives inline, - * at the bottom of the list panel rather than here, - * so the header stays focused on navigation. - */ -function ClarificationShowAllToggle({ - workspaceId, - status, - showAll, - onToggle, -}: { - workspaceId: string - status: StatusFilter - showAll: boolean - onToggle: (next: boolean) => void -}) { - const { t } = useTranslation() - const { effectiveRole } = useWorkspacePolicy(workspaceId) - const { data: members } = useWorkspaceMembers(workspaceId) - // Nothing to disambiguate on a solo workspace, every question is yours. - const multiMember = (members?.items.length ?? 0) > 1 - if (effectiveRole !== 'owner' || status !== 'pending' || !multiMember) - return null - return ( - - ) -} - -function ClarificationHeaderActions({ - workspaceId, - status, - onChange, -}: { - workspaceId: string - status: StatusFilter - onChange: (next: StatusFilter) => void -}) { - const { t } = useTranslation() - const { data: pending } = usePendingClarification(workspaceId) - const pendingCount = pending?.items.length ?? 0 - return ( - onChange(value as StatusFilter)}> - - - {t('review.clarify.tabPending')} - {pendingCount > 0 && ( - - {pendingCount} - - )} - - {t('review.clarify.tabAnswered')} - {t('review.clarify.tabApplied')} - {t('review.clarify.tabSkipped')} - - - ) -} - -function ClarificationListItem({ - ticket, - active, - onSelect, -}: { - ticket: Clarification - active: boolean - onSelect: () => void -}) { - const { t } = useTranslation() - return ( - -
- - {questionExcerpt(ticket.question, 90)} - - -
-
- - {t('review.clarify.candidateCount', { count: ticket.candidates.length })} - - {ticket.proposalId && ( - - → - {' '} - {ticket.proposalId} - - )} -
- - ) -} - export function ClarificationDetail({ workspaceId, ticket, @@ -378,7 +122,7 @@ export function ClarificationDetail({ }) { const { t } = useTranslation() const queryClient = useQueryClient() - const canWrite = useWorkspacePolicy(workspaceId).can('clarification.write') + const canWrite = useWorkspacePolicy(workspaceId).can('handoff.write') const isPending = ticket.status === 'pending' // The two answer paths are mutually exclusive. // Picking an existing candidate closes the custom-answer form, diff --git a/packages/studio/src/pages/Proposals.tsx b/packages/studio/src/components/handoff/ProposalDetail.tsx similarity index 74% rename from packages/studio/src/pages/Proposals.tsx rename to packages/studio/src/components/handoff/ProposalDetail.tsx index da2438d1..0329b4c6 100644 --- a/packages/studio/src/pages/Proposals.tsx +++ b/packages/studio/src/components/handoff/ProposalDetail.tsx @@ -1,245 +1,21 @@ -import type { EdgeId, GraphEdgeCreate, GraphNodeCreate, GraphOperation, NodeId, Proposal, ProposalId, ProposalStatus, ValidationIssue, ValidationSeverity } from '@braidhq/schema' +import type { EdgeId, GraphEdgeCreate, GraphNodeCreate, GraphOperation, NodeId, Proposal, ValidationIssue, ValidationSeverity } from '@braidhq/schema' import { useMutation, useQueryClient } from '@tanstack/react-query' -import { AlertCircle, AlertTriangle, Check, ChevronDown, ChevronRight, Inbox, Info, MinusCircle, PencilLine, PlusCircle, X } from 'lucide-react' -import { useEffect, useMemo, useState } from 'react' +import { AlertCircle, AlertTriangle, Check, ChevronDown, ChevronRight, Info, MinusCircle, PencilLine, PlusCircle, X } from 'lucide-react' +import { useMemo, useState } from 'react' import { useTranslation } from 'react-i18next' -import { EmptyState } from '@/components/EmptyState' import { emphasizeAddedFor, narrowToChanges, useProposalGraphDataSource } from '@/components/graph/GraphDataSource' import { FocusToggle, OnlyChangesToggle } from '@/components/graph/GraphToolbar' import { useFocusedSelection } from '@/components/graph/useFocusedSelection' -import { ListRow } from '@/components/ListRow' -import { PageActions } from '@/components/PageActions' import { NodeReferenceTag } from '@/components/references/ReferenceTag' import { ReferenceText } from '@/components/references/ReferenceText' import { StatusBadge } from '@/components/StatusBadge' -import { SurfaceLayout } from '@/components/SurfaceLayout' import { Badge } from '@/components/ui/badge' import { Button } from '@/components/ui/button' -import { FILTER_TAB_TRIGGER, FILTER_TABS_LIST } from '@/components/ui/filterTabs' -import { Tabs, TabsList, TabsTrigger } from '@/components/ui/tabs' import { api } from '@/lib/api' -import { queryKeys, useProposalsByStatus, useProposalValidation, useWorkspaceMembers } from '@/lib/queries' +import { queryKeys, useProposalValidation } from '@/lib/queries' import { useGraphNavigation } from '@/lib/useGraphNavigation' +import { GraphSurface } from '@/pages/GraphSurface' import { useWorkspacePolicy } from '@/policy' -import { GraphSurface } from './GraphSurface' - -interface ProposalsPageProps { - workspaceId: string - /** - * One-shot deep-link target. - * When set, such as clicking "Proposal #abc" on an applied Clarification, - * the page scans its current list for the matching proposal. - * If found in the current status filter, it is selected, - * otherwise the page sweeps the other statuses, - * and switches the filter to wherever the proposal actually lives. - */ - focusedProposalId?: ProposalId | null - onFocusConsumed?: () => void -} - -type StatusFilter = Extract - -export function ProposalsPage({ workspaceId, focusedProposalId, onFocusConsumed }: ProposalsPageProps) { - const { t } = useTranslation() - // Status filter is both the list query and the detail pane's read-only cue. - // Switching status clears the selected proposal, - // so the right pane cannot show an item that no longer matches. - const [status, setStatus] = useState('pending') - const [showAll, setShowAll] = useState(false) - const { data, isLoading } = useProposalsByStatus(workspaceId, status, showAll) - const [selected, setSelected] = useState(null) - // Tracks an in-progress sweep across statuses for a deep-link focus. - // Each entry remembers which status filters we've already checked, - // so we don't loop on an id that doesn't exist in any list. - const [focusSweep, setFocusSweep] = useState<{ proposalId: ProposalId, attempted: Set } | null>(null) - - function changeStatus(next: StatusFilter): void { - setStatus(next) - setSelected(null) - } - - // Seed the sweep when a new focusedProposalId arrives. - // Status is intentionally excluded from deps, - // this effect must fire only on the externally driven id change, - // not when the user is mid-sweep switching filters. - useEffect(() => { - if (focusedProposalId) - setFocusSweep(prev => prev?.proposalId === focusedProposalId ? prev : { proposalId: focusedProposalId, attempted: new Set([status]) }) - }, [focusedProposalId, status]) - - // Drive the sweep. Try the current list, - // if no match advance to the next unchecked status. - // Consumes the focus once we select the proposal or exhaust the statuses. - useEffect(() => { - if (!focusSweep || isLoading || !data) - return - const match = data.items.find(p => p.id === focusSweep.proposalId) - if (match) { - setSelected(match) - setFocusSweep(null) - onFocusConsumed?.() - return - } - const candidates: StatusFilter[] = ['pending', 'applied', 'rejected'] - const next = candidates.find(s => !focusSweep.attempted.has(s)) - if (!next) { - setFocusSweep(null) - onFocusConsumed?.() - return - } - setStatus(next) - setFocusSweep({ proposalId: focusSweep.proposalId, attempted: new Set([...focusSweep.attempted, next]) }) - }, [focusSweep, data, isLoading, onFocusConsumed]) - - // Auto-select the first item when entering a list with no selection. - // Covers initial mount, status switch, and complete-and-clear from detail. - // Skip while a deep-link focus sweep is in flight, - // so we do not race the sweep's setSelected call. - useEffect(() => { - if (focusSweep || selected || isLoading || !data?.items.length) - return - setSelected(data.items[0]!) - }, [data, selected, isLoading, focusSweep]) - - return ( -
- - - - - - {isLoading - ? ( -
{t('common.loading')}
- ) - : !data || data.items.length === 0 - ? null - : ( -
    - {data.items.map(proposal => ( - setSelected(proposal)} - className="flex-col gap-1" - > -
    - {proposal.id} - -
    -
    - {t('review.proposals.operationsBy', { count: proposal.operations.length, name: proposal.generatedBy })} -
    -     - ))} -
- )} - - )} - > -
- {selected - ? ( - setSelected(null)} - key={selected.id} - /> - ) - : ( - - )} -
- -
- ) -} - -/** - * Owner-only toggle that flips the personal-pending filter to "everyone's". - * Only rendered on the pending tab, - * applied and rejected lists are shared by definition, - * so a toggle there would do nothing. - * Cmd-click suppression keeps it small and tucked next to the status tabs. - */ -function ShowAllToggle({ - workspaceId, - status, - showAll, - onToggle, -}: { - workspaceId: string - status: StatusFilter - showAll: boolean - onToggle: (next: boolean) => void -}) { - const { t } = useTranslation() - const { effectiveRole } = useWorkspacePolicy(workspaceId) - const { data: members } = useWorkspaceMembers(workspaceId) - // Nothing to disambiguate on a solo workspace, every proposal is yours. - const multiMember = (members?.items.length ?? 0) > 1 - if (effectiveRole !== 'owner' || status !== 'pending' || !multiMember) - return null - return ( - - ) -} - -// Pending, Applied, Rejected segment. -// Rendered via PageActions into the top tab row, -// so it takes no row of its own. -// Pending wears a live count badge, the only one worth surfacing. -// Applied and rejected lists grow monotonically, a count there is noise. -function ProposalsStatusFilter({ - workspaceId, - status, - onChange, -}: { - workspaceId: string - status: StatusFilter - onChange: (next: StatusFilter) => void -}) { - const { t } = useTranslation() - const { data: pending } = useProposalsByStatus(workspaceId, 'pending') - const pendingCount = pending?.items.length ?? 0 - return ( - onChange(value as StatusFilter)}> - - - {t('review.proposals.tabPending')} - {pendingCount > 0 && ( - - {pendingCount} - - )} - - {t('review.proposals.tabApplied')} - {t('review.proposals.tabRejected')} - - - ) -} export function ProposalDetail({ workspaceId, @@ -258,7 +34,7 @@ export function ProposalDetail({ // Apply and Reject are only meaningful while the proposal is pending. // Applied and rejected entries are read-only history. const isPending = proposal.status === 'pending' - const canWrite = useWorkspacePolicy(workspaceId).can('proposal.write') + const canWrite = useWorkspacePolicy(workspaceId).can('handoff.write') const validation = useProposalValidation(workspaceId, isPending ? proposal.id : null) const errorCount = validation.data?.issues.filter(issue => issue.severity === 'error').length ?? 0 diff --git a/packages/studio/src/lib/landingSurface.ts b/packages/studio/src/lib/landingSurface.ts index 466060ce..fdc31c63 100644 --- a/packages/studio/src/lib/landingSurface.ts +++ b/packages/studio/src/lib/landingSurface.ts @@ -18,6 +18,26 @@ export function useCanAsk(workspaceId: string | null): boolean { ) } +/** + * Whether this reader may have a document written, or written again. + * + * Writing one is the form's generator skill run on a projected subject, + * so it is the same question asked of the skills that write, + * rather than of the surface that shows what they wrote. + * Reading a document asks nothing, since it is a reading of the graph. + * + * Hidden skills count here. A generator is hidden because nobody picks it + * off the Actions list, not because nobody runs it. + */ +export function useCanWriteDocuments(workspaceId: string | null): boolean { + const policy = useWorkspacePolicy(workspaceId) + const { data: skills } = useSkills(workspaceId ?? undefined) + return (skills?.items ?? []).some(skill => + skill.frontmatter.braid.category === 'generate' + && policy.can('skill.run', { skill: skill.frontmatter, skillId: skill.id }), + ) +} + /** * Where a bare workspace url puts a reader. * @@ -37,3 +57,51 @@ export function useLandingSurface(workspaceId: string | null): Surface | undefin return undefined return canAsk && snapshot.nodes.length > 0 ? 'ask' : 'graph' } + +/** + * Which surfaces this reader may open. + * + * The sidebar, the command palette, and the `g` chords all ask this, + * and a shortcut reaching what the sidebar hides is a hole, + * rather than a convenience. + * + * Build is open to every member, because its board says what the model has + * made of each document, which is a reading of the graph. + * What it takes a capability to do there is gated inside the surface. + */ +export function useSurfaceReach(workspaceId: string | null): (surface: Surface | null) => boolean { + const policy = useWorkspacePolicy(workspaceId) + const { data: skills } = useSkills(workspaceId ?? undefined) + const canAsk = useCanAsk(workspaceId) + const canRunSkills = (skills?.items ?? []).some(skill => + !skill.frontmatter.braid.hidden + && policy.can('skill.run', { skill: skill.frontmatter, skillId: skill.id }), + ) + const readsHandoffs = policy.can('handoff.read') + const isMember = policy.effectiveRole !== null + + return (surface) => { + switch (surface) { + case 'settings': + return true + case null: + case 'graph': + case 'history': + case 'activity': + case 'build': + case 'documents': + return isMember + case 'ask': + return canAsk + case 'actions': + case 'batch': + return canRunSkills + case 'inbox': + return readsHandoffs + default: { + const exhaustive: never = surface + throw new Error(`Unhandled surface: ${JSON.stringify(exhaustive)}`) + } + } + } +} diff --git a/packages/studio/src/locales/en/inbox.ts b/packages/studio/src/locales/en/inbox.ts index 8ad187b2..4e75ed39 100644 --- a/packages/studio/src/locales/en/inbox.ts +++ b/packages/studio/src/locales/en/inbox.ts @@ -14,6 +14,11 @@ export const inbox = { answeredWaiting: '{count} answered, waiting for the step that reads them.', transcript: 'Run record ({count} events)', untitled: 'no description', + showingAll: 'Showing All', + mineOnly: 'Mine Only', + showingAllTooltip: 'Showing what every member is waiting on', + mineOnlyTooltip: 'Showing only yours, and what the system raised', + bySystem: 'System', } export default inbox diff --git a/packages/studio/src/locales/zh-Hant/inbox.ts b/packages/studio/src/locales/zh-Hant/inbox.ts index 47ce942e..8f5bce68 100644 --- a/packages/studio/src/locales/zh-Hant/inbox.ts +++ b/packages/studio/src/locales/zh-Hant/inbox.ts @@ -14,6 +14,11 @@ export const inbox = { answeredWaiting: '{count} 題已回答，等待讀取它們的步驟執行。', transcript: '原始執行紀錄 ({count} 筆事件)', untitled: '沒有說明', + showingAll: '顯示全部', + mineOnly: '僅顯示我的', + showingAllTooltip: '顯示所有成員待處理的項目', + mineOnlyTooltip: '僅顯示你自己的，以及系統提出的', + bySystem: '系統', } export default inbox diff --git a/packages/studio/src/pages/Documents.tsx b/packages/studio/src/pages/Documents.tsx index dc2f0020..12b3c26e 100644 --- a/packages/studio/src/pages/Documents.tsx +++ b/packages/studio/src/pages/Documents.tsx @@ -15,6 +15,7 @@ import { Input } from '@/components/ui/input' import { api } from '@/lib/api' import { EvidenceDetailContext, WorkspaceScopeContext } from '@/lib/blocks/WorkspaceScopeContext' import { useLocale } from '@/lib/i18n' +import { useCanWriteDocuments } from '@/lib/landingSurface' import { queryKeys, useModelSnapshot, useOntology, useView, useViewKinds, useViews } from '@/lib/queries' import { runStore } from '@/lib/runStore' import { useRun } from '@/lib/useRun' @@ -95,6 +96,7 @@ export function DocumentsPage({ workspaceId, onSelectNode }: { } const busy = runId !== null + const canWrite = useCanWriteDocuments(workspaceId) // Three situations read as one blank column, // and each has a different next move. @@ -116,7 +118,7 @@ export function DocumentsPage({ workspaceId, onSelectNode }: { variant="ghost" size="xs" className="[&_svg]:size-3" - disabled={busy || !writable} + disabled={busy || !writable || !canWrite} onClick={() => setWriting(true)} > @@ -181,6 +183,7 @@ export function DocumentsPage({ workspaceId, onSelectNode }: { locale={locale} busy={busy} onSelectNode={onSelectNode} + canWrite={canWrite} onRegenerate={() => write(open.kind, open.form, open.subject, {})} /> )} @@ -248,7 +251,7 @@ function DocumentRow({ group, name, forms, openPath, onOpen, locale, staleLabel ) } -function DocumentReader({ workspaceId, view, form, title, locale, busy, onSelectNode, onRegenerate }: { +function DocumentReader({ workspaceId, view, form, title, locale, busy, canWrite, onSelectNode, onRegenerate }: { workspaceId: string view: GeneratedView form: ViewFormDescriptor | undefined @@ -256,6 +259,7 @@ function DocumentReader({ workspaceId, view, form, title, locale, busy, onSelect locale: Locale busy: boolean onSelectNode: (nodeId: NodeId) => void + canWrite: boolean onRegenerate: () => void }) { const { t } = useTranslation() @@ -266,7 +270,7 @@ function DocumentReader({ workspaceId, view, form, title, locale, busy, onSelect + diff --git a/packages/studio/src/pages/Inbox.tsx b/packages/studio/src/pages/Inbox.tsx index 949d0576..e9f81473 100644 --- a/packages/studio/src/pages/Inbox.tsx +++ b/packages/studio/src/pages/Inbox.tsx @@ -7,6 +7,8 @@ import { useCallback, useEffect, useMemo, useState } from 'react' import { useTranslation } from 'react-i18next' import { RunBlocks } from '@/components/blocks/RunBlocks' import { EmptyState } from '@/components/EmptyState' +import { ClarificationDetail, questionExcerpt } from '@/components/handoff/ClarificationDetail' +import { ProposalDetail } from '@/components/handoff/ProposalDetail' import { ListRow, ListRowTitle } from '@/components/ListRow' import { RunTranscript } from '@/components/RunTranscript' import { SurfaceBand } from '@/components/SurfaceBand' @@ -17,12 +19,11 @@ import { Tabs, TabsList, TabsTrigger } from '@/components/ui/tabs' import { api } from '@/lib/api' import { TRANSCRIPT_VIEW } from '@/lib/blocks/audience' import { buildItems } from '@/lib/inboxItems' -import { useClarificationByStatus, useCoverage, useProposalsByStatus } from '@/lib/queries' +import { useClarificationByStatus, useCoverage, useMe, useProposalsByStatus, useWorkspaceMembers } from '@/lib/queries' import { runStore } from '@/lib/runStore' -import { useRun } from '@/lib/useRun' -import { ClarificationDetail, questionExcerpt } from './Clarification' -import { ProposalDetail } from './Proposals' +import { useRun } from '@/lib/useRun' +import { useWorkspacePolicy } from '@/policy' type KindFilter = 'all' | 'asked' | 'proposal' @@ -47,10 +48,14 @@ export function InboxPage({ workspaceId, focusedProposalId, onFocusConsumed }: { const [kind, setKind] = useState('all') const [listOpen, setListOpen] = useState(true) const [selectedId, setSelectedId] = useState(null) + const [showAll, setShowAll] = useState(false) + const { data: me } = useMe() + const { data: members } = useWorkspaceMembers(workspaceId) + const seesEveryone = useWorkspacePolicy(workspaceId).can('workspace.manage') - const clarifications = useClarificationByStatus(workspaceId, 'pending') + const clarifications = useClarificationByStatus(workspaceId, 'pending', showAll) const answered = useClarificationByStatus(workspaceId, 'answered') - const proposals = useProposalsByStatus(workspaceId, 'pending') + const proposals = useProposalsByStatus(workspaceId, 'pending', showAll) const coverage = useCoverage(workspaceId) const isLoading = clarifications.isLoading || proposals.isLoading @@ -139,7 +144,24 @@ export function InboxPage({ workspaceId, focusedProposalId, onFocusConsumed }: { collapse={{ collapsed: !listOpen, onToggle: next => setListOpen(!next), showLabel: t('common.showList') }} list={( <> - setListOpen(false)} />}> + + {/* Nothing to disambiguate on a solo workspace, every item is yours. */} + {seesEveryone && (members?.items.length ?? 0) > 1 && ( + + )} + setListOpen(false)} /> + + )} + > setKind(value as KindFilter)}> {t('inbox.filter.all', { count: items.length })} @@ -154,6 +176,7 @@ export function InboxPage({ workspaceId, focusedProposalId, onFocusConsumed }: { key={item.id} item={item} stageLabels={coverage.data?.stages ?? []} + viewerId={me?.id} active={item.id === selectedId} onSelect={() => setSelectedId(item.id)} /> @@ -314,16 +337,19 @@ function ItemDetail({ workspaceId, item, onComplete, onSettled }: { ) } -function InboxRow({ item, active, onSelect, stageLabels }: { +function InboxRow({ item, active, onSelect, stageLabels, viewerId }: { item: Item active: boolean onSelect: () => void /** What the ontology calls each step, so no row shows a skill id. */ stageLabels: readonly CoverageStage[] + /** Who is reading, so a row says whose it is only when it is not theirs. */ + viewerId: string | undefined }) { const { t } = useTranslation() const { i18n } = useTranslation() const { label, title, source } = describe(item, stageLabels, i18n.language) + const raisedBy = originOf(item, viewerId, t('inbox.bySystem')) return ( @@ -338,6 +364,9 @@ function InboxRow({ item, active, onSelect, stageLabels }: { {source && ( {source} )} + {raisedBy && ( + {raisedBy} + )} {title || t('inbox.untitled')} @@ -347,6 +376,28 @@ function InboxRow({ item, active, onSelect, stageLabels }: { ) } +/** + * Whose handoff this is, when it is not the reader's own. + * + * Their own is left unlabelled, since a name on every row would say nothing. + * A service handoff belongs to the workspace rather than to anybody, + * so it is named for what raised it rather than for a person. + */ +function originOf(item: Item, viewerId: string | undefined, systemLabel: string): string | undefined { + const record = item.kind === 'proposal' + ? item.record + : item.kind === 'question' + ? item.record + : item.kind === 'parked' ? item.questions[0] : undefined + if (!record) + return undefined + if (record.ownerKind === 'service') + return systemLabel + if (!viewerId || record.owner === viewerId) + return undefined + return record.ownerDisplayName ?? record.owner +} + function nameStep(skillId: string | undefined, stages: readonly CoverageStage[], locale: string): string | undefined { if (!skillId) return undefined diff --git a/packages/studio/src/policy/checks.ts b/packages/studio/src/policy/checks.ts index e524285f..513c48cc 100644 --- a/packages/studio/src/policy/checks.ts +++ b/packages/studio/src/policy/checks.ts @@ -7,36 +7,27 @@ import type { CapabilityCheck } from './CapabilityCheck' * the logic is small enough to read top-to-bottom here. * * Keep these byte-equivalent with the server-side checks, - * under packages/server/src/policy/checks/. + * under packages/server/src/policy/checks.ts. * If they diverge, the optimistic UI will lie, * about what the server will allow. */ export const checks: readonly CapabilityCheck[] = [ - // Server-scope. Evaluated with member=undefined, - // so only admins resolve to effectiveRole='owner' under that path. - { id: 'workspace.create', evaluate: v => v.effectiveRole === 'owner' }, - // Server admin reads serverRole directly, not effectiveRole, - // since a workspace owner also resolves to an owner effectiveRole. - { id: 'server.admin', evaluate: v => v.user.serverRole === 'admin' }, - // Workspace-scope read/write pairs. - { id: 'workspace.read', evaluate: v => v.effectiveRole !== null }, - { id: 'workspace.write', evaluate: v => v.effectiveRole === 'owner' }, - { - id: 'proposal.read', - evaluate: v => v.effectiveRole === 'owner' || v.effectiveRole === 'maintainer', - }, + // Server scope, read off serverRole, since these resolve with no member + // and a workspace owner resolves to an owner effectiveRole too. + { id: 'server.write', evaluate: v => v.user.serverRole === 'admin' }, + { id: 'server.manage', evaluate: v => v.user.serverRole === 'admin' }, + // Workspace scope. Anything every member may read has no check, + // because membership alone already decided it. { - id: 'proposal.write', + id: 'handoff.read', evaluate: v => v.effectiveRole === 'owner' || v.effectiveRole === 'maintainer', }, { - id: 'clarification.read', - evaluate: v => v.effectiveRole === 'owner' || v.effectiveRole === 'maintainer', - }, - { - id: 'clarification.write', + id: 'handoff.write', evaluate: v => v.effectiveRole === 'owner' || v.effectiveRole === 'maintainer', }, + { id: 'workspace.write', evaluate: v => v.effectiveRole === 'owner' }, + { id: 'workspace.manage', evaluate: v => v.effectiveRole === 'owner' }, { id: 'history.write', evaluate: v => v.effectiveRole === 'owner' }, { id: 'skill.run', diff --git a/packages/studio/test/pages/Clarification.test.ts b/packages/studio/test/components/handoff/ClarificationDetail.test.ts similarity index 97% rename from packages/studio/test/pages/Clarification.test.ts rename to packages/studio/test/components/handoff/ClarificationDetail.test.ts index ced6e7a6..e913beb9 100644 --- a/packages/studio/test/pages/Clarification.test.ts +++ b/packages/studio/test/components/handoff/ClarificationDetail.test.ts @@ -1,6 +1,6 @@ import type { EdgeId, GraphOperation, NodeId } from '@braidhq/schema' import { describe, expect, it } from 'vitest' -import { candidateLetter, formatOpsSummary, questionExcerpt, summarizeOps } from '../../src/pages/Clarification' +import { candidateLetter, formatOpsSummary, questionExcerpt, summarizeOps } from '../../../src/components/handoff/ClarificationDetail' function addNode(id: string): GraphOperation { return {