feat: team-edition collaboration surfaces + workspace-context seam - #5302
feat: team-edition collaboration surfaces + workspace-context seam#5302lefarcen wants to merge 63 commits into
Conversation
Storage layer for the read-only comment drift ladder (C lane, spec D2), building on the contract from #5222. - preview_comments gains anchor_state / anchored_version / author_member_id / last_good_position_json. The migration runs after the slide-key table rebuild so a legacy rebuild cannot drop the new columns. - upsert persists creation metadata (anchoredVersion, authorMemberId); resolved state stays null for the drift ladder to fill in. - updatePreviewCommentAnchor is the engine write-back: COALESCE keeps the last-good position/version on a lost resolve, and it never bumps updated_at. - contract: anchoredVersion on PreviewCommentTarget, authorMemberId on the upsert request (server-set), and PreviewCommentAnchorUpdateRequest. Tests cover fresh-db columns, creation round-trip, engine write-back with lost-keeps-last-good, and legacy-migration backfill.
The read-only team-collab comment layer (C lane, spec D2) must survive the
author restructuring the HTML. resolveCommentAnchor replaces the silent
exact-match with an explicit ladder that never mis-points:
- exact anchor hit -> anchored (older content version -> reanchored)
- content-based fuzzy match (selector / htmlHint / text; position only breaks
ties) -> stale
- nothing found -> lost (ghost pin at lastGoodPosition)
liveSnapshotForComment keeps its exact single-user behavior; the ladder is a new
opt-in function for the collab UI. Re-exports PreviewCommentAnchorState to web.
Tests: 5 ladder cases + web typecheck green.
PATCH /api/projects/:id/conversations/:cid/comments/:commentId/anchor calls updatePreviewCommentAnchor so the client-side drift ladder can persist how each comment resolved (anchor_state + last-good position). Mirrors the existing status PATCH; deliberately skips updateProject() because anchor resolution is a derived read-back, not a content edit.
CommentPreviewOverlays gains an opt-in driftLadder prop: when set it resolves
each comment through resolveCommentAnchor (keeping stale/lost with an explicit
badge + ghost styling) instead of the exact-match silent drop. Single-user
callers leave it off, so their behavior is unchanged. The marker exposes
data-anchor-state and a comment-saved-marker--{state} class; core.css styles
reanchored (amber), stale (dashed), and lost (grey ghost).
C's trigger + orchestration for team sync: coalesce rapid author edits into a single publish (half-written intermediate states never reach members), flush at run boundaries, and re-publish if a change lands mid-publish (last-write-wins, no lost change). Publishing content + advancing the published ref is E's resource hub behind a ResourcePublishAdapter interface — this owns only the *when*. onPublished fires only after the adapter resolves (durable) so members are never told to pull a not-yet-durable version (spec I.0). 6 unit tests.
Poll-friendly heartbeat set of who is viewing a shared project, decoupled from the resource/sync layer (spec D2/D6). Timer-free: present() sweeps expired members on read (matching the polling model), and onChange fires on explicit join/leave. Not a realtime-cursor engine (live cursors were cut). 6 unit tests.
createCollabRuntime bundles the presence tracker + publish scheduler (with a stub resource-hub adapter until E's client ships) behind one factory the server wires once. registerCollabPresenceRoutes exposes GET presence + POST heartbeat/leave, so presence is now a live, poll-friendly capability. 4 route tests over a real express server.
Exposes C's coalescing publish scheduler as a live, client-driven capability: POST /collab/changed (debounced), POST /collab/publish (flush at run boundary), GET /collab/status (published head version). The client is authoritative about being in a shared context, so it drives the trigger — no dependency on D's visibility fact. Publishing still routes through the stub resource adapter until E's client lands. The runtime tracks the published head per project. 3 route tests.
Client glue that consumes the daemon collab APIs (C lane): heartbeats presence, polls the published head version so a member knows when to pull, and reports author-side edits / requests a run-boundary publish. Polling-based per spec §D6 (live cursors cut). Injectable fetch + timers for tests.
Closes the UI/CLI dual-track for the C-lane collab capability (AGENTS.md capability-exposure rule). `od collab` mirrors the presence + sync routes the web CollabClient drives: status / presence / heartbeat / leave / changed / publish, all with --json. Exercised end-to-end via tsx against a stub daemon socket (real SUBCOMMAND_MAP dispatch + request shaping).
Consumes the full presence vertical (tracker -> route -> CollabClient) as React surface (C lane, spec §D6 在场). useCollab starts/stops the client on mount and re-renders on the present set / published version; PresenceBar is the poll-driven avatar stack (no live cursors). CSS Module per web ownership rules. renderHook + RTL tests with mock fetch / fake timers.
Closes the comment drift-ladder loop end-to-end (C lane, spec §D2). When a comment first drifts to `lost`, planLostAnchorWriteBacks captures its last-good position and persistCommentAnchor PATCHes the daemon anchor route, so the ghost pin survives reload. anchored/reanchored/stale stay derived per-viewer (never written back — they'd conflict across members). FileViewer fires this once per comment (ref-guarded against pod-draw re-renders; server COALESCEs too). Inert until collab mode sets driftLadder + onLostAnchors.
The presence identity + sync-status shapes were declared twice (daemon PresenceMember, web CollabPresenceMember). Per the repo boundary rule (shared web/daemon DTOs live in packages/contracts), add api/collab.ts as the single source of truth and have both sides consume it, so the daemon route, web CollabClient, and od collab CLI can never drift apart.
A drivable demo of the C-lane presence + sync loop against the real daemon routes: open /collab-demo/<projectId> in two tabs and each appears in the other's presence overlay; an author Publish advances the head both tabs poll, surfacing a Pull prompt on the member tab. The member identity and shared entry are clearly-labeled demo stubs (real B identity / D visibility pending) — everything the daemon does (presence set, coalesced publish, version head) is real. Router round-trip + component tests included.
Three fixes found by launching it: (1) CSS Module pure-mode rejects bare element selectors — scope button/select/input under .view; (2) apps/web's webpack does not resolve NodeNext-style .js import specifiers — drop the extensions on the collab sibling imports (matches the rest of apps/web); (3) exempt /collab-demo from the first-run onboarding redirect so the deep-link stays reachable without completing onboarding. Verified live: two tabs see each other's presence and Publish advances the polled head (Pull vN).
Adds the comment drift ladder to the collab demo surface so all three C-lane verticals are drivable. Four scenario buttons re-resolve a fixed comment through the real resolveCommentAnchor + planLostAnchorWriteBacks: unchanged → anchored, re-published → reanchored, id churned but content intact → stale (content recovery), section removed → lost (ghost pin) with the durable PATCH write-back shown. Engine is production code; only the DOM snapshots are synthetic (labelled). Verified live in-browser across all four states.
Daemon-side Tier-1 skeleton for team resource sharing over the Vela resource hub: - integrations/resource-hub.ts: hub HTTP client (index ops real; auth and workspace principal are seams; blob byte transfer pending transport). - routes/resources/index.ts: registerResourceSharingRoutes (list/_status real; share/pull 501 pending wrap logic). - resource-cli.ts: `od resource` (list real; share/pull stub). Hot-file footprint kept to 4 lines (server.ts + cli.ts); app-config.ts untouched. New files typecheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A hard tab close skips React unmount, so the fetch-based leave never sent and the member lingered until the daemon's 30s presence TTL. Add CollabClient. leaveBeacon() (navigator.sendBeacon, keepalive-fetch fallback) fired from a useCollab pagehide listener, so a closing tab hands off a leave that survives the unload. Also persist the demo member id in sessionStorage so a reload reuses one identity instead of piling up a fresh 'Demo member' each time. Verified live: navigating away drops the present set to [] at once.
resource-hub.ts client: implement pushBlob/pullBlob + prepareUpload/ commitUpload. Bytes flow daemon<->object-store direct via hub-issued short-TTL presigned URLs (transport decision 2026-07-07); the hub is never in the byte path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The C-lane plug point that replaces the demo's stubbed identity once B ships. resolveCollabSession takes a faithful subset of B's CurrentWorkspaceContext (workspaceType / workspaceMemberId / role / memberStatus / lifecycleState — mirrored field-for-field from vela packages/shared/src/workspace-context.ts) and decides whether presence + sync run: team workspace + active member + live lifecycle → on with identity from workspaceMemberId; personal / removed / frozen / deleting / deleted → off with a diagnostic reason. Encodes D's team-only visibility gate. Decision logic is real; only the context source (B's endpoint) remains to wire.
The daemon's single plug point for B's identity/visibility. GET /api/workspace/ context returns the caller's WorkspaceCollabContext (a faithful subset of B's CurrentWorkspaceContext, now a shared contract DTO); the web client feeds it to resolveCollabSession to decide whether collab runs and who the present member is. WorkspaceContextProvider is swappable — a dev provider (settable via PUT, seedable via OD_DEV_WORKSPACE_CONTEXT) stands in until B is reachable; a real B-backed provider derives the context per-request from the token and omits the PUT setter. Validated context parsing + route round-trip.
resource-drive.ts: kind-agnostic tree<->manifest+blobs primitives — packTree (content-address a directory), pushTree (upload missing blobs + publish a version), materializeTree / materializeRef (safe §2.7 landing). resource-hub.ts client gains listVersions + getManifest. `od resource` CLI gains neutral put/get (directory round-trip). Feature-specific sharing stays a consumer layer built on top, not here. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Wires the D->C orchestration seam the D lane emits to (from the project-sync contract): POST /api/projects/:id/collab/sync-intent accepts a ProjectSyncIntent; project_team_share_requested marks the project pending_upload and flushes a publish (driving E's resource mechanism behind the scheduler), then the state machine advances pending_upload -> synced (onPublished) / sync_failed (onError). GET /collab/status and CollabClient now carry syncState so members see it. Dual- track: od collab share + syncState in od collab status. Demo gets a Share-to-team button + sync-state badge. 12 new/updated daemon+web tests green.
…ter)
Adds the member-side pull to the C sync surface: ResourcePublishAdapter grows
an optional syncLatest (E v1.0 §2.4 getRef('published') — read the head without
advancing it), the stub reports the in-memory head, and the runtime exposes
pullLatest. POST /api/projects/:id/collab/pull + od collab pull return the head
version a member pulled. C owns *when* to pull (sync timing); E's client
fetches + extracts the bytes behind the adapter. 2 new tests green.
The foundation invariant (地基, C/麻薯-owned) every lane mounts at each 'copy a team resource into a personal editable copy' route: assertTeamResource CopyAllowed / evaluateTeamResourceCopy over TeamResourceState (active|frozen| deleted). A frozen or deleted team resource is blocked (WORKSPACE_RESOURCE_ FROZEN / _DELETED) — the AC-9 escape hole that would let a downgraded team keep using frozen content. Pure TS so daemon/web/CLI all import it; throws a coded TeamResourceCopyForbiddenError the daemon maps to 403. Route-mounting follows once D1's team-resource-state model lands (mounting a stubbed-pass guard now would be false enforcement). 7 tests green.
The new role-gating layer for the Settings shell (E-frontend). Pure derivation over the member role (from B's foundation context): workspaceSettingsSections ForRole / canSeeWorkspaceSettingsSection decide which *workspace* sections the shell shows — members (owner/admin), billing (owner-only), team-space (owner/ admin); nothing for a member or outside a team context. E owns only this shell visibility; the section internals belong to B (members) / A (billing) / D (team-space), and there is no team-level BYOK this cycle. Wiring into the (unconditional, 8k-line) SettingsDialog is the follow-up. 4 tests green.
…nt (D1) Starts D1's team-resource state model and makes the D3 copy red-line REAL (not a stubbed-pass). TeamResourceStateProvider reports whether a design-system/plugin/ skill is team-shared + its state (the E-resource-hub seam, 沅锡); GET/PUT /api/workspace/resources/:kind/:id/state expose it, and POST .../copy-check runs assertTeamResourceCopyAllowed against the resolved state — a frozen team resource is rejected with 403 WORKSPACE_RESOURCE_FROZEN (proven by a test that seeds a frozen resource). Unregistered resources resolve to personal, so the guard is correctly dormant until E's hub reports real team resources. Wiring the same provider+guard into the existing copy-out routes (plugin duplicate, DS copy, skill edit-shadow) is the follow-up. 4 tests green.
Moves D3 from a check endpoint to a real copy-out route. enforceTeamResource CopyAllowed (resolve state + assert) is called in POST /api/plugins/:id/ duplicate-project before any project is created; a frozen team plugin is rejected 403 WORKSPACE_RESOURCE_FROZEN. Guard is optional (teamResources dep) + dormant until the resource-hub reports a frozen team plugin, so existing behavior is unchanged. Same helper wires into the remaining escape routes (DS copy, skill edit-shadow) next. Helper tested (frozen → throws).
Second real escape route (after plugin duplicate): PUT /api/skills/:id clones a built-in/shared skill into USER_SKILLS_DIR on first edit — a frozen team skill must not be edit-shadowed into a personal editable copy. enforceTeamResource CopyAllowed runs after the skill is resolved; a frozen team skill is rejected 403 WORKSPACE_RESOURCE_FROZEN. Optional (teamResources dep) + dormant until the hub reports a frozen team skill, so existing behavior is unchanged. Guard helper already tested (frozen → throws).
The resource hub now runs inside services/api, so the default hub URL is the API port (18080). Also correct the stale module header: blob byte transfer (presigned client-direct) is implemented, not pending. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Read-only is now ownership-based, not role-based: the daemon records the member who shares a project (its single writer) at sync-intent time and returns it in the collab status; the client marks the project read-only only for viewers who are not that owner. The sharer keeps full edit access to their own project while everyone else sees it read-only — correct single-writer semantics, derived in-lane without a separate project-ownership source. Falls back to the workspace role until the owner id has been polled so a member never briefly gets edit affordances on a shared project. Verified end-to-end: the owner who shared collab-live-1 keeps the edit toolbar and active composer; a different member sees the read-only notice with edit surfaces withheld.
Widens the entry nav rail from a 56px icon column into a 236px labeled sidebar: each nav button now shows its icon alongside a text label, matching the team workspace shell in the demo. Pairs with the top-bar workspace switcher for the team-workspace navigation. Team-scoped entries (members, dashboard, workspace settings) are owned by other lanes and are left out until their views exist. Verified across entry views (home, design systems) with no layout regression.
Generalizes the design-system team-share service into a kind-parametrized
resource-share service and mounts it for design systems, plugins, and skills:
each packs the resource's own directory and pushes it to the resource hub under
its own kind (design_system / plugin / skill), reusing the same verified publish
adapter and the one workspace-context principal. Plugin dirs resolve via the
installed-plugin record's fsPath; skill dirs via the skill registry (the adapter
now awaits an async dir resolver). Routes are mounted per kind at
/api/workspace/{design-systems,plugins,skills}/{team,:id/share}.
Verified end-to-end against the resource-hub fixture: a plugin-kind and a
skill-kind resource each publish and pull back byte-identical.
Adds `od collab share-resource <design-systems|plugins|skills> <id>` and
`od collab team-resources <kind>` covering all three shareable resource kinds
against the /api/workspace/{kind}/{team,:id/share} routes; the design-system
forms are kept as aliases. Keeps team sharing on the CLI dual-track for every
kind so external agents can drive it headless.
Replaces the plugins team tab placeholder with a working team collection: it lists the member's installed plugins, each with a share-to-team action that pushes the plugin to the resource hub (plugin kind) so teammates can pull it, and marks already-shared ones. Mirrors the design-system team collection and reads the same /api/workspace/plugins endpoints; off-team it degrades to an empty collection. Web typecheck green.
Extends the team tab into a team-resources view: the member's personal skills join installed plugins, each with a share-to-team action that pushes the skill to the resource hub (skill kind). Mirrors the plugin/design-system team collections and reads /api/workspace/skills. Verified end-to-end in the real product: sharing a personal skill from the UI marks it shared and lists it under /api/workspace/skills/team.
…ollab-c-lane # Conflicts: # packages/contracts/src/api/project-sync.ts # packages/contracts/src/index.ts
Expand WorkspaceCollabContext to a faithful mirror of B's CurrentWorkspaceContext (providerMode, billingState, planId, seatSummary, permissions, billingRecovery), matching the shape B shipped in vela's shared workspace-context. Add the same permission/seat derivation helpers so the daemon's dev context stub and the real B proxy both derive through one rule set and cannot drift. The provider mode (platform_credits vs personal_byok) is orthogonal to team collab: a BYOK workspace keeps full team features, gated on lifecycle/role, never on provider. Team surfaces now consume permission bits (canWriteSyncedFiles, canShareProjects, canManageSharedResources) instead of re-deriving from role/lifecycle.
…ssion bit Read-only now has two independent gates: the existing shared-project ownership gate, plus a workspace-level gate that consumes B's canWriteSyncedFiles. A locked or frozen workspace (or a removed member) freezes everyone read-only, the project owner included, instead of only non-owners of shared projects. Consuming the permission bit keeps this in lockstep with B rather than re-deriving the rule. Add a red spec for the owner-frozen-when-locked case, and update the dev context fixtures to the enriched shape parseWorkspaceCollabContext now derives.
…oint Add createVelaWorkspaceContextProvider (T2): the daemon reuses the same vela login session AMR and the vela CLI use (readVelaControlApiContext reads the control key + api url from ~/.amr/config.json) to call B's authoritative GET /api/v1/workspaces/ current, then maps the CurrentWorkspaceContext onto WorkspaceCollabContext. One vela session drives AMR, resource sharing, and the workspace context — no second identity. The mapper trusts B's permissions/seatSummary (re-deriving only if malformed), derives teamId from workspaceId (the team workspace IS the team scope the resource hub keys by), and preserves providerMode so a BYOK workspace keeps full team features. Any failure — no session, signed out, B unreachable — degrades to null so collab stays single-player and never throws. Provider selection is env-gated (OD_WORKSPACE_CONTEXT_SOURCE=vela) so demo and tools-dev runs keep the dev stub they drive via PUT.
… context Rebuild EntryNavRail as a two-state shell gated on the real WorkspaceCollabContext (GET /api/workspace/context): a full team rail when signed into a team workspace, a lean local rail otherwise. The gate is workspaceType==='team', never providerMode — a BYOK / non-AMR workspace keeps every team feature. - Team rail: workspace switcher + plan chip, search, Home, Community (now in the rail, not under Home), and a TEAM section — Drafts, All projects, Design systems, Plugins, Members, Board, Workspace settings. Visibility comes from B's permission bits (canManageMembers/canInviteMembers/canViewWorkspaceSettings/canManageBilling), never re-derived from role. A locked workspace shows a recovery banner. - Local rail: search, Home, Community, Design systems, Plugins + a sign-in callout. - Team entries route to TeamSlotPlaceholder — C owns the shell + entry points; the Members/Board/Settings views themselves are B/A/D's lanes and are NOT built here. - Fix the invite affordance: inviting a colleague opens the members/invite slot, not the onboarding flow (that jump was wrong — it is not a first-run flow). - Move the workspace switcher out of the topbar into the rail. Community renders in-shell; deep-linkable routes added for the new views.
Add a ResourcePublishAdapter that drives the resource hub through the vela CLI (vela resource push/head/pull) instead of the in-process SDK. It reuses the same vela login session AMR uses — one identity — and keeps the content-addressing in the CLI so any vela-embedding project shares the exact code path. syncLatest maps to the new `vela resource head` (cheap version check, no download). Transport is opt-in via OD_RESOURCE_TRANSPORT=vela-cli and coexists with the SDK adapter (the default) so the收口 rolls out only where the CLI is present. The child process is injected in tests, so the wiring is verified without a live CLI or hub. Both transports gate on the same workspace context — a personal or signed-out session never publishes. Also fix a latent test-only typecheck error in the T2 provider spec.
Add a Workspace section to Settings that appears only for a team member who can view workspace settings (workspaceType==='team' && permissions.canViewWorkspace Settings). Its entries — members, subscription & seats, auto-recharge, team space — are shown by consuming B's permission bits directly (canManageMembers / canInviteMembers / canManageBilling / canManageAutoRecharge), never re-derived from role. A locked workspace shows the recovery banner. Per the settings-IA boundary, this is shell only: each entry renders the shared TeamSlotPlaceholder inline — the members / billing / team-space views themselves are B/A/D's lanes and are not built here. Personal settings and BYOK stay exactly as they were (no team-level provider this phase). Rewrite settings-access to permission-bit gating and update the execution mock to answer the new /api/workspace/context fetch.
…chine) Add the client half of the team-invite handoff (C's lane): the web acceptance page plus the contracts and local state B's server contract hands off to. A member opens the invite link, the page previews it, guards an account mismatch, accepts via B's API, then hands off to the desktop client through the deeplink — with download-fallback and retry-open paths when the client is absent or fails. - contracts/workspace-invites.ts: preview/accept DTOs, the continuation, and the local LocalPendingInviteContinuation / LocalWorkspaceActivation shapes, plus pure parseInviteDeeplink/buildInviteDeeplink. Reuses collab.ts role/lifecycle enums — none re-declared. - InviteAcceptanceFlow.tsx: every state — preview, account-mismatch, accepting, success (retry-open), not-installed fallback, open-failed, and each terminal / recoverable exception. - invite-continuation.ts: single-slot storage (self-healing, expiry-dropping), activation derivation, account-match heuristic. The raw invite token is never persisted — only the nonce/continuation. Desktop opendesign:// registration and shell routing stay out (injectable seams); 27 unit tests cover deeplink parsing, storage round-trips, and account matching.
lefarcen
added
size/XXL
nettee
left a comment
nettee
left a comment
There was a problem hiding this comment.
I found three blocking issues in the new collaboration persistence and authorization path. They are all in changed lines and I left the concrete fixes inline.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.| const published = new Map<string, number>(); | ||
| const syncStates = new Map<string, ProjectSyncState>(); | ||
| // projectId → the member who shared it (the single writer). Members compare | ||
| // this to their own id to know whether they view the project read-only. | ||
| const owners = new Map<string, string>(); | ||
| // Always track the published head + sync state so members can poll them; also | ||
| // forward to any caller-supplied callback. (exactOptionalPropertyTypes forbids | ||
| // assigning an explicit `undefined` to an optional property, hence we always | ||
| // wrap onError rather than passing options.onError through conditionally.) | ||
| const schedulerOptions: CollabPublishSchedulerOptions = { | ||
| adapter, | ||
| onPublished: (result) => { | ||
| published.set(result.projectId, result.version); | ||
| syncStates.set(result.projectId, 'synced'); | ||
| options.onPublished?.(result); | ||
| }, | ||
| onError: (result) => { | ||
| // A failed publish leaves the prior head standing; surface it as a | ||
| // recoverable sync state rather than wedging the project. | ||
| syncStates.set(result.projectId, 'sync_failed'); | ||
| options.onError?.(result); | ||
| }, | ||
| }; | ||
| const scheduler = new CollabPublishScheduler(schedulerOptions); | ||
| const presenceOptions: CollabPresenceTrackerOptions = {}; | ||
| if (options.onPresenceChange) presenceOptions.onChange = options.onPresenceChange; | ||
| const presence = new CollabPresenceTracker(presenceOptions); | ||
| const teamResources = options.teamResources ?? createDevTeamResourceStateProvider(); | ||
| return { | ||
| presence, | ||
| scheduler, | ||
| workspaceContext, | ||
| teamResources, | ||
| publishedVersion: (projectId) => published.get(projectId) ?? null, | ||
| projectSyncState: (projectId) => syncStates.get(projectId) ?? 'local_only', |
There was a problem hiding this comment.
This runtime keeps the published version, sync state, and owner id only in process-local maps and then reports null / local_only whenever those maps are empty. That means a daemon restart forgets that a project was already shared, so the next viewer sees the project as local-only, loses the read-only gate, and never learns which published version to pull until someone re-shares it. The changed lines show the problem directly: published, syncStates, and owners are initialized as empty Maps here, and publishedVersion / projectSyncState / projectOwnerMemberId read only from those maps. Please derive these values from durable state instead of process memory, for example by hydrating them from the resource hub (or persisted project metadata) when the runtime starts or when /collab/status is queried, and add a restart/regression test that proves an already-shared project stays shared after recreating the runtime.
| // Ids shared this session. The published resources are the durable record on | ||
| // the hub; this is the fast local view the team collection reads until a hub | ||
| // listing query lands. | ||
| const shared = new Set<string>(); |
There was a problem hiding this comment.
The team collection is backed only by this in-memory Set, so it only contains resources shared through the current daemon process. After a daemon restart, or from a teammate on a different machine, GET /api/workspace/.../team will come back empty even though the resource was successfully published to the hub. That breaks the user-visible behavior described in the PR body (“Team collections … members pull shared resources into their own client”), because there is no durable listing path here. Please make sharedIds() read from durable hub state instead of session memory, for example by listing team resources from the hub and filtering by kind / idPrefix, and cover the cross-process case with a test that recreates the service after one share.
| // Share a personal resource to the team. | ||
| app.post(`${root}/:id/share`, async (req, res) => { | ||
| const id = typeof req.params.id === 'string' ? decodeURIComponent(req.params.id) : ''; | ||
| if (!id) return res.status(400).json({ error: 'invalid resource id' }); | ||
| try { | ||
| const result = await share.share(id); | ||
| if (!result) return res.json({ shared: false }); | ||
| res.json({ shared: true, version: result.version }); |
There was a problem hiding this comment.
This route publishes straight to the team as long as share.share() succeeds, but nothing on this path checks the new workspace permission bits before doing so. In the server wiring, teamShareGetPrincipal only derives identity from workspaceContext.current(); it never enforces permissions.canManageSharedResources, so any active team member who can hit this endpoint or the matching od collab command can publish shared resources even though the UI models that as an owner/admin capability. That is an authorization bug, not just a missing button gate, because the API and CLI remain writable for callers who should be read-only. Please resolve the caller workspace context per request and return 403 unless permissions.canManageSharedResources is true before invoking share.share(), and add a route-level test for a member context to prevent regressions.
Visual regression reviewHead:
0 changed · 0 unchanged · 47 new without baseline · 1 failed Capture or diff failures
New cases without baselines
27 additional new case(s) omitted from this comment. Visual diff is advisory only and does not block merging. |
Product decision: team management (members, dashboard, billing, workspace settings) lives in the cloud web app, and the local client only links out to it — it does not embed those views. Align the shell accordingly: - The nav rail drops the Members and dashboard/Board entries and keeps a single Workspace settings entry that opens the cloud console in the browser. The plan chip links to the same console for billing. - The Settings > Workspace region collapses from an in-client entry list to one link to that console. - WorkspaceCollabContext carries workspaceSettingsUrl (the console URL); the daemon populates it from the upstream context when present, else builds it from OD_VELA_WEB_URL. Team workspaces only. This corrects the earlier demo-shaped assumption that those views would be embedded in the local client via placeholders.
nettee
left a comment
nettee
left a comment
There was a problem hiding this comment.
I found six blocking issues in the current head: the new read-only/share gates are still bypassable, and the new resource-hub persistence/pull path loses or misapplies shared state in normal flows. I left each fix inline.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.Location: apps/daemon/src/resource-drive.ts RIGHT lines 131-150
This materializer only creates or overwrites entries from the incoming manifest; it never removes paths that disappeared from the previous version or replaces conflicting existing node types before writing the new tree. That means a pull leaves deleted files/directories behind, and file↔dir/symlink transitions can fail or produce a tree that no longer matches the published version. Because collab and team-resource pulls land directly into the live workspace, this breaks the PR's byte-identical sync guarantee. Please stage into a clean temp directory and swap it into place, or explicitly prune/replace any destination paths not present in the manifest, and add a regression test that deletes or type-flips a file between versions.
Inline comment could not be anchored: inline anchor is outside the PR diff anchorable ranges
| app.post(`${root}/:id/share`, async (req, res) => { | ||
| const id = typeof req.params.id === 'string' ? decodeURIComponent(req.params.id) : ''; | ||
| if (!id) return res.status(400).json({ error: 'invalid resource id' }); | ||
| try { | ||
| const result = await share.share(id); | ||
| if (!result) return res.json({ shared: false }); | ||
| res.json({ shared: true, version: result.version }); |
There was a problem hiding this comment.
This share endpoint still has no server-side authorization check for permissions.canManageSharedResources. On the current wiring, share.share() only derives a principal from workspaceContext.current() and this route never rejects a member context before publishing, so any signed-in team member who can call the API or the matching od collab command can promote resources to the team even though the new UI models that as an owner/admin capability. Please resolve the caller workspace context per request and return 403 unless canManageSharedResources is true before invoking share.share(), and add a route-level test for the denied member case.
| // Ids shared this session. The published resources are the durable record on | ||
| // the hub; this is the fast local view the team collection reads until a hub | ||
| // listing query lands. | ||
| const shared = new Set<string>(); |
There was a problem hiding this comment.
The team collection is still backed only by this in-memory Set, and sharedIds() later just replays it. After a daemon restart, or from a teammate on a different machine, GET /api/workspace/.../team will come back empty even though the resource was already published successfully. That breaks the PR's advertised team collections because there is no durable listing path here. Please derive team listings from durable hub state instead of session memory, for example by listing team resources for this kind/id prefix, and add a cross-process regression test that recreates the service after one share.
| const published = new Map<string, number>(); | ||
| const syncStates = new Map<string, ProjectSyncState>(); | ||
| // projectId → the member who shared it (the single writer). Members compare | ||
| // this to their own id to know whether they view the project read-only. | ||
| const owners = new Map<string, string>(); | ||
| // Always track the published head + sync state so members can poll them; also | ||
| // forward to any caller-supplied callback. (exactOptionalPropertyTypes forbids | ||
| // assigning an explicit `undefined` to an optional property, hence we always | ||
| // wrap onError rather than passing options.onError through conditionally.) | ||
| const schedulerOptions: CollabPublishSchedulerOptions = { | ||
| adapter, | ||
| onPublished: (result) => { | ||
| published.set(result.projectId, result.version); | ||
| syncStates.set(result.projectId, 'synced'); | ||
| options.onPublished?.(result); | ||
| }, | ||
| onError: (result) => { | ||
| // A failed publish leaves the prior head standing; surface it as a | ||
| // recoverable sync state rather than wedging the project. | ||
| syncStates.set(result.projectId, 'sync_failed'); | ||
| options.onError?.(result); | ||
| }, | ||
| }; | ||
| const scheduler = new CollabPublishScheduler(schedulerOptions); | ||
| const presenceOptions: CollabPresenceTrackerOptions = {}; | ||
| if (options.onPresenceChange) presenceOptions.onChange = options.onPresenceChange; | ||
| const presence = new CollabPresenceTracker(presenceOptions); | ||
| const teamResources = options.teamResources ?? createDevTeamResourceStateProvider(); | ||
| return { | ||
| presence, | ||
| scheduler, | ||
| workspaceContext, | ||
| teamResources, | ||
| publishedVersion: (projectId) => published.get(projectId) ?? null, | ||
| projectSyncState: (projectId) => syncStates.get(projectId) ?? 'local_only', | ||
| projectOwnerMemberId: (projectId) => owners.get(projectId) ?? null, |
There was a problem hiding this comment.
The project's published version, sync state, and owner id are still stored only in process-local maps and then read back exclusively from those maps. After a daemon restart, an already shared project falls back to publishedVersion: null, syncState: 'local_only', and ownerMemberId: null, so the next viewer loses the read-only gate and never learns which published version to pull until somebody shares again. Please hydrate these values from durable state instead of process memory, such as persisted project metadata or the resource hub, and add a restart/regression test that proves a previously shared project stays shared after recreating the runtime.
| async pullLatest(projectId) { | ||
| // The real hub adapter materializes the published tree locally; the stub | ||
| // has no bytes. Either way, report the head version. | ||
| if (adapter.pull) await adapter.pull({ projectId }); |
There was a problem hiding this comment.
pullLatest() calls adapter.pull() before it checks whether a published head exists. With the real hub adapter, pull() immediately resolves the published ref via materializeRef(), so a perfectly normal pre-share pull now throws instead of returning the version: null result this route and the stub tests expect. Please check syncLatest() first and return { version: null } before pulling, or make the real adapter treat a missing published ref as a no-op.
| @@ -881,7 +884,7 @@ export function DesignFilesPanel({ | |||
| } | |||
There was a problem hiding this comment.
This new viewerOnly gate only hides the visible create/upload actions. The panel still accepts file drops through handleDrop() above, which unconditionally calls onUploadFiles(dropped), and FileWorkspace.uploadFiles() has no viewerOnly check. A read-only member can therefore mutate a shared project simply by dragging files onto the Design Files panel, which bypasses the single-writer/read-only behavior this PR is supposed to enforce. Please block the drop/upload path when viewerOnly is true and add a focused test that a viewer-only drop never reaches uploadProjectFiles().
…loud web) Product consolidated team surfaces into the cloud web app, and the invite preview/accept page now lives there (workspace-invite-preview / -continuation). The per the B-C contract, C's remaining part is the desktop side — parsing the opendesign:// deeplink, validating the continuation, and persisting the local workspace activation. That state machine (invite-continuation.ts) and the shared contract stay; only the redundant in-client acceptance page is removed.
…eal /settings route The console serves the current workspace's settings at /settings (workspace implicit from the session), not a workspace-scoped path. Construct the fallback from OD_VELA_WEB_URL + /settings so the client can link out without waiting on the upstream context to carry the URL; an explicit URL still wins when present.
nettee
left a comment
nettee
left a comment
There was a problem hiding this comment.
I found five blocking issues that still break the promised shared-state durability and the server-side ownership/permission gates on the current head. I left the concrete fixes inline.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.| scheduler, | ||
| workspaceContext, | ||
| teamResources, | ||
| publishedVersion: (projectId) => published.get(projectId) ?? null, |
There was a problem hiding this comment.
publishedVersion, projectSyncState, and projectOwnerMemberId are all read back only from the three in-memory maps initialized above. After a daemon restart, an already shared project therefore falls back to publishedVersion: null, syncState: 'local_only', and ownerMemberId: null, so the next viewer loses the read-only gate and no longer knows which version to pull until somebody shares again. That breaks the PR's advertised persistent shared-project behavior. Please hydrate these values from durable state instead of process memory, such as persisted project metadata or the resource hub, and add a restart/regression test that recreates the runtime after one successful share.
| async pullLatest(projectId) { | ||
| // The real hub adapter materializes the published tree locally; the stub | ||
| // has no bytes. Either way, report the head version. | ||
| if (adapter.pull) await adapter.pull({ projectId }); |
There was a problem hiding this comment.
pullLatest() materializes the tree before it checks whether a published ref exists. With the real hub adapter, pull() calls materializeRef(..., 'published', ...), so a perfectly normal pre-share pull now throws instead of returning { version: null }. The current route test only uses the stub adapter, so this regression is invisible there. Please check syncLatest() first and return early when it is null, or make the real adapter treat a missing published ref as a no-op, and cover that path with a real-adapter regression test.
| if (result) shared.add(resourceId); | ||
| return result; | ||
| }, | ||
| sharedIds: () => [...shared], |
There was a problem hiding this comment.
The team collection is still backed only by this session-local Set. sharedIds() never consults durable hub state, so after a daemon restart, or from a teammate on a different machine, GET /api/workspace/.../team comes back empty even though the resource was already published successfully. That breaks the PR's stated behavior that shared design systems/plugins/skills show up in a team collection for other members to pull. Please make the listing path read from durable hub state instead of process memory, for example by listing shared resources by kind/idPrefix, and add a cross-process test that recreates the service after one share.
| app.post('/api/projects/:id/collab/changed', (req, res) => { | ||
| scheduler.notifyChanged(req.params.id, 'change'); | ||
| res.json({ ok: true }); | ||
| }); | ||
|
|
||
| // Run boundary — flush any pending publish immediately (publish the stable | ||
| // end-of-run state rather than waiting out the debounce). | ||
| app.post('/api/projects/:id/collab/publish', (req, res) => { | ||
| scheduler.notifyChanged(req.params.id, 'run'); | ||
| scheduler.runBoundary(req.params.id); | ||
| res.json({ ok: true }); | ||
| }); | ||
|
|
||
| // visibility-to-sync orchestration seam. The visibility surface flips project visibility and emits a | ||
| // ProjectSyncIntent here; the sync trigger owns the reaction. `project_team_share_requested` | ||
| // marks the project pending and flushes a publish (which drives E's resource | ||
| // mechanism behind the scheduler). `project_visibility_changed` is accepted as | ||
| // a no-op signal for now (the share request is the actionable one). | ||
| app.post('/api/projects/:id/collab/sync-intent', async (req, res) => { | ||
| const event = (req.body as { event?: unknown } | undefined)?.event; | ||
| if (typeof event !== 'string' || !SYNC_INTENT_EVENTS.has(event as ProjectSyncIntentEvent)) { | ||
| return res.status(400).json({ error: 'invalid sync intent event' }); | ||
| } | ||
| if (event === 'project_team_share_requested') { | ||
| // The caller sharing the project is its single writer; record their id so | ||
| // members can distinguish it from a project of their own. | ||
| const context = await workspaceContext.current({ | ||
| authorization: req.headers.authorization, | ||
| }); | ||
| requestTeamShare(req.params.id, context?.workspaceMemberId); | ||
| } | ||
| res.json({ ok: true, syncState: projectSyncState(req.params.id) }); |
There was a problem hiding this comment.
These write routes never enforce the single-writer contract that the new UI is trying to represent. changed and publish accept any caller unconditionally, and sync-intent records whoever calls it as the owner without checking permissions.canWriteSyncedFiles / canShareProjects or comparing the caller against the already-recorded owner. Because od collab publish and od collab share expose these endpoints directly, a non-owner member can still mutate a local copy and publish it back to the shared project even when the web surface shows them as read-only. Please resolve the caller context on every write-shaped collab route, reject writes when the workspace is non-writable or the caller is not the recorded owner, and add route-level plus CLI negative tests for a viewer-only member.
| app.post(`${root}/:id/share`, async (req, res) => { | ||
| const id = typeof req.params.id === 'string' ? decodeURIComponent(req.params.id) : ''; | ||
| if (!id) return res.status(400).json({ error: 'invalid resource id' }); | ||
| try { | ||
| const result = await share.share(id); | ||
| if (!result) return res.json({ shared: false }); | ||
| res.json({ shared: true, version: result.version }); |
There was a problem hiding this comment.
This POST handler treats shared: false as the only denial case and never checks the caller's workspace permissions before publishing. The shared contract added in this PR explicitly carries permissions.canManageSharedResources as the resource-sharing gate, so hiding the button in the UI is not enough: any team member can still POST here directly and publish a design system/plugin/skill to the team. Please resolve the caller's workspace context in the route, return 403 unless canManageSharedResources is true, and add a member-vs-owner/admin test so the backend matches the role-based behavior the new surfaces rely on.
2 participants
Why
Team-edition collaboration: members of a shared workspace need to see each other's presence, comment on shared projects, share design systems / plugins / skills across the team, and move between a personal and a team workspace. This PR lands the client-side collaboration surfaces and the workspace-context integration they all sit on, and leaves labeled integration slots for the team-management views (members, billing, dashboard, project lists) that other feature areas own — so those can be plugged into a concrete interface instead of being reinvented per surface.
Opening it now to (a) land the collaboration + resource-sharing engine the team edition builds on, and (b) give the workspace / billing / project-visibility feature areas a concrete seam to mount their views into.
What users will see
od collab …CLI subcommands for sharing resources to the team (dual-track with the UI).Team management lives in the cloud web console. Members, the team dashboard, subscription/billing, and workspace settings are managed there — the local client keeps a single Workspace settings entry that opens that console in the browser (its URL is carried on the workspace context as
workspaceSettingsUrl). The client does not embed those views.The remaining in-client seam is the project surfaces: the drafts / all-projects entries render a shared placeholder for the project-listing views the visibility feature area owns (routes
/drafts,/all-projects), reading the sharedWorkspaceCollabContext(role + permission bits) this PR ships. The invite-acceptance component is likewise standalone and prop-driven, awaiting the shell to mount it at the invite route.Surface area
od collab …resource sharing;OD_RESOURCE_TRANSPORT=vela-cli(opt-in CLI transport),OD_WORKSPACE_CONTEXT_SOURCE=vela(real context provider),OD_VELA_WEB_URL(cloud console base for the Workspace settings link)packages/contracts: workspace context, collab presence/sync, project-sync state, team resources, workspace invitesScreenshots
Entry points reviewers can drive via
pnpm tools-dev: the team nav rail (sidebar), presence avatars + read-only banner on a shared project, the team collections in Design systems / Plugins / Skills, the Workspace section in Settings, and the invite-acceptance page. (Screenshots to follow.)Bug fix verification
Validation
pnpm guard,pnpm typecheckpnpm --filter @open-design/web typecheck+ web collab / settings / invite suites greenpnpm --filter @open-design/daemon typecheck+ collab suites green