-
Notifications
You must be signed in to change notification settings - Fork 0
feat: implement V2 Advanced Rehearsal Collaboration Features #158
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from 5 commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
8acc846
feat: implement V1.1 metadata-only local handoff
seonghobae 2aef571
feat: implement V2 Advanced Rehearsal Collaboration Features
seonghobae 052280a
Merge branch 'develop' into feature/issue-152-collaboration
seonghobae 538a174
Address review feedback: annotations, deep links, validation, workspa…
seonghobae b4816cb
Address review feedback: file validation, audio resolution, markdown …
seonghobae aa7a10d
fix: package-lock and type dependencies
seonghobae 83c8315
Merge remote-tracking branch 'origin/develop' into feature/issue-150-…
seonghobae 37f4976
Merge remote-tracking branch 'origin/develop' into feature/issue-152-…
seonghobae adb92cb
Merge branch 'develop' into feature/issue-150-local-handoff
seonghobae 857c062
Merge branch 'develop' into feature/issue-152-collaboration
seonghobae 442084a
fix(tests): resolve failing archive import/export tests
seonghobae 1642c56
Merge PR 156 to fix typecheck dependencies
seonghobae 682bf7f
fix(desktop): support RehearsalWorkspace in App.tsx
seonghobae 4ca24df
fix(lint): resolve lint errors in PR 158
seonghobae 462c1ee
fix(deps): regenerate lockfile to fix CI
seonghobae 9f44679
fix(deps): regenerate lockfile to fix CI
seonghobae 2c17a0e
Merge branch 'develop' into feature/issue-152-collaboration
seonghobae 38cfa3c
Merge branch 'develop' into feature/issue-152-collaboration
seonghobae 22e19bb
Merge branch 'develop' into feature/issue-152-collaboration
seonghobae a688f45
Merge branch 'develop' into feature/issue-152-collaboration
seonghobae File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| import { Annotation } from "@bandscope/shared-types"; | ||
|
|
||
| /** | ||
| * Merges two arrays of annotations, keeping unique ones and sorting by timestamp. | ||
| * | ||
| * @param existing - The existing annotations. | ||
| * @param incoming - The incoming annotations. | ||
| * @returns The merged annotations array. | ||
| */ | ||
| export function mergeAnnotations(existing: Annotation[] = [], incoming: Annotation[] = []): Annotation[] { | ||
| const merged = [...existing]; | ||
| const existingIds = new Set(existing.map((a) => a.id)); | ||
|
|
||
| for (const ann of incoming) { | ||
| if (!existingIds.has(ann.id)) { | ||
| merged.push(ann); | ||
| existingIds.add(ann.id); | ||
| } | ||
| } | ||
|
|
||
| const mergedWithIndex = merged.map((item, index) => ({ item, index })); | ||
| mergedWithIndex.sort((a, b) => { | ||
| const ta = Date.parse(a.item.timestamp); | ||
| const tb = Date.parse(b.item.timestamp); | ||
| const tsa = Number.isFinite(ta) ? ta : 0; | ||
| const tsb = Number.isFinite(tb) ? tb : 0; | ||
| if (tsa !== tsb) return tsa - tsb; | ||
| return a.index - b.index; | ||
| }); | ||
| return mergedWithIndex.map(x => x.item); | ||
| } |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| import { validateBandScopeUri } from "@bandscope/shared-types"; | ||
|
|
||
| /** | ||
| * Parsed details of a deep link | ||
| */ | ||
| export type ParsedDeepLink = { | ||
| songId: string; | ||
| sectionId: string; | ||
| }; | ||
|
|
||
| /** | ||
| * Parse a deep link URI | ||
| * | ||
| * @param uri - The URI to parse | ||
| * @returns The parsed deep link or null | ||
| */ | ||
| export function parseDeepLink(uri: string): ParsedDeepLink | null { | ||
| if (!validateBandScopeUri(uri)) { | ||
| return null; | ||
| } | ||
|
|
||
| // bandscope://song/[songId]/section/[sectionId] | ||
| const match = uri.match(/^bandscope:\/\/song\/([a-zA-Z0-9-]+)\/section\/([a-zA-Z0-9-]+)$/); | ||
| if (!match) { | ||
| return null; | ||
| } | ||
| const [, songId, sectionId] = match; | ||
| if (!songId || !sectionId) { | ||
| return null; | ||
| } | ||
|
|
||
| return { songId, sectionId }; | ||
| } | ||
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| <!-- /autoplan restore point: /Users/seonghobae/.gstack/projects//feature-issue-152-collaboration-autoplan-restore-20260425-230327.md --> | ||
| # Plan: V2 Advanced Rehearsal Collaboration Features | ||
|
|
||
| Status: APPROVED | ||
|
|
||
| ## Problem Statement | ||
| With V1 providing individual rehearsal certainty via part stems and section guidance, and V1.1 enabling metadata-only local handoff, bands can now share static rehearsal artifacts. However, a major pain point remains: rehearsal preparation is inherently conversational and dynamic. | ||
| Band leaders need to communicate specific simplification requirements ("play root notes only here"), suggest transpositions, or flag difficult transitions. Currently, this collaboration happens outside the app (in WhatsApp or physical notes), leading to disconnected workflows where the context is lost when opening BandScope. | ||
|
|
||
| ## Scope | ||
| - **Assignment Semantics**: Allow assigning specific roles to specific band members within the shared workspace. | ||
| - **Contextual Comments**: Enable adding text annotations directly to specific sections or roles in the `SongRehearsalPack` (e.g., "Simplify bassline in Chorus 2"). | ||
|
|
||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| ## Out of Scope | ||
| - **Approvals & Status**: Let band members mark their assigned parts as "Ready" or "Needs Help." | ||
| - **Cloud Sync Backbone**: Introduce an opt-in cloud synchronization mechanism to replace local file sharing, allowing real-time or near-real-time updates to the rehearsal workspace. | ||
| - Built-in audio/video calling. | ||
| - Complex branching/version control of rehearsal workspaces. | ||
| - Deep integration with external task managers (Jira, Trello). | ||
|
|
||
|
|
||
| ## CEO Review Completion Summary | ||
| - Mode: SCOPE REDUCTION | ||
| - Scope Decisions: | ||
| - Approved: Scrap the Cloud Sync Backbone entirely to protect the local-first wedge and avoid massive operational/security overhead. Rely on existing V1.1 local handoff. | ||
| - Approved: Scrap formal "Status/Approval" workflows ("Ready", "Needs Help"). Bands are not enterprises; do not build Jira for musicians. | ||
| - Approved: Focus exclusively on **Contextual Annotations** (e.g., "play root notes here") that save to the local file. | ||
| - Approved: Add **Deep Links / Annotated Snippets** to embrace WhatsApp/group chats rather than fighting them. A band leader can copy a rich text snippet that deep-links into the local BandScope app at the exact section. | ||
| - Dual Voices: `[single-model]` (Codex unavailable, Claude subagent provided 5 critical/high findings). | ||
|
|
||
|
|
||
| ## Design UI/UX Specifications | ||
|
|
||
| ### Information Architecture & Interactions | ||
| - **Annotations UI**: Live in a persistent but collapsible right-side drawer, or as tightly packed inline badges above section headers, ensuring the music timeline remains primary. | ||
| - **Triggers**: Add an "Add Note" icon button that appears on hover next to section headers and role rows. Add a "Copy Link" action to the ellipsis menu for every section. | ||
| - **Role Assignment**: Assignment acts as a visual highlight, not a hard filter. Highlighting a role dims other instruments slightly but keeps them visible for rehearsal awareness. | ||
|
|
||
| ### Interaction States | ||
| - **Deep-Link Error State**: If a deep link opens and the local `.bndscp` file is missing, show an empty state: "Song not found. Ask the leader to share the .bandscope file first" with a giant "Import File" CTA. | ||
| - **Empty Annotations**: Zero-data state for the annotation panel: "No notes for this section." | ||
|
|
||
| ### User Journey | ||
| - **Handoff Snippet**: The copied deep link must include a plain-text fallback. Example: | ||
| `We're struggling with the bridge. Play root notes only. 1. Open the song file in BandScope. 2. Click this link: bandscope://song/123/section/bridge` | ||
|
|
||
| ## Design Review Completion Summary | ||
| - Initial Score: 3/10 | ||
| - Final Score: 10/10 | ||
| - Decisions Made: 5 structural issues fixed via Claude Subagent. | ||
| - Dual Voices: `[single-model]` (Codex unavailable). | ||
|
|
||
|
|
||
| ## Engineering Review Completion Summary | ||
| - Initial Assessment: Critical gaps in local sync conflict resolution, URL scheme security, and OS integration testing. | ||
| - Final State: Deep-link security bounded, conflict resolution scoped to append-only logs, and E2E testing mandated. | ||
| - Dual Voices: `[single-model]` (Codex unavailable, Claude subagent provided 5 critical/high findings). | ||
|
|
||
| ### Architecture & Conflict Resolution | ||
| - **Merge Strategy**: Because we dropped cloud sync, `.bndscp` files will diverge. Annotations must be modeled as an append-only log (with UUIDs and timestamps). When opening a shared file for an existing song, the app must merge new annotations into the local state instead of blindly overwriting. | ||
| - **Dimming Performance**: "Highlighting a role" MUST bypass React re-renders of the heavy waveform components. Use CSS variables or opacity transitions on parent containers to dim non-active tracks purely via the GPU. | ||
|
|
||
| # | ||
| ## Security Notes | ||
| ### Attack Surface | ||
| Custom URI payloads (e.g., `bandscope://song/123/section/bridge`) are untrusted external input crossing the OS-to-App boundary. | ||
|
|
||
| ### Trust Boundary | ||
| The deep-link parser logic forms a strict boundary between OS URL handling and React component rendering. | ||
|
|
||
| ### Mitigations | ||
| Strict regex validation is enforced for deep link payloads (e.g., IDs must match `^[a-zA-Z0-9-]+$`). The URI payload is NEVER used directly in file system calls or raw DOM injection to prevent Local File Inclusion (LFI) and XSS. | ||
|
|
||
| ### Realistic Threats | ||
| Maliciously crafted `bandscope://` links intended to execute arbitrary local files or run XSS payloads within the UI context. | ||
|
|
||
| ### Remaining Risk | ||
| Minor risk of deep link parser denial-of-service on extreme string lengths, but limited to individual application instances. | ||
|
|
||
| ### Test Points | ||
| - Malicious URI payload injections via hash routes. | ||
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
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
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
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.