Conversation
manuscripts-cf-worker addresses a document by one opaque docID, not a (projectID, manuscriptID) pair, so EditorAppProps and everything that threads through the store now take a single docID: Api.ts's doc-related calls, Store's state, BasicSource, ApiSource, buildData, buildUtilities, StepsExchanger, and use-create-editor all collapse to one docID field. api.getProject/getUserProfiles/getProjectPermittedActions were never part of manuscripts-cf-worker's /v3 API to begin with (that gap predates this change) — they now just reuse the same docID value where projectID used to go, rather than inventing a second identifier for them. use-create-editor.ts no longer passes projectID into @manuscripts/body-editor's useEditor() — that prop was dead there (see manuscripts-body-editor's matching change removing it from EditorProps). Also, since these endpoints don't exist against manuscripts-cf-worker: - getUser() returns a static profile instead of calling GET /user - buildData()'s userRole is a static ProjectRole.owner instead of derived from a fetched project - buildData()'s manuscriptPermittedActions is a fixed list instead of fetched via getProjectPermittedActions (now unused and removed) - useGetUserName() returns a static 'User' instead of deriving a name from project/hostUsers/collaboratorsById Requires manuscripts-body-editor's projectID removal to typecheck clean (verified locally against a linked build). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical WebSocket, credential, and identity issues plus moderate data and permissions issues block approval.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR collapses projectID and manuscriptID into a single docID across the editor and manuscripts-cf-worker integration.
Changes:
- Threads
docIDthrough props, store state, sources, and collaboration. - Updates document, steps, and WebSocket API handling.
- Adds static user, role, permission, and username fallbacks.
File summaries
| File | Summary and final review findings |
|---|---|
src/store/Store.ts |
Replaces project/manuscript identifiers with docID. |
src/Main.tsx |
Threads docID into the editor. |
src/index.tsx |
Updates public props and memoization. |
src/hooks/use-get-user-name.ts |
Uses a static username. |
src/hooks/use-create-editor.ts |
Initializes the editor and collaboration with docID. |
src/EditorApp.tsx |
Initializes sources with docID. |
src/api/StepsExchanger.ts |
Uses document-scoped collaboration calls. |
src/api/buildUtilities.ts |
Updates utility arguments. Moderate (3 votes): refreshProject() calls an unavailable project endpoint and can reject; use a supported document operation. |
src/api/buildData.ts |
Adds static roles and permissions. Moderate (2 votes): the returned data omits required project state. Moderate (3 votes): the unsupported user-profiles request always fails. Moderate (1 vote): missing handleSuggestion disables suggestion acceptance. |
src/api/ApiSource.ts |
Builds API data using docID. |
src/api/Api.ts |
Updates document endpoints and WebSocket authentication. Critical (3 votes): pending joins can create sockets and reconnect after shutdown. Critical (1 vote): bearer tokens are exposed in WebSocket URLs. Critical (1 vote): the hard-coded identity can misattribute users and comments. |
Review details
Suppressed comments (1)
src/api/buildData.ts:25
- This fixed list never includes
ManuscriptActions.handleSuggestion.getCapabilitiesmaps that action directly tocan.handleSuggestion, andSuggestionActionsonly renders Accept when it is enabled, so this migration removes approval of every suggestion. Include the action if the static owner permissions are meant to preserve the existing editor behavior.
const STATIC_MANUSCRIPT_PERMITTED_ACTIONS: ManuscriptActions[] = [
ManuscriptActions.rejectOwnSuggestion,
ManuscriptActions.handleOwnComments,
ManuscriptActions.resolveOwnComment,
- Files reviewed: 11/11 changed files
- Comments generated: 6
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+104
to
+108
| getUser = async (): Promise<UserProfile> => ({ | ||
| _id: 'static-user-profile-id', | ||
| userID: 'static-user-id', | ||
| connectID: 'static-connect-id', | ||
| }) |
| const join = async () => { | ||
| try { | ||
| const token = await this.getAuthToken() | ||
| const url = `${base.href}${path}?token=${encodeURIComponent(token ?? '')}` |
Comment on lines
+271
to
275
| const join = async () => { | ||
| try { | ||
| const token = await this.getAuthToken() | ||
| const url = `${base.href}${path}?token=${encodeURIComponent(token ?? '')}` | ||
| ws = new WebSocket(url) |
| const profilesById = new Map() | ||
| profilesById.set(user._id, user) | ||
| const profiles = await api.getUserProfiles(projectID) | ||
| const profiles = await api.getUserProfiles(docID) |
Comment on lines
+122
to
+126
| userRole: ProjectRole.owner, | ||
| ...users, | ||
| ...state, | ||
| ...doc, | ||
| project, | ||
| manuscriptPermittedActions, | ||
| manuscriptPermittedActions: STATIC_MANUSCRIPT_PERMITTED_ACTIONS, |
| const [project, document] = await Promise.all([ | ||
| api.getProject(projectID), | ||
| api.getDocument(projectID, manuscriptID), | ||
| api.getProject(docID), |
2 tasks
- state.project/refreshProject and Api.getProject/getProjectPermittedActions were dead (never read) — removed, along with the now-unused Project import. - WMsPermittedActions/manuscriptPermittedActions collapse into a single permittedActions: string[], stored as-is from host app props. The two former enums (body-editor's Actions, transform's ManuscriptActions) merge into one local Actions enum in lib/capabilities.tsx, since there's no longer a distinction to preserve. - The current user and collaborator list now come entirely from host app props instead of being fetched: EditorAppProps gains a required currentUser: User prop alongside the existing users?: User[] (state.users, renamed from hostUsers). HostUser and UserProfile-typed state (user/collaborators/collaboratorsById) collapse into one local User type (HostUser's fields minus the unused connectId). Api.getUser/getUserProfiles and buildData's user-fetching gate are removed accordingly. - getCurrentUser is dropped from the body-editor props object built in use-create-editor.ts — it was never read on the body-editor side either (see companion body-editor change removing it from EditorProps). - useGetUserName converts from a hook to a plain getUserName(users, userID) function in lib/get-user-name.ts: it was only ever a thin lookup over state.users, and both call sites already subscribe via useStore themselves, so a second hook subscription added nothing. - userRole/ProjectRole/getUserRole (lib/roles.ts) are removed entirely — the only value ever assigned was a hardcoded ProjectRole.owner, nothing read it, and the underlying role computation depended on the already-removed state.project. Requires manuscripts-body-editor's LEAN-6048 (Actions enum + getCurrentUser removal) to typecheck clean end-to-end — verified locally against a linked build; the registry-published body-editor is one version behind until that lands. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Every real consumer of state.user (comment/suggestion ownership checks, the userID attr written onto new comments, the userID passed to body-editor) only ever read .id — so the User object gave nothing over the userID field that was already declared (if unused) on state. EditorAppProps.currentUser/currentUserID collapses the same way into userID: string. state.users (the roster, used by getUserName) keeps the full User shape, now imported from @manuscripts/transform instead of being defined locally. Requires manuscripts-transform's LEAN-6048 (User export replacing UserProfile) to typecheck clean — verified locally against a linked build; the registry-published transform is one version behind until that lands. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
docID, not a(projectID, manuscriptID)pair.EditorAppPropsand everything that threads through the store now take a singledocID:Api.ts's doc-related calls,Store's state,BasicSource,ApiSource,buildData,buildUtilities,StepsExchanger, anduse-create-editorall collapse to onedocIDfield.api.getProject/getUserProfiles/getProjectPermittedActionswere never part of manuscripts-cf-worker's/v3API to begin with (pre-existing gap) — they now just reuse the samedocIDvalue whereprojectIDused to go.use-create-editor.tsno longer passesprojectIDinto@manuscripts/body-editor'suseEditor()— that prop was dead there (see manuscripts-body-editor's matching PR removing it fromEditorProps).getUser()returns a static profile,buildData()'suserRoleis a staticProjectRole.owner,manuscriptPermittedActionsis a fixed list, anduseGetUserName()returns a static'User'.Note
Depends on manuscripts-body-editor's
projectIDremoval being published — until then,pnpm typecheck(and this repo's pre-push hook) fails on the missingprojectIDprop, since the currently-published body-editor still requires it. Verified locally against a linked build of that change; pushed here with--no-verifyat the user's request since this is a known, expected, temporary cross-repo ordering issue.Test plan
pnpm test— 13/13 passpnpm typecheckpasses once linked against manuscripts-body-editor's matching change🤖 Generated with Claude Code