Skip to content

feat(workspace): copy tonight's first check for the band chat - #1120

Open
seonghobae wants to merge 6 commits into
developfrom
feat/workspace-copy-first-action
Open

feat(workspace): copy tonight's first check for the band chat#1120
seonghobae wants to merge 6 commits into
developfrom
feat/workspace-copy-first-action

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Summary

The ready workspace already names tonight's first playable range. A bandmate still had to retype that sentence into KakaoTalk, Discord, or Messages before rehearsal. This PR copies the same localized first-check sentence already shown on the map. Blank or non-string payloads fail closed. A blocked clipboard names the next action (select tonight's first check and copy it) instead of inventing a span or dumping the failure.

Does not mix with #811, #828, #897, #1114, #1115, or #1118. firstRangeSqueeze remains the span authority. #828 still owns MIR/#770.

Exact current identity

  • Protected target: develop@749511c3ad4000090048718f685c6bee6b3d2c25.
  • Exact current head: 038c367feceacde1d6d311fb8512f89f9ac1e898.
  • Branch: feat/workspace-copy-first-action.

All predecessor-head check/review results are historical only.

Current exact scope

  • copyFirstRangeAction writes only a non-blank string the workspace already rendered.
  • Prefer navigator.clipboard.writeText on a user click; fall back to a hidden textarea + document.execCommand("copy") only when that API is absent or rejects.
  • Remove the fallback textarea immediately after the attempt and restore the previously focused connected element best-effort.
  • Clipboard errors are swallowed; the UI never renders exception text.
  • English and Korean next-action copy live on the first-range card.
  • Workspace assigns every copy attempt a monotonically advancing request sequence and the exact sentence requested. A sentence change invalidates every pending request before paint; only the latest request for the still-displayed sentence may publish copyStatus.

Root-cause verification and repairs

The predecessor exact head d190f320a0c567095231659d3d80d1c1e6b02972 failed release-preflight in ./scripts/harness/quickcheck.sh. The first repository-owned failure was ESLint jsdoc/require-jsdoc in copyFirstRangeAction.ts. Head b3048b709c98a23df53de7a88ee49ccbf432d9de documented those exact clipboard/result boundaries without lint suppression or rule weakening.

Fresh review inspection then confirmed a separate functional race: an older async clipboard request could complete after a role/sentence change or after a newer copy request and overwrite the current card's status. The current implementation fixes that at the mounted Workspace boundary rather than disabling repeated copy attempts or hiding the review finding.

Workspace.copy-request-order.test.tsx adds executable deferred-promise regressions for both cases:

  • two overlapping requests for one sentence finish in reverse order; the newest successful result remains authoritative when the older request later fails;
  • a pending request becomes stale when role selection changes the displayed sentence; its later failure cannot publish an unavailable status for the new sentence.

No force push, bypass, self-approval, workflow weakening, or stale evidence transfer was used.

Verification

  • predecessor release-preflight failure was inspected to the failing job/log and its JSDoc root cause was repaired in source.
  • valid review-driven stale-request race is repaired in production code with dedicated deferred-promise tests on the current branch.
  • fresh exact-head repository/central checks for 038c367f… must become terminal-success.
  • qualifying independent non-author approval must apply to the unchanged exact head.

Security Notes

Attack surface

Copied text is derived from untrusted analysis payloads: role names, section labels, and scientific-pitch range labels. Clipboard write is a local user-gesture side channel.

Trust boundary

firstRangeSqueeze remains the span authority. copyFirstRangeAction only writes a non-blank string that the workspace already rendered. It does not invent a span, read the clipboard, or dereference files or URLs. Request-order state remains renderer-local and carries no external authority.

Mitigations

  • Reject blank, whitespace-only, and non-string payloads before any write.
  • Do not log, toast, or render clipboard exception messages.
  • Keep English and Korean next-action copy on the card.
  • Restore prior focus best-effort after the legacy textarea fallback without converting copy success into a focus error.
  • Reject stale async completion at the UI state-publication boundary by exact request sequence + requested sentence.

Test points

  • copyFirstRangeAction.test.ts proves fail-closed blank payloads, exact-text writes, redacted writer failures, clipboard API success, execCommand fallback, focus restoration, and unavailable behavior.
  • Workspace.test.tsx proves the button names tonight's first check, writes the clash sentence, copies the missing-range next action, localizes the Korean control, and hides clipboard errors.
  • Workspace.copy-request-order.test.tsx proves reverse completion order and sentence-change invalidation.

Dependency and Supply Chain

i18n impact

  • Korean and English locale impact was updated.
  • workspaceFirstRangeCopyAction, workspaceFirstRangeCopied, and workspaceFirstRangeCopyUnavailable remain aligned.

Merge gate

Do not merge until this unchanged exact head has every applicable repository and central CI/build/release/security/SAST/SBOM/supply-chain/coverage/review gate terminal-success, zero valid unresolved findings, and a qualifying independent non-author approval under live protection. Queued, pending, neutral, skipped-required, cancelled, failed, predecessor-head, protected-base, model-only, self/author, or administrative-bypass evidence is non-passing.

The ready map already names tonight's first playable range. Players still
had to retype that sentence into KakaoTalk, Discord, or Messages. Copy the
same localized next-action sentence onto the clipboard, fail closed on
blank payloads, and name a manual copy when the window blocks the write.
@coderabbitai

coderabbitai Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

준비된 Workspace에 오늘의 첫 악기 확인 문장을 클립보드로 복사하는 기능이 추가되었습니다. Clipboard API와 execCommand 폴백을 지원하며, 영어·한국어 상태 메시지와 테스트 및 관련 문서가 추가되었습니다.

Changes

첫 확인 문장 복사

Layer / File(s) Summary
클립보드 복사 액션
apps/desktop/src/features/workspace/copyFirstRangeAction.ts, apps/desktop/src/features/workspace/copyFirstRangeAction.test.ts
copyFirstRangeAction이 비공백 문자열만 처리합니다. Clipboard API를 우선 사용하고, 실패하면 숨겨진 textareaexecCommand("copy")를 사용합니다. 성공, 실패, 포커스 복원 및 입력 검증을 테스트합니다.
Workspace 복사 UI 및 검증
apps/desktop/src/features/workspace/Workspace.tsx, apps/desktop/src/features/workspace/Workspace.test.tsx, apps/desktop/src/locales/*/common.json
Workspace에 복사 버튼과 aria-live 상태 메시지가 추가되었습니다. 영어와 한국어 문구를 제공하며, 성공·차단·이름 없는 범위의 동작을 검증합니다.
기능 설명 문서 반영
AGENTS.md, ARCHITECTURE.md, CHANGELOG.md, CLAUDE.md, docs/architecture/overview.md, docs/doctoring/copy-first-range-action.md
첫 악기 확인 문장의 밴드 채팅 복사 동작과 copyFirstRangeAction의 폴백 및 보안 경계를 문서화했습니다.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: 🟡 Moderate · up to d190f

The PR adds copying for the displayed first-check sentence, but an older copy attempt can still mark a newer sentence as copied or unavailable, and the new helper may fail the repository’s documentation lint rule. Merge readiness therefore requires fixing or explicitly accepting these bounded issues.

Sequence Diagram(s)

sequenceDiagram
  actor 연주자
  participant Workspace
  participant copyFirstRangeAction
  participant 클립보드
  연주자->>Workspace: 첫 확인 복사 버튼 클릭
  Workspace->>copyFirstRangeAction: firstRangeCopy 전달
  copyFirstRangeAction->>클립보드: Clipboard API 또는 execCommand로 복사
  클립보드-->>copyFirstRangeAction: copied 또는 unavailable 반환
  copyFirstRangeAction-->>Workspace: 결과 상태 전달
  Workspace-->>연주자: 성공 또는 수동 복사 안내 표시
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 4 files. (8 skipped: 8 u…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 워크스페이스에서 오늘 밤 첫 악기 점검 문장을 밴드 채팅에 복사하는 핵심 변경 사항을 정확하고 간결하게 설명합니다.
Full details: Docstring Coverage

Explanation

Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 4 files. (8 skipped: 8 unsupported.)

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/workspace-copy-first-action

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

devin-ai-integration[bot]

This comment was marked as resolved.

seonghobae commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator Author

@opencode-agent Become the sole writer for the canonical feat/workspace-copy-first-action lane, now bound to exact head d190f320a0c567095231659d3d80d1c1e6b02972 and protected develop@749511c3ad4000090048718f685c6bee6b3d2c25. Do not create a competing branch/PR.

Fresh current-head review verification found the remaining BandScope-owned correctness defect in Workspace.tsx: handleCopyFirstRange awaits copyFirstRangeAction(firstRangeCopy) and unconditionally writes the result into shared copyStatus. If the active role/firstRangeCopy changes while the clipboard promise is pending, the old completion labels the newly displayed check as copied/unavailable even though that sentence was never attempted. Overlapping copy attempts have the same last-completion-wins race.

Use TDD: add deferred-promise RED regressions that (1) start copy A, switch the displayed role/check to B, then resolve A and prove B remains idle; and (2) start attempts A then B for the same displayed sentence, resolve B first and A last, and prove only the latest request can determine the final status. Implement the smallest causal fix at the Workspace ownership boundary: bind each request to both a monotonic/current request identity and the exact sentence, invalidate pending work when firstRangeCopy changes, and commit a result only when it is still the latest request for the currently displayed sentence. Do not move clipboard authority into unrelated layers.

A later same-head CodeRabbit finding is also verified against source and eslint.config.js: the returned writeText arrow functions in both clipboardApiWriter() and execCommandWriter() have no JSDoc, while the repository explicitly requires JSDoc for ArrowFunctionExpression under apps/desktop/src/**/*.{ts,tsx}. Add concise decision-relevant JSDoc directly before both writeText properties; do not alter the lint rule. Treat this as part of the same canonical repair before full verification.

Also preserve and verify the already-landed accessibility repair on this exact lane: commits 3593800… / d190f32… add a regression and restore the previously focused control after the hidden-textarea execCommand fallback. Keep that focus restoration best-effort so it cannot turn a successful clipboard copy into an error. Do not resolve current review threads until focused + full GREEN evidence exists on the successor exact head.

After the narrow repairs, run the focused desktop tests (including deferred role-change/overlap and focus restoration), lint/JSDoc, then full desktop/shared/analysis verification, exact 100% owned production coverage where exposed, and all current CI/security/SBOM/release/review gates. Refetch the successor head/base before changing thread/PR state; predecessor or queued evidence is non-passing. The PR body currently names 4faeac5… as its exact head even though the live branch/PR head is d190f32…; update the body only after the successor head is established so it does not advertise a stale identity.

coderabbitai[bot]

This comment was marked as resolved.

Copy link
Copy Markdown
Collaborator Author

@opencode-agent Rebind the sole-writer lease for the canonical feat/workspace-copy-first-action lane to exact current head b3048b709c98a23df53de7a88ee49ccbf432d9de and protected develop@749511c3ad4000090048718f685c6bee6b3d2c25. The intervening delta from d190f320a0c567095231659d3d80d1c1e6b02972 is the intended JSDoc-boundary repair only; it did not address the current state race.

Fresh exact-head source still has handleCopyFirstRange as setCopyStatus(await copyFirstRangeAction(firstRangeCopy)), with useEffect only resetting status after firstRangeCopy changes. The previously verified defect therefore remains current: an older pending copy can complete after the displayed role/check changes and write a stale copied/unavailable status into the new sentence, and two same-sentence attempts can resolve out of order so the older request overwrites the newer result.

Use TDD on this same branch: add deferred-promise RED regressions for (1) A starts, displayed first check changes to B, A resolves, B remains idle; and (2) A then B start for the same displayed sentence, B resolves first, A last, only B determines final status. Make the narrow Workspace-owned repair by binding completion to a monotonic request identity plus the exact current sentence and invalidating pending authority when that sentence changes. Preserve the already-landed focus-restoration repair and current clipboard trust boundary. Run focused desktop tests, lint/JSDoc, full quickcheck, coverage and exact-head hosted gates, then refetch the successor head/base before resolving findings or changing merge state. Do not create a competing branch/PR, weaken gates, or transfer predecessor evidence.

Copy link
Copy Markdown
Collaborator Author

Current-head dependency note for b3048b709c98a23df53de7a88ee49ccbf432d9de: GitHub Advanced Security's Trivy check is neutral with 1 configuration not found for .github/workflows/trivy.yml:trivy-fs-scan. This is not a clipboard/source-code finding in #1120; canonical root ownership is #1119, which adds ordinary PR-head Trivy SARIF coverage and is separately repairing the stale push-only policy test. Keep this leaf unchanged for that signal and regenerate exact-head evidence after the shared Trivy contract lands. Current ci, build-baseline, SBOM and aggregate Security Scan are still queued, so none is counted as success.

Copy link
Copy Markdown
Collaborator Author

@opencode-agent The prior sole-writer repair is complete; switch to independent review-only mode for exact current head 038c367feceacde1d6d311fb8512f89f9ac1e898 against protected develop@749511c3ad4000090048718f685c6bee6b3d2c25. Do not mutate the contributor branch unless a fresh current-head review proves a new BandScope-owned defect. Verify the clipboard fallback focus restoration, monotonic request identity + exact-sentence stale-completion guard, deferred overlap/role-change regressions, fail-closed blank copy, EN/KO accessible status, and current UI trust boundary. Submit an authenticated formal APPROVED or CHANGES_REQUESTED verdict anchored to this unchanged head through the existing review path. Do not reuse predecessor review evidence, self-approve, weaken gates, or create another PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant