refactor(core): Clean up workflow reviews backend (no-changelog)

[kgrhartlage]

Summary

Zero-risk cleanup of the workflow reviews backend. No behavior changes.

• Delete dead repository methods. setReviewers and the singular findByRequestId on the review author and reviewer repositories (@n8n/db) had no production callers. The dedicated test file for setReviewers is deleted with it. The @n8n/db package documents no compatibility policy for its public method surface, so nothing protects these. Note that the workflow repository keeps its own findByRequestId — that one is live.
• Merge two identical helpers. normalizeVersionDescription and normalizeReviewDescription in workflow-review-request.service.ts had the same body. They are now one normalizeDescription.
• Rename the lock. DbLock.WORKFLOW_REVIEW_REQUEST_CREATE becomes WORKFLOW_REVIEW_MUTATION, because it guards every review mutation, not only creation. The numeric value stays 1004 — that value is the actual Postgres advisory lock key.
• Fix stale comments. One comment named a status batch endpoint that no longer exists. Two comments claimed that rows arrive in insertion order because the query sorts by id. Ids are nanoids, so id order is not insertion order.
• Stop sharing a query builder across concurrent calls. findRequestsForWorkflow ran getRawAndEntities() and getCount() in a Promise.all on one builder. Each call mutates the builder while it runs. They now run in sequence.

The diff on workflow-review-request.service.ts looks larger than the change is. The shorter enum name lets two withLockContext(...) callbacks fit on one line, so Biome re-indents those two blocks. That part of the diff is whitespace only, and format:check requires it.

How to test

This is a refactor, so the tests are the test. No env vars, license, or feature flag are needed to verify it.

# @n8n/db: build, lint, unit tests
cd packages/@n8n/db && pnpm build && pnpm lint && pnpm test

# packages/cli: types, unit tests, integration tests (SQLite)
cd packages/cli && pnpm typecheck
pnpm vitest run src/modules/workflow-reviews.ee
pnpm test:integration --coverage.enabled=false src/modules/workflow-reviews.ee

Results locally: @n8n/db builds and lints clean with 552 tests passing; packages/cli typechecks clean with 242 unit tests and 247 integration tests passing.

To confirm the lock rename is name-only, check that the enum member still reads WORKFLOW_REVIEW_MUTATION = 1004 in packages/@n8n/db/src/services/db-lock.service.ts. A changed value would move the advisory lock key and let two review mutations run at once on Postgres.

Related Linear tickets, Github issues, and Community forum posts

https://linear.app/n8n/issue/LIGO-1034

Review / Merge checklist

• I have seen this code, I have run this code, and I take responsibility for this code.
• PR title and summary are descriptive. (conventions)
• Docs updated or follow-up ticket created.
• Tests included.
• PR Labeled with Backport to Beta, Backport to Stable, or Backport to v1 (if the PR is an urgent fix that needs to be backported)

🤖 PR Summary generated by AI

[n8n-assistant]

PR review overview

Based on ownership of the 15 changed files in this PR:

Ownership	Files owned	Share	Source code	Test files	Misc
@n8n-io/catalysts	15	100%	+204 / -254	+6 / -54	+0 / -0
Total	15	100%	+204 / -254	+6 / -54	+0 / -0

[kgrhartlage]

@cubic-dev-ai roast my PR

[cubic-dev-ai]

@cubic-dev-ai roast my PR

@kgrhartlage I have started the AI code review. It will take a few minutes to complete.

[cubic-dev-ai]

No issues found across 15 files

Architecture diagram

sequenceDiagram
    participant C as Client/Controller
    participant S as WorkflowReviewRequestService
    participant L as DbLockService
    participant R as WorkflowReviewRequestRepository
    participant DB as Postgres Database

    Note over C,DB: Workflow Review Mutation Flow (Create, Decide, or Update)

    C->>S: create() / decide() / updateVersion()
    
    S->>L: CHANGED: withLockContext(WORKFLOW_REVIEW_MUTATION)
    Note right of L: Lock key remains 1004 (PG Advisory Lock)

    L->>DB: Acquire Advisory Lock (1004)
    DB-->>L: Lock Acquired

    rect rgb(240, 240, 240)
        Note over S,DB: Inside Transaction Context
        S->>R: findById(requestId, ctx)
        R-->>S: current state
        
        alt Request is Updatable
            S->>S: NEW: normalizeDescription(description)
            S->>R: saveRequest(updatedData, ctx)
            R->>DB: UPDATE/INSERT
        else Request Stale/Invalid
            S-->>C: Throw NotFoundError / Conflict
        end
    end

    L->>DB: Release Advisory Lock
    S-->>C: RequestSummary

    Note over C,DB: Workflow Search/List Flow

    C->>R: findRequestsForWorkflow(options)
    R->>R: Create QueryBuilder (qb)

    rect rgb(230, 245, 255)
        Note over R,DB: CHANGED: Sequential Execution (Prevents Builder Mutation Race)
        R->>DB: qb.getRawAndEntities()
        DB-->>R: entities + raw data
        R->>DB: qb.getCount()
        DB-->>R: total count
    end

    R-->>C: { entities, count }

    Note over R,DB: NEW: Repository Cleanup
    Note right of R: Deleted: setReviewers()<br/>Deleted: authorRepo.findByRequestId()<br/>Deleted: reviewerRepo.findByRequestId()

Loading

_{Re-trigger cubic}

[codecov]

Codecov Report

❌ Patch coverage is 90.62500% with 6 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...flow-reviews.ee/workflow-review-request.service.ts	90.16%	3 Missing and 3 partials ⚠️

📢 Thoughts on this report? Let us know!

[cubic-dev-ai]

No issues found across 15 files

Architecture diagram

sequenceDiagram
    participant C as Client/Controller
    participant S as WorkflowReviewRequestService
    participant L as DbLockService
    participant R as WorkflowReviewRequestRepository
    participant DB as Postgres Database

    Note over C,DB: Workflow Review Mutation Flow (Create, Decide, or Update)

    C->>S: create() / decide() / updateVersion()
    
    S->>L: CHANGED: withLockContext(WORKFLOW_REVIEW_MUTATION)
    Note right of L: Lock key remains 1004 (PG Advisory Lock)

    L->>DB: Acquire Advisory Lock (1004)
    DB-->>L: Lock Acquired

    rect rgb(240, 240, 240)
        Note over S,DB: Inside Transaction Context
        S->>R: findById(requestId, ctx)
        R-->>S: current state
        
        alt Request is Updatable
            S->>S: NEW: normalizeDescription(description)
            S->>R: saveRequest(updatedData, ctx)
            R->>DB: UPDATE/INSERT
        else Request Stale/Invalid
            S-->>C: Throw NotFoundError / Conflict
        end
    end

    L->>DB: Release Advisory Lock
    S-->>C: RequestSummary

    Note over C,DB: Workflow Search/List Flow

    C->>R: findRequestsForWorkflow(options)
    R->>R: Create QueryBuilder (qb)

    rect rgb(230, 245, 255)
        Note over R,DB: CHANGED: Sequential Execution (Prevents Builder Mutation Race)
        R->>DB: qb.getRawAndEntities()
        DB-->>R: entities + raw data
        R->>DB: qb.getCount()
        DB-->>R: total count
    end

    R-->>C: { entities, count }

    Note over R,DB: NEW: Repository Cleanup
    Note right of R: Deleted: setReviewers()<br/>Deleted: authorRepo.findByRequestId()<br/>Deleted: reviewerRepo.findByRequestId()

Loading

_{Re-trigger cubic}

[kgrhartlage]

the shorter enum name lets two withLockContext(...) callbacks fit on one line, so biome re-indents ~200 lines of those blocks. It's pure whitespace, there is no other change here