Skip to content

feat(inbounds): add a narrow endpoint for subscription sort order - #6179

Open
n0ctal wants to merge 1 commit into
MHSanaei:mainfrom
n0ctal:upstream-narrow-subsortindex
Open

feat(inbounds): add a narrow endpoint for subscription sort order#6179
n0ctal wants to merge 1 commit into
MHSanaei:mainfrom
n0ctal:upstream-narrow-subsortindex

Conversation

@n0ctal

@n0ctal n0ctal commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Reordering an inbound in subscription output requires /update/:id, which accepts and rewrites the whole inbound including its client list. Add /panel/api/inbounds/:id/subSortIndex, taking only the index — the same shape as the existing /setEnable/:id.

Why

SubSortIndex is presentation-only: internal/sub/service.go orders subscription entries by it, and nothing in the Xray config depends on it. Changing it should not require sending a client list back.

Today it does. UpdateInbound takes a *model.Inbound, so a UI or script that wants to move an inbound one place up reads the inbound, sets one integer and writes everything back. If a client was added or edited in that window, the reorder overwrites it — a lost update with no error on either side, and the larger the inbound the wider the window.

The repository already treats this as a known shape: the summary of /update/:id in endpoints.ts says "Heavy on inbounds with thousands of clients — prefer /setEnable for enable-only flips". This is the same argument for the other metadata-only field.

Scope

  • internal/web/controller/inbound.goPOST /:id/subSortIndex, binding only subSortIndex (min=1), modelled on setInboundEnable.
  • internal/web/service/inbound.goSetInboundSubSortIndex reads the stored inbound, no-ops when unchanged, updates the single column, marks a node-owned inbound dirty in the same transaction, and pushes through the existing rt.UpdateInbound. No new runtime method and no new remote endpoint.
  • frontend/src/pages/api-docs/endpoints.ts + regenerated frontend/public/openapi.json via make gen.
  • internal/web/service/inbound_subsort_test.go — asserts the index changes and the settings JSON is byte-identical afterwards.

Deliberately not included: a narrow remote push. The node still receives a full UpdateInbound, so this PR fixes the lost-update problem without widening the node contract. Narrowing the remote call is a separate change if you want it.

Validation

  • go test ./internal/web/service/ -run SetInboundSubSortIndex -count=1 green.
  • go test ./internal/web/ -run RouteRegistryContract -count=1 green, so the registry entry matches the route.
  • make gen run; go build ./..., go vet, gofmt -l clean.

Risk

Low and additive. No existing route, service method or model field changes behaviour; /update/:id keeps working exactly as before.

@n0ctal

n0ctal commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up on the two remaining red checks, with what I could verify from here.

golangci — the tree is clean; this looks like a stale analysis cache, not a real finding.

I checked out upstream/main unmodified, installed the same linter CI resolves (golangci-lint v2.12.2, version: latest picks it in both runs) and ran it:

  • golangci-lint run over the whole module → 0 issues
  • golangci-lint run ./internal/web/service/..., the package CI reports SA5011 in → 0 issues

The same job is green on main at the very commit these PRs branch from, and the reported files are ones no PR here touches. The findings themselves are the t.Fatal-then-dereference shape, which staticcheck does not flag on a cold run. golangci-lint-action restores an analysis cache (Cache hit for: golangci-lint.cache-Linux-2952-…), so busting that cache is the thing I would try first — I cannot do it from a fork.

frontend — that one is real and fixable: npm audit --omit=dev --audit-level=high trips on GHSA-rgw5-rvv9-x895 via swagger-ui-react → … → brace-expansion@5.0.8. It fails on main too. Opened #6180 with a scoped override to the patched 5.0.9; merging it turns this job green for every open PR, including this one.

Nothing in either check comes from this branch.

Changing an inbound's position in subscription output currently goes through
/update/:id, which takes a whole inbound: the caller has to send settings and
the entire client list back, and whatever it read before the edit is what gets
written. Two people reordering and editing clients in the same inbound race on
one blob, and the reorder wins by overwriting.

Mirror the existing /setEnable/:id shape. The handler takes only the index and
the service reads the stored inbound, so nothing in the request can reach the
settings JSON. Node-owned inbounds are marked dirty in the same transaction and
pushed through the existing runtime update.
@n0ctal
n0ctal force-pushed the upstream-narrow-subsortindex branch from 5157d8e to ba1fd60 Compare August 8, 2026 02:11
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