feat: abrir clipboards com códigos personalizados - #9
Conversation
📝 WalkthroughWalkthroughThe text relay adds custom 1–16 character room codes and an atomic open-or-create endpoint. Active codes remain unique, deleted codes can be reused, PIN-protected rooms retain cookie and PIN validation, and the desktop client exposes the new flow. ChangesCustom text-room lifecycle
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant TextSession
participant text-client
participant text-session-service
participant text-rooms-repository
TextSession->>text-client: openRoom(code, pin)
text-client->>text-session-service: POST /api/text/:code/open
text-session-service->>text-rooms-repository: create or find active room
text-rooms-repository-->>text-session-service: room identity and access state
text-session-service-->>text-client: code, created, protected, expiration
text-client-->>TextSession: OpenRoomResult
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/server/text-session-service.test.ts (1)
315-329: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the collision test create a real insert conflict.
InMemoryTextRoomsRepository.createTextRoom()does not yield between Line 62 and Line 77. The first request completes its duplicate check and appends the room before the second request runs. This test does not cover the retry path atsrc/server/text-session-service.tsLines 150-155.Use a barrier-based repository double, or an integration test with PostgreSQL, where both calls observe no room and exactly one insert returns
null.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/text-session-service.test.ts` around lines 315 - 329, Update the concurrent-open test around startTestServer to use a repository double that synchronizes both createTextRoom calls after the duplicate check, ensuring both observe no existing room before proceeding. Make exactly one insert return null and the other succeed, then preserve the assertions for two successful responses with one created and one reused result so the retry path in the text session service is exercised.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@migrations/004_reusable_text_room_codes.sql`:
- Around line 1-10: Revise the text_rooms migration to avoid table rewrites and
long write locks: add id as nullable without a volatile default, set the default
only for future inserts, backfill existing null ids in batches, and
validate/enforce non-nullability using an online constraint approach. Move the
partial index creation to a non-transactional migration step and build it
concurrently, updating the migration structure around the text_rooms DDL and
index statements.
In `@src/server/text-session-service.ts`:
- Around line 115-131: Move the maxSessions capacity check and createTextRoom
call into a single serialized repository operation, replacing the separate
count-then-create sequence in the current room-creation flow. Update POST
/api/text to use the same repository operation so all text-room creation paths
enforce the limit atomically while preserving the existing 503 session_limit
response.
---
Nitpick comments:
In `@src/server/text-session-service.test.ts`:
- Around line 315-329: Update the concurrent-open test around startTestServer to
use a repository double that synchronizes both createTextRoom calls after the
duplicate check, ensuring both observe no existing room before proceeding. Make
exactly one insert return null and the other succeed, then preserve the
assertions for two successful responses with one created and one reused result
so the retry path in the text session service is exercised.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7a701255-28f1-4e18-829f-2bfda2f9e341
📒 Files selected for processing (10)
README.mdmigrations/004_reusable_text_room_codes.sqlsrc/desktop/TextSession.tsxsrc/desktop/text-client.tssrc/server/ids.test.tssrc/server/ids.tssrc/server/schema.tssrc/server/text-rooms-repository.tssrc/server/text-session-service.test.tssrc/server/text-session-service.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 10 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
|
@coderabbitai review |
|
Findings fixed in df7a684; CodeRabbit re-verified and resolved the review threads on the updated commit.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/server/text-session-service.test.ts (3)
348-364: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThis test covers route behavior, not repository atomicity.
BarrierTextRoomsRepositoryawaits the barrier before it delegates tosuper.createTextRoomWithinLimit. The check-and-insert body then runs without anyawaitinside it, so the event loop never interleaves two bodies. The test proves that two concurrent route calls yield one200and one503 session_limit. It cannot detect a lost capacity check in the real SQL path, which depends onpg_advisory_xact_lockinsrc/server/text-rooms-repository.ts.Add an integration test against PostgreSQL to cover the transaction path, or state this limitation in a comment so a later reader does not treat this test as proof of atomicity.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/text-session-service.test.ts` around lines 348 - 364, Clarify the test named “enforces the session limit across concurrent room creations” by adding a comment that it verifies route-level outcomes only and does not prove repository or PostgreSQL atomicity. Explicitly note that the real transaction path relies on pg_advisory_xact_lock in TextRoomsRepository, or replace/augment the test with a PostgreSQL integration test covering that path.
142-162: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
BarrierTextRoomsRepositoryhangs when a test calls creation only once.The barrier resolves only when
arrivalsreaches exactly 2. A test that issues a single creation waits onawait this.barrierforever, and the failure appears as a suite timeout rather than a clear assertion. Make the arrival count explicit in the constructor, and add a timeout so a mismatch fails fast.♻️ Proposed refactor
class BarrierTextRoomsRepository extends InMemoryTextRoomsRepository { private arrivals = 0; private releaseBarrier: (() => void) | null = null; - private readonly barrier = new Promise<void>((resolve) => { - this.releaseBarrier = resolve; - }); + private readonly barrier: Promise<void>; + + constructor(private readonly expectedArrivals = 2, private readonly timeoutMs = 1000) { + super(); + this.barrier = new Promise<void>((resolve, reject) => { + this.releaseBarrier = resolve; + setTimeout( + () => reject(new Error(`barrier expected ${expectedArrivals} arrivals, saw ${this.arrivals}`)), + timeoutMs, + ).unref?.(); + }); + } override async createTextRoomWithinLimit( input: CreateTextRoomInput, maxSessions: number, now: Date, ): Promise<TextRoomCreationResult> { this.arrivals += 1; - if (this.arrivals === 2) { + if (this.arrivals >= this.expectedArrivals) { this.releaseBarrier?.(); } await this.barrier; return super.createTextRoomWithinLimit(input, maxSessions, now); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/text-session-service.test.ts` around lines 142 - 162, Update BarrierTextRoomsRepository to accept an explicit expected arrival count in its constructor, release the barrier when arrivals reach that configured count, and add a bounded timeout to the barrier wait so insufficient arrivals fail fast. Update its instantiation sites to provide the intended count while preserving the existing synchronization behavior.
659-663: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winThe sweep assertion depends on wall-clock timing.
startTextSessionSweepschedulesvoid sweepExpiredRooms(...)on a 5 ms interval and does not await it.Bun.sleep(20)gives roughly four ticks on an idle machine. Under CI load, the deletion can miss the window and Line 663 fails intermittently. Poll for the condition instead of sleeping for a fixed period.♻️ Proposed refactor
const timer = startTextSessionSweep(repository, 5, () => new Date(clock.current)); - await Bun.sleep(20); - clearInterval(timer); - - expect(await repository.findTextRoomByCode("ROOM01")).toBeNull(); + try { + const deadline = Date.now() + 2000; + while (await repository.findTextRoomByCode("ROOM01")) { + if (Date.now() > deadline) { + throw new Error("sweep did not delete the expired room"); + } + await Bun.sleep(5); + } + } finally { + clearInterval(timer); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/text-session-service.test.ts` around lines 659 - 663, Replace the fixed Bun.sleep(20) delay in the startTextSessionSweep test with polling that repeatedly checks repository.findTextRoomByCode("ROOM01") until it returns null, using the test’s supported retry or wait utility and a bounded timeout. Keep clearing the interval after the condition is observed, then assert the room is absent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@migrations/006_text_room_uuid_primary_key.sql`:
- Around line 19-26: In migrations/006_text_room_uuid_primary_key.sql lines
19-26, update the primary-key migration so text_rooms_id_not_null is dropped
only when the primary-key swap actually runs, and raise an exception for any
unexpected primary-key column instead of completing with id nullable. In
migrations/005_text_room_active_code_index.sql lines 3-5, add a standalone
removal of any existing text_rooms_active_code_unique index before the
concurrent index creation, ensuring the rebuild cannot be skipped by IF NOT
EXISTS.
In `@src/server/text-rooms-repository.ts`:
- Around line 48-67: Protect the active-room transitions in
scheduleTextRoomExpiry() and markTextRoomActive() with the same capacity
advisory lock used by creation. In the creation transaction, remove the
redundant existingRows code lookup and rely on the partial unique index for
conflicts. Replace hashtext('quickdrop:text-room-capacity') with a fixed
advisory-lock key shared by all these paths, preserving capacity serialization.
---
Nitpick comments:
In `@src/server/text-session-service.test.ts`:
- Around line 348-364: Clarify the test named “enforces the session limit across
concurrent room creations” by adding a comment that it verifies route-level
outcomes only and does not prove repository or PostgreSQL atomicity. Explicitly
note that the real transaction path relies on pg_advisory_xact_lock in
TextRoomsRepository, or replace/augment the test with a PostgreSQL integration
test covering that path.
- Around line 142-162: Update BarrierTextRoomsRepository to accept an explicit
expected arrival count in its constructor, release the barrier when arrivals
reach that configured count, and add a bounded timeout to the barrier wait so
insufficient arrivals fail fast. Update its instantiation sites to provide the
intended count while preserving the existing synchronization behavior.
- Around line 659-663: Replace the fixed Bun.sleep(20) delay in the
startTextSessionSweep test with polling that repeatedly checks
repository.findTextRoomByCode("ROOM01") until it returns null, using the test’s
supported retry or wait utility and a bounded timeout. Keep clearing the
interval after the condition is observed, then assert the room is absent.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 976f9af8-1e34-43a1-b2e6-5ab940617504
📒 Files selected for processing (6)
migrations/004_reusable_text_room_codes.sqlmigrations/005_text_room_active_code_index.sqlmigrations/006_text_room_uuid_primary_key.sqlsrc/server/text-rooms-repository.tssrc/server/text-session-service.test.tssrc/server/text-session-service.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/server/text-session-service.ts
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="migrations/005_text_room_active_code_index.sql">
<violation number="1" location="migrations/005_text_room_active_code_index.sql:3">
P2: If a prior `CREATE INDEX CONCURRENTLY` attempt failed, it leaves an INVALID index entry in the catalog under this name. Because this statement only uses `IF NOT EXISTS`, a rerun of the migration will silently skip rebuilding the invalid index, and `text_rooms_active_code_unique` will never actually enforce the intended uniqueness constraint on active codes. Drop any existing (possibly invalid) index by that name in a standalone statement before running the concurrent create.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
|
@coderabbitai review |
|
1 participant
Problema
O formulário atual sugere que o usuário pode escolher um código, mas esse campo só entra em salas existentes. Para criar uma sala, é necessário usar outro botão e transportar um código aleatório de seis caracteres — justamente o que é difícil quando as máquinas não compartilham clipboard.
Solução
Entrega o primeiro fluxo funcional do novo clipboard de texto:
A,DEV,SERVER-1);Backend e banco
POST /api/text/:code/open, retornando{ code, created, protected };text_rooms.idpassa a ser UUID/PK;code where deleted_at is null;INSERT ... ON CONFLICT DO NOTHINGusa o PostgreSQL como fonte da verdade na corrida de criação.Compatibilidade
POST /api/textcontinua gerando código aleatório;POST /api/text/:code/accesscontinua disponível;Validação
bun run typecheck)bun run db:check)Fora desta PR
As próximas PRs tratam a experiência dentro do clipboard: botão primário Copiar texto, URL canônica
/t/CODIGO, limpeza/TTL curto, timeline, CLI e integração com tray/bar.Summary by cubic
Abre clipboards de texto com códigos personalizados em uma única ação. Antes o fluxo separava “entrar” (apenas códigos existentes de 6 caracteres) de “criar”; agora “Abrir” aceita 1–16 letras/números/_/-, cria se não existir e entra se já existir, com PIN opcional e reuso do código após expirar.
text_rooms.idvira UUID/PK;codepermanece NOT NULL com índice único parcial; exclusões e limpezas passam a operar por id. As migrações endurecem reexecuções e evitam locks longos ao (re)criar o índice de código ativo de forma concorrente.A–Z,0–9,_,-), primário “Abrir”, secundário “Opções de privacidade” (PIN) e “Gerar código aleatório”. WebSocket ePOST /api/text/:code/accesspermanecem compatíveis.Rollout/migração
Written for commit 8b9f9b2. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation