feat(i18n): migrate modules/group to ResponseErrorL (Phase 2.1)

[an9xyz]

Summary

Phase 2.1 i18n migration of modules/group — the next hot module after thread (#176) and user (#188 / #197). All 386 legacy error-response sites across api.go (296), api_manager.go (46) and invite.go (44) are migrated from the legacy c.ResponseError(...) / c.ResponseErrorWithStatus(...) envelope to the localized httperr.ResponseErrorL facade, reusing the established thread/user playbook.

Organized as 9 reviewable commits: code registry → respond helpers → per-file migration → tests → business-error classification fixes.

What changed

• 39 new err.server.group.* codes (pkg/errcode/group.go) + zh-CN runtime translations + regenerated AST marker. Codes are grouped by class: validation (400), permission (401/403), not-found (404), conflict (409), rate-limit (429), and Internal=true query/store/notify (500).
• modules/group/api_i18n.go respond helpers (respondGroupRequestInvalid with field detail, respondGroupMdContentTooLarge, shared respondGroupForbidden / respondGroupNotLoggedIn). getGroupInfo now returns sentinel errors so its ~14 call sites map to 404-vs-500 via respondGroupInfoError instead of leaking a raw string at a flat 400.
• Internal failures (DB read/write, transaction/event, IM send) collapse to Internal=true codes; every such site keeps (or gains) a g.Error(..., zap.Error(err)) log so the underlying error is never put on the wire.
• Business rejections returned by the service layer (AddGroupMembers / RemoveGroupMembers) are classified to user-facing codes — external-join forbidden (403), blank-members (400), member-not-in-group (404), group-disbanded TOCTOU (404) — rather than collapsing to a 500.

Behavior change (frontend, please note)

Following the i18n design (D14), every migrated error response now returns HTTP wire status 400 during the compatibility window; the semantic status lives in error.http_status and clients should branch on error.code. Notably, four previously-403 paths (bot-ownership / external-member rejections) now return a 400 envelope with error.http_status: 403. Web/admin clients consuming these endpoints must read error.code / error.http_status rather than the HTTP status. This is consistent with the thread/user migrations already merged.

Test plan

• go build ./...
• go vet ./modules/group/
• make i18n-lint (D23 direct-error-response ratchet + unregistered-code)
• make i18n-extract-check (AST marker recall)
• go test ./modules/group ./pkg/errcode ./pkg/i18n/...
• New tests: TestGroupNoLegacyResponseError (source guard over all 3 files), TestRespondGroupHelpers (helper / sentinel / Internal-no-leak / dual-envelope), and regression tests for the business-error classifications (group-exit not-found, expired invite auth code, blank members, admin-remove non-member).

Follow-ups (non-blocking)

• Service-layer sentinel errors (errors.Is) to replace the err-string matching used for business-vs-internal classification — the same shared follow-up tracked for thread/user.
• groupSettingUpdate action errors are collapsed to store_failed; distinguishing invalid-value (400) / no-permission (403) needs the same service sentinels.

[Jerry-Xin]

In-scope PR for octo-server: the changes are relevant to modules/group error localization and follow the existing i18n migration pattern.

💬 Non-blocking

🟡 Warning: Business-error classification still depends on substring matching at modules/group/api.go, modules/group/api.go, modules/group/api.go, modules/group/api.go, and modules/group/invite.go. This is acceptable for this phase and already called out as a follow-up, but service-level sentinel errors would make these mappings much safer.

✅ Highlights

🔵 Suggestion: The helper layer in modules/group/api_i18n.go keeps field details and shared auth codes centralized, and respondGroupInfoError cleanly separates not-found from internal query failures.

✅ The PR adds useful regression coverage for the high-risk migration cases, including expired invite auth codes, blank member lists, missing groups, and management removal of non-members.

Verification performed:
go build ./... passed.
go vet ./modules/group/ passed.
make i18n-lint passed.
make i18n-extract-check passed.
go test ./modules/group ./pkg/errcode ./pkg/i18n/... could not complete because local MySQL on 127.0.0.1:3306 is unavailable; the no-DB i18n helper tests passed with go test ./modules/group -run 'TestGroupNoLegacyResponseError|TestRespondGroupHelpers'.

[lml2468]

[APPROVE]

Thorough, well-organized Phase 2.1 migration — 386 error sites across 3 files converted to ResponseErrorL with proper semantic classification.

✅ Highlights

• Clean sentinel pattern for getGroupInfo — errGroupInfoQueryFailed / errGroupInfoNotFound with respondGroupInfoError dispatcher eliminates the old (nil, nil) return path and the 14+ call sites that used to leak raw Chinese strings at a flat 400.
• Business-error classification in memberAdd / memberRemove — TOCTOU group-disbanded → 404, blank members → 400, external-join-forbidden → 403 are now properly distinguished instead of collapsing to a generic 500.
• groupMemberInviteSure empty auth_code guard — The new early return for authInfo == "" before JSON decode is a real bug fix: previously an expired auth_code fell through to util.ReadJsonByByte("") which would fail with a decode error mapped to store_failed (500). Now correctly returns auth_code_invalid. Regression test pinned.
• TestGroupNoLegacyResponseError source guard strips comments before scanning — solid ratchet against regression.
• Existing tests updated for D14 wire-status change (HTTP 400 envelope); assertions now validate error.code instead of HTTP status code.

🔵 Suggestions (non-blocking)

1. groupInvitePage ReadFile error → ErrGroupStoreFailed (api.go:4078): reading a static HTML asset from disk is not a "store" operation. ErrGroupQueryFailed would be semantically closer, though both are Internal=true so the user-facing impact is identical. Take-or-leave.

2. String-based service-layer error matching (api.go:1081-1093, api.go:2626-2632): strings.Contains(err.Error(), "禁止外部成员") / "no valid members" / "group not found or disbanded" / "none of the members are in this group" is fragile — if anyone renames these strings in service.go the classification silently degrades to store_failed. Already tracked as a shared follow-up (service-layer sentinel errors), just noting it's now load-bearing in 4 more places.

CI: Build ✅ | Lint ✅ | Vet ✅ | i18n Lint ✅ | i18n Extract Check ✅ | Test pending | CodeQL pending. check-sprint failure is sprint-tracking, not code quality.

[lml2468]

Summary

I reviewed the modules/group ResponseErrorL migration at head 482badf7171e6a250a94b2eca7203bde64605f91, including the new err.server.group.* registry, the migrated handlers, and the high-risk business-error paths from the prior user-module migration. The new group errcode statuses are generally sane, and the migrated files do not retain active legacy ResponseError / ResponseErrorf / ResponseErrorWithStatus calls. However, one handler path still collapses client-side validation and authorization failures into Internal=true 500 responses.

Blocking Issues (🔴)

• modules/group/api.go groupSettingUpdate maps errors returned by settingActionMap and groupUpdateActionMap to ErrGroupStoreFailed (500, Internal=true), but those action functions can return normal client errors. Examples from modules/group/api_setting_action.go: every per-user setting action returns "invalid value type" for malformed JSON value types (400), groupUpdateContext.checkPermissions() returns "没有权限！" for non-manager/non-owner users (403), and allow_external returns "allow_external only accepts 0 or 1" for invalid input (400). Those all currently go through api.go lines 2716-2719 or 2738-2741 and become err.server.group.store_failed. This is the same anti-pattern as PR #197: bad input / forbidden client states are reported as Internal=true 500.
• The same groupSettingUpdate group update branch also maps getGroupFnc() returning "修改的群不存在" to ErrGroupQueryFailed at api.go lines 2726-2729. That is a missing/disbanded group state and should use the existing not-found mapping instead of an Internal=true query failure.

Suggestions (🟡)

• The current string matching around service-layer business errors is acceptable for this phase, but the next cleanup should move these cases to typed sentinel errors so handlers do not need to classify by message text.

Verdict

CHANGES_REQUESTED. Please split groupSettingUpdate action errors into the existing localized client codes (request_invalid, creator_or_manager_only / forbidden, not_found) before falling back to Internal=true store/query failures.

[yujiawei]

Code Review — PR #198 (octo-server)

Scope: Phase 2.1 i18n migration of modules/group from the legacy c.ResponseError(errors.New("中文")) pattern to the localized httperr.ResponseErrorL(c, errcode.ErrGroupXxx, params, details) facade. 39 new error codes, full zh-CN + en-US coverage, helper layer in api_i18n.go.

Verdict: CHANGES_REQUESTED — two client-facing error paths now mask 400/403 conditions as internal-server (500) with the message suppressed. Both are small, localized fixes. Everything else is solid.

---

What I verified (✅)

• Build & format: go build ./... passes; gofmt clean; go vet ./modules/group/... clean.
• Registry integrity: All 39 err.server.group.* codes have matching zh-CN and en-US translations — 0 missing, 0 orphans. Every DefaultMessage matches its en-US marker character-for-character. HTTP statuses are semantically correct (409 for already_member/invite_status_invalid, 429 for daily_create_limit, 403 for authz codes, 404 for not-found, 500 Internal=true for the three internal codes). No duplicate IDs (codes.Register panics on dup).
• Cross-file string coupling (the prior cross-file-dependency lesson): the 4 handler mappings that key off service-layer strings via strings.Contains — "禁止外部成员", "no valid members", "group not found or disbanded", "none of the members are in this group" — all match hardcoded Go literals in modules/group/service.go (not i18n-translated, so they cannot drift with locale changes). Each business error reachable from AddGroupMembers/RemoveGroupMembers is correctly special-cased; genuine internal failures correctly fall through to store_failed.
• Security / authz: No permission guard was weakened, reordered, or lost. The only removed nil-check (groupExit's if groupInfo == nil) was dead code — old getGroupInfo returned a non-nil error for nil/disbanded groups, so that branch was unreachable; it now correctly maps to 404 via respondGroupInfoError. Information disclosure actually improves (unknown service errors that the old memberAdd leaked via c.ResponseError(err) are now masked behind Internal=true codes).
• Wire contract: The facade always emits transport HTTP 400 (legacy-compat) with the real semantic status in error.http_status — consistent with prior phases (user/thread). The 5 former real-403 sites (ResponseErrorWithStatus) now emit wire-400 + semantic-403-in-body; existing E2E tests were correctly updated to assert this, and no in-repo client/middleware keys off the old status.
• Tests: TestGroupNoLegacyResponseError (legacy-pattern guard) and TestRespondGroupHelpers (envelope/status/zh-CN assertions) pass. The 4 fragile TOCTOU/validation mappings have DB-backed regression tests in api_i18n_regression_test.go. (DB-backed tests couldn't run in my environment — no MySQL — so I relied on static verification for those.)

---

P1 — Blocking (client errors masked as 500)

P1-1 groupSettingUpdate collapses permission (403) and validation (400) errors into ErrGroupStoreFailed (500)

modules/group/api.go:2716-2721 and 2738-2743:

err = settingActionFnc(ctx, value)
if err != nil {
    g.Error("修改群设置信息错误", zap.Error(err))
    httperr.ResponseErrorL(c, errcode.ErrGroupStoreFailed, nil, nil) // 500, Internal, message masked
    return
}

Unlike groupUpdate (which gates upfront with ErrGroupManagerOnly), groupSettingUpdate has no upfront permission gate — authorization lives entirely inside the action funcs. Those funcs return user-facing errors, not just DB failures:

• groupUpdateContext.checkPermissions() returns errors.New("没有权限！") (api_setting_action.go:94) — a 403, invoked first by every groupUpdateActionMap entry (forbidden / invite / allow_external / view-history / pinned-message).
• Type/range validation returns errors.New("invalid value type") and errors.New("allow_external only accepts 0 or 1") (api_setting_action.go:156…373) — 400.

Legacy code passed these through c.ResponseError(err) → HTTP 400 + the real message. After this PR a non-manager toggling a group setting, or any client sending a wrong-typed value, receives error.http_status=500 with the message masked to "Failed to update group data." The permission check still blocks the mutation (no escalation), but the error class is wrong and 5xx alerting/error-budget gets polluted by normal client traffic.

Suggested fix: distinguish the action-func error like the Add/Remove handlers do — map the permission sentinel to respondGroupForbidden (or ErrGroupManagerOnly) and the validation strings to respondGroupRequestInvalid, falling back to store_failed only for genuine DB/event failures. (Cleaner long-term: have the action funcs return typed sentinels instead of bare strings.)

P1-2 avatarUpload maps missing-file (FormFile) to ErrGroupStoreFailed (500)

modules/group/api.go:426-431:

file, _, err := c.Request.FormFile("file")
if err != nil {
    g.Error("读取文件失败！", zap.Error(err))
    httperr.ResponseErrorL(c, errcode.ErrGroupStoreFailed, nil, nil) // 500, masked
    return
}

FormFile returns http.ErrMissingFile when the file multipart field is absent/empty — a pure client mistake. The sibling ParseMultipartForm branch two lines up correctly uses respondGroupRequestInvalid (400); this branch should match it. Legacy returned HTTP 400 + "读取文件失败！"; now the client gets an opaque 500 and can't tell they simply forgot to attach a file.

Suggested fix: map this branch to respondGroupRequestInvalid(c, "file") (400). The genuine storage failure (UploadFile at line ~452) correctly stays store_failed.

---

P2 — Non-blocking (suggestions)

• Coverage claim vs reality: only 2 of the 4 fragile strings.Contains mappings have handler-level regression tests (no valid members→request_invalid, none of the members→member_not_in_group). The "禁止外部成员"→external_join_forbidden (api.go:1081) and "group not found or disbanded"→not_found (api.go:1092, 2631) mappings are only tested at the service layer (service_test.go), which doesn't exercise the handler's string→errcode translation — exactly the coupling point that breaks if either side is reworded. Consider adding two DB-free/handler tests to close this.
• CreateGroup/UpdateGroupInfo (api.go:784, 879) collapse all service errors into ErrGroupStoreFailed. This is coarse (not fragile — no string dependency) and largely masked by the upstream getGroupInfo pre-check; the only residual gap is a narrow TOCTOU disband window surfacing as 500 instead of 404. Pre-existing; flagging for symmetry with Add/Remove.
• Test strength: the migrated bot-ownership tests assert error.code is present but not error.http_status == 403. The semantic status is covered by the api_i18n_test.go probe tests, so this is defense-in-depth only.
• Latent test-rot (not introduced here): the skipped TestGroupDetailGet_NonMemberDenied (api_test.go:976) still asserts the old wire-403 + Chinese string; it will fail on both assertions if un-skipped under issue #17 without updating it.

---

Overall this is a careful, well-structured migration — the registry/locale discipline and the TOCTOU regression tests are exactly right. Fixing the two P1 client-error-masking sites (both ~1-line changes) brings the error semantics in line with the rest of the migration.

[Jerry-Xin]

Summary

Phase 2.1 group module migration — solid work overall: errcode classification is accurate for the majority of the 200+ migrated sites, api_i18n.go helper layer is clean, and regression tests cover the high-risk paths. Two blocking issues remain in groupSettingUpdate where action-layer errors are swallowed into the wrong HTTP status.

🔴 Blocking

1. groupSettingUpdate: settingActionFnc / groupUpdateActionFnc errors collapsed to 500

api.go:2716-2720 and api.go:2738-2741: Both action dispatchers catch the error from the action function and unconditionally map it to ErrGroupStoreFailed (Internal=true → 500).

However, the action functions in api_setting_action.go return client-facing errors:

• errors.New("invalid value type") — should be 400 (bad request), not 500
• checkPermissions() → errors.New("没有权限！") — should be 403 (forbidden), not 500

This is the same anti-pattern fixed in #197: client errors must not be masked as internal server errors, or the client cannot distinguish "I sent bad data" from "the server is broken".

Fix: Either introduce typed/sentinel errors in the action layer and switch on them at the dispatch site, or (simpler for this phase) do substring/errors.Is dispatch similar to what memberAdd and memberRemove already do in this PR for AddGroupMembers / RemoveGroupMembers returns.

2. getGroupFnc in groupSettingUpdate: group-not-found → 500 instead of 404

api.go:2693-2695 + api.go:2726-2729: The local getGroupFnc closure returns errors.New("修改的群不存在") when group == nil, and the caller maps it to ErrGroupQueryFailed (500).

A non-existent group is a 404, not a query failure. The pattern is already established: getGroupInfo() returns errGroupInfoNotFound and callers dispatch via respondGroupInfoError. Either reuse getGroupInfo here or return errGroupInfoNotFound from getGroupFnc and handle it with respondGroupInfoError.

💬 Non-blocking

🟡 Business-error classification in memberAdd and memberRemove still relies on strings.Contains substring matching (e.g. "禁止外部成员", "no valid members", "none of the members are in this group"). Acceptable for this phase since service-level sentinel errors are a tracked follow-up, but this is fragile — a Chinese-to-English refactor or a typo fix in the service layer silently breaks the mapping.

✅ Highlights

• api_i18n.go helper layer keeps the respondGroupInfoError / respondGroupRequestInvalid / respondGroupForbidden shapes clean and DRY.
• api_i18n_regression_test.go covers exactly the high-risk edge cases (exit non-existent group, expired auth code, blank members, manager-path ghost removal). Good defensive testing.
• The TestGroupNoLegacyResponseError lint guard is a smart way to prevent regressions — any future c.ResponseError( in the migrated files will fail the build.

[lml2468]

Re-review (round 2) — 0269ed11dd94

Previous Blockers — Resolved ✅

1. groupSettingUpdate action errors collapsed into 500 — Fixed. Three sentinel errors introduced (errSettingInvalidValueType, errSettingAllowExternalRange, errGroupUpdateForbidden) and the dispatch now classifies via errors.Is: invalid type/range → 400 (ErrGroupRequestInvalid), forbidden → 403 (ErrGroupCreatorOrManagerOnly). Only genuine DB/event failures fall through to ErrGroupStoreFailed (500).

2. getGroupFnc group not found → 500 — Fixed. Returns errGroupInfoNotFound sentinel, routed through respondGroupInfoError to 404.

Bonus Fix

• avatarUpload missing file field: ErrGroupStoreFailed (500) → respondGroupRequestInvalid (400). Correct — http.ErrMissingFile is a client error.

Tests

Four regression tests pin all three fixes. Coverage is precise and matches the error classification contracts.

Verdict

APPROVED. All round-1 blockers resolved cleanly with typed sentinel errors — no string matching, no classification collapse.

[Jerry-Xin]

The PR is in scope for octo-server and the i18n migration is largely consistent with the existing thread/user localized error pattern.

💬 Non-blocking

• 🟡 Warning: modules/group/api_setting_action.go:3 is not gofmt-formatted. gofmt -l reports this changed production file; the diff is import ordering only.
• 🟡 Warning: I could not complete go test ./modules/group ./pkg/errcode ./pkg/i18n/... because group tests require MySQL at 127.0.0.1:3306, which is unavailable in this workspace. go vet ./modules/group/ and go test ./pkg/errcode ./pkg/i18n/... passed.

✅ Highlights

• The shared helpers in modules/group/api_i18n.go:27 centralize request details and sentinel mapping cleanly.
• Internal group codes in pkg/errcode/group.go:223 are marked Internal: true, which preserves the no-leak behavior through the renderer.
• The migration removes legacy ResponseError calls from the targeted group files and adds focused regression coverage for the higher-risk classification paths.

[yujiawei]

Code Review — PR #198 (octo-server)

Verdict: APPROVED

A clean, well-structured Phase 2.1 i18n migration of modules/group. All 386 legacy error-response sites across api.go, api_manager.go, and invite.go are migrated to the localized httperr.ResponseErrorL facade, with thoughtful business-error classification (400/403/404 vs Internal 500) instead of a blanket collapse. The work is consistent with the merged thread/user migrations, and the test coverage is strong. No blocking (P0/P1) issues. The suggestions below are non-blocking.

What was verified ✅

• Code-registry consistency — all 39 err.server.group.* codes are registered (pkg/errcode/group.go), translated zh-CN (pkg/i18n/locales/active.zh-CN.toml), and have en-US AST markers (tools/i18nmarkers/server/active.en-US.toml). No registered-but-unused or used-but-unregistered codes; the four "suspicious" speculative codes (too_large_to_sync, daily_create_limit, member_not_friend, file_helper_not_allowed) are all wired to reachable call sites.
• Migration completeness — the only remaining c.ResponseError token in the three migrated files is inside a comment (api.go:226). TestGroupNoLegacyResponseError enforces this as a source guard.
• No-leak contract — pkg/httperr/respond.go + pkg/i18n renderer drop both message and details for Internal=true codes, so query_failed/store_failed/notify_failed never put the underlying error on the wire. build ./modules/group/... ./pkg/errcode/... passes; DB-free tests (TestRespondGroupHelpers, TestGroupNoLegacyResponseError, i18n table tests) pass. (Remaining test failures in CI-less local runs are environmental — they require MySQL.)
• Business-error classification — every strings.Contains match string actually appears in its producer error (service.go:1272/1286/1314/1498/1508, api.go:1176); none silently fall through to 500. The errors.Is sentinel wiring in groupSettingUpdate (api_setting_action.go → api.go:2722/2750/2754) is returned by identity (no %w re-wrapping), so identity comparison holds.
• getGroupInfo sentinel pattern — getGroupInfo (api.go:3486) logs the DB error before returning errGroupInfoQueryFailed, so respondGroupInfoError's query branch correctly avoids double-logging.
• 403 → 400 wire change — confirmed the legacy c.ResponseError already returned wire 400; the only true wire-status change is the four paths that used ResponseErrorWithStatus(..., 403). These are intentional per the D14 compatibility window (semantic status now in error.http_status), and the updated tests assert both the 400 wire status and the error.code / error.http_status body fields.

Suggestions (non-blocking)

S1 — groupMemberInviteSure maps a bot-ownership denial to 500 instead of 403 (P2)

invite.go:328-337 only special-cases the "禁止外部成员" string; any other error from addMembersTxWithSpace falls into the else → ErrGroupStoreFailed (Internal 500). But addMembersTxWithSpace re-runs the bot-ownership backstop and returns ErrBotOwnershipDenied (api.go:1134-1137, whose comment explicitly names groupMemberInviteSure as a target path). So a bot-ownership authorization rejection surfaced at invite-confirm time is rendered as a 500 internal error with its message suppressed, instead of the intended 403 err.server.group.bot_ownership_denied.

• Not a security bypass: the foreign bot is still not added (the tx is rolled back); only the error envelope is wrong.
• Reachability is narrow: the create-time pre-check (invite.go:50) covers the common flow; reaching this branch requires a TOCTOU/deferred-confirmation window where ownership is re-evaluated against inviter.UID at confirm time.
• Suggested fix — mirror the handler pattern at api.go:990-993:

if errors.Is(err, ErrBotOwnershipDenied) {
    httperr.ResponseErrorL(c, errcode.ErrGroupBotOwnershipDenied, nil, nil)
    return
}

S2 — Internal response with no underlying error in the log (P2)

invite.go:155-159 (getToGroupMemberConfirmInviteDetailH5): g.Error("查询是否管理者或创建者失败！") is missing zap.Error(err). Combined with ErrGroupQueryFailed (Internal=true) suppressing the wire message, the underlying DB error is now completely invisible — the message is dropped from the response and not attached to the log. The PR's own stated contract is "every such site keeps a g.Error(..., zap.Error(err)) log". This one line violates it (it was already missing the arg pre-migration, but the migration is what makes the wire message disappear too). Add zap.Error(err).

S3 — Classification leans on duplicated free-text error literals (P2, author already tracks this)

The 400/404/403-vs-500 decision hinges on strings.Contains against raw error strings produced in service.go/api.go — including Chinese literals ("禁止外部成员"). Two independent producers of nearly-identical external-block strings (api.go:1176, service.go:1314) feed the same matcher across two call paths and must stay in lock-step; any rewording or localization silently degrades the classification to 500 with no compile-time signal. Note also that "group not found or disbanded" is load-bearing in two mechanisms (string-match in the service path, sentinel-identity via errGroupInfoNotFound in the getGroupInfo path). The PR already lists "promote service-layer errors to exported sentinels + switch to errors.Is" as a follow-up — strongly endorse it; the pattern is already used correctly in api_setting_action.go.

S4 — Wire-level regression coverage is partial (P2)

api_i18n_regression_test.go pins the blank-members→request_invalid and ghost-remove→member_not_in_group paths end-to-end, but there is no wire-level test for the TOCTOU "group not found or disbanded" → 404 paths (api.go:1094 add, api.go:2633 remove) nor the "禁止外部成员" → external_join_forbidden 403 add/invite paths. Adding these would pin all of the string-match classifications against a producer rename (a natural pairing with S3).

S5 — Minor (P2)

• groupMemberInviteAdd's req.Check() failure maps to respondGroupRequestInvalid(c, "") with no field detail, where the legacy message identified "被邀请者不能为空"; passing "uids" as the field would preserve the hint.
• TestGroupNoLegacyResponseError covers api.go/api_manager.go/invite.go but not api_setting_action.go. That file currently makes no HTTP responses (it only returns errors), so this is purely defensive — but adding it (and the .ResponseErrorWithStatusAndData( variant to the banned list) would keep the guard future-proof.
• isManager() read-path failure (api_setting_action.go:92-110) is classified to ErrGroupStoreFailed (mutation 500) rather than ErrGroupQueryFailed (read 500). Both are Internal 500, so client-invisible; purely a log/semantic nicety.

Security-sensitive notes for a human verifier

• 403 → 400 wire change on the four bot-ownership / external-member rejection paths is intentional (D14) but client-observable. Confirm web/admin clients branch on error.code / error.http_status, not the HTTP status line.
• CheckLoginRole / CheckLoginRoleIsSuperAdmin errors (including the empty-role case) now all map to the shared 403 forbidden envelope via respondGroupForbidden. These are admin routes behind auth middleware, so the simplification is acceptable, but worth a glance if any client distinguished the empty-role ("登录用户角色错误") case.
• The scan-join auth_code path (api.go:1980-2050) correctly maps all decode/validation failures to auth_code_invalid (400), user-mismatch to a dedicated 403-semantic code, and every Internal failure logs the underlying error. Authorization ordering is unchanged.