Skip to content

Conversation

@kastov
Copy link
Collaborator

@kastov kastov commented Dec 28, 2025

No description provided.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 28, 2025

Greptile Summary

This release adds bulk user management endpoints (/add-users and /remove-users) to improve performance when handling multiple users simultaneously. The implementation includes new command schemas, DTOs, controller endpoints, and service methods with performance logging using enhanced-ms.

Key changes:

  • Added bulk endpoints for adding and removing multiple users in single operations
  • Updated contract library version from 0.6.1 to 2.5.0 to match main package
  • Removed shadowsocks2022 support from single user add command
  • Updated Xray to v25.12.8 and dependency versions (NestJS 11.1.10, systeminformation 5.28.3)
  • Modified deploy workflow to update npm before publishing and removed provenance flag

Critical Issue Found:
The addUsers method (lines 201-301) doesn't validate operation results. Unlike addUser which collects responses and checks for failures, addUsers always returns success even if all user additions fail. This creates a silent failure scenario where the API reports success despite actual failures.

Confidence Score: 2/5

  • Critical bug in addUsers method will cause silent failures in production
  • The missing error handling in addUsers is a critical logical error that will cause the API to report success when operations actually fail, leading to data inconsistency. The bulk operations follow good patterns otherwise, but this bug must be fixed before merge.
  • Pay close attention to src/modules/handler/handler.service.ts - the addUsers method requires immediate fix for response validation

Important Files Changed

Filename Overview
.github/workflows/deploy-lib.yml Updated npm to latest and removed provenance flag from publish command
libs/contract/commands/handler/add-users.command.ts New command schema for bulk user addition with affectedInboundTags and users array
libs/contract/commands/handler/remove-users.command.ts New command schema for bulk user removal with users array containing userId and hashUuid
src/modules/handler/handler.controller.ts Added addUsers and removeUsers endpoints following existing controller patterns
src/modules/handler/handler.service.ts Added bulk operations with performance logging, but addUsers missing error handling validation logic

Sequence Diagram

sequenceDiagram
    participant Client
    participant Controller as HandlerController
    participant Service as HandlerService
    participant Internal as InternalService
    participant XTLS as XtlsApi

    rect rgb(240, 248, 255)
        note right of Client: Bulk Add Users Flow
        Client->>Controller: POST /handler/add-users
        Controller->>Service: addUsers(data)
        
        loop For each affectedInboundTag
            Service->>Internal: addXtlsConfigInbound(tag)
        end
        
        loop For each user
            loop For each existing inbound
                Service->>XTLS: removeUser(tag, userId)
                Service->>Internal: removeUserFromInbound(tag, hashUuid)
            end
            
            loop For each inboundData item
                alt trojan type
                    Service->>XTLS: addTrojanUser(...)
                    XTLS-->>Service: response
                    Service->>Internal: addUserToInbound(tag, vlessUuid)
                else vless type
                    Service->>XTLS: addVlessUser(...)
                    XTLS-->>Service: response
                    Service->>Internal: addUserToInbound(tag, vlessUuid)
                else shadowsocks type
                    Service->>XTLS: addShadowsocksUser(...)
                    XTLS-->>Service: response
                    Service->>Internal: addUserToInbound(tag, vlessUuid)
                end
            end
        end
        
        Service-->>Controller: AddUserResponseModel
        Controller-->>Client: response
    end
    
    rect rgb(255, 248, 240)
        note right of Client: Bulk Remove Users Flow
        Client->>Controller: POST /handler/remove-users
        Controller->>Service: removeUsers(data)
        Service->>Internal: getXtlsConfigInbounds()
        Internal-->>Service: inboundTags
        
        loop For each user
            loop For each inbound tag
                Service->>XTLS: removeUser(tag, userId)
                XTLS-->>Service: response
                Service->>Internal: removeUserFromInbound(tag, hashUuid)
            end
        end
        
        Service-->>Controller: RemoveUserResponseModel
        Controller-->>Client: response
    end
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

18 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

@kastov kastov merged commit cf846fe into main Dec 28, 2025
8 checks passed
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.

2 participants