diff --git a/.github/workflows/auto-add-to-project.yml b/.github/workflows/auto-add-to-project.yml index 73687b3a..d3f6bc9a 100644 --- a/.github/workflows/auto-add-to-project.yml +++ b/.github/workflows/auto-add-to-project.yml @@ -5,7 +5,7 @@ on: issues: types: [opened] pull_request_target: - types: [opened] + types: [opened, edited] permissions: {} diff --git a/.github/workflows/check-sprint.yml b/.github/workflows/check-sprint.yml index c1e9e383..6a36a036 100644 --- a/.github/workflows/check-sprint.yml +++ b/.github/workflows/check-sprint.yml @@ -33,7 +33,7 @@ name: Check Sprint on: pull_request_target: - types: [opened, synchronize, reopened] + types: [opened, synchronize, reopened, edited] branches: [main] permissions: {} diff --git a/.github/workflows/octo-pr-feed.yml b/.github/workflows/octo-pr-feed.yml deleted file mode 100644 index 29a31ef0..00000000 --- a/.github/workflows/octo-pr-feed.yml +++ /dev/null @@ -1,27 +0,0 @@ -name: Octo PR Feed - -on: - pull_request_target: - types: [opened, closed, reopened, ready_for_review, review_requested] - -permissions: {} - -jobs: - notify: - if: ${{ !github.event.pull_request.draft }} - uses: Mininglamp-OSS/.github/.github/workflows/octo-pr-feed.yml@main - with: - repo_name: ${{ github.event.repository.name }} - pr_number: ${{ github.event.pull_request.number }} - pr_title: ${{ github.event.pull_request.title }} - pr_url: ${{ github.event.pull_request.html_url }} - pr_author: ${{ github.event.pull_request.user.login }} - pr_merged: ${{ github.event.pull_request.merged }} - pr_additions: ${{ github.event.pull_request.additions }} - pr_deletions: ${{ github.event.pull_request.deletions }} - pr_changed_files: ${{ github.event.pull_request.changed_files }} - event_action: ${{ github.event.action }} - feed_group_id: "1c303c142e9840f2a9b46c10b0972e8d" - project_group_id: "9ea115c7462b4b45b8c85d07d07e0dde" - secrets: - OCTO_BOT_TOKEN: ${{ secrets.OCTO_BOT_TOKEN }} diff --git a/.github/workflows/octo-pr-result-notify.yml b/.github/workflows/octo-pr-result-notify.yml new file mode 100644 index 00000000..9c2ee1ef --- /dev/null +++ b/.github/workflows/octo-pr-result-notify.yml @@ -0,0 +1,43 @@ +name: Octo PR Result Notify + +on: + pull_request_target: + types: [closed, reopened] + pull_request_review: + types: [submitted] + +permissions: {} + +jobs: + pr-result: + if: github.event_name == 'pull_request_target' + uses: Mininglamp-OSS/.github/.github/workflows/octo-pr-result-notify.yml@main + with: + repo_name: ${{ github.event.repository.name }} + pr_number: ${{ github.event.pull_request.number }} + pr_title: ${{ github.event.pull_request.title }} + pr_url: ${{ github.event.pull_request.html_url }} + pr_author: ${{ github.event.pull_request.user.login }} + event_kind: ${{ github.event.action == 'closed' && github.event.pull_request.merged == true && 'pr_merged' || github.event.action == 'closed' && 'pr_closed' || 'pr_reopened' }} + pr_additions: ${{ github.event.pull_request.additions }} + pr_deletions: ${{ github.event.pull_request.deletions }} + pr_changed_files: ${{ github.event.pull_request.changed_files }} + secrets: + OCTO_BOT_TOKEN: ${{ secrets.OCTO_BOT_TOKEN }} + + review-result: + if: >- + github.event_name == 'pull_request_review' && + (github.event.review.state == 'approved' || github.event.review.state == 'changes_requested') && + github.event.pull_request.head.repo.full_name == github.repository + uses: Mininglamp-OSS/.github/.github/workflows/octo-pr-result-notify.yml@main + with: + repo_name: ${{ github.event.repository.name }} + pr_number: ${{ github.event.pull_request.number }} + pr_title: ${{ github.event.pull_request.title }} + pr_url: ${{ github.event.pull_request.html_url }} + pr_author: ${{ github.event.pull_request.user.login }} + event_kind: ${{ github.event.review.state == 'approved' && 'review_approved' || 'review_changes_requested' }} + reviewer: ${{ github.event.review.user.login }} + secrets: + OCTO_BOT_TOKEN: ${{ secrets.OCTO_BOT_TOKEN }} diff --git a/.github/workflows/octo-pr-review-feed.yml b/.github/workflows/octo-pr-review-feed.yml new file mode 100644 index 00000000..d3384d40 --- /dev/null +++ b/.github/workflows/octo-pr-review-feed.yml @@ -0,0 +1,22 @@ +name: Octo PR Review Feed + +on: + pull_request_target: + types: [opened, ready_for_review, review_requested, synchronize] + +permissions: {} + +jobs: + notify: + if: ${{ !github.event.pull_request.draft }} + uses: Mininglamp-OSS/.github/.github/workflows/octo-pr-review-feed.yml@main + with: + repo_name: ${{ github.event.repository.name }} + pr_number: ${{ github.event.pull_request.number }} + pr_title: ${{ github.event.pull_request.title }} + pr_url: ${{ github.event.pull_request.html_url }} + pr_author: ${{ github.event.pull_request.user.login }} + event_action: ${{ github.event.action }} + project_group_id: '9ea115c7462b4b45b8c85d07d07e0dde' + secrets: + OCTO_BOT_TOKEN: ${{ secrets.OCTO_BOT_TOKEN }} diff --git a/Makefile b/Makefile index 4e4734ff..b2eefeaa 100644 --- a/Makefile +++ b/Makefile @@ -23,4 +23,48 @@ stop-dev: echo " https://github.com/Mininglamp-OSS/octo-deployment"; \ exit 1 env-test: - docker-compose -f ./testenv/docker-compose.yaml up -d \ No newline at end of file + docker-compose -f ./testenv/docker-compose.yaml up -d + +# ---- i18n message marker pipeline (TODOS §0.8 / D18) ---------------------- +# +# i18n-extract : regenerate tools/i18nmarkers/{shared,server}/active.en-US.toml +# from codes.Register / errcode.register AST call sites. +# i18n-extract-check : CI guard — fails (exit 3) when on-disk markers diverge +# from what extraction would emit. Wired up alongside +# the rest of the 0.10 lint suite. +# i18n-merge : optional convenience target that feeds BOTH generated marker +# files (shared + server) into the upstream `goi18n` CLI to +# produce translate..toml stubs for new keys. Requires: +# go install github.com/nicksnyder/go-i18n/v2/goi18n@v2.6.1 +# First-run side effect: goi18n rewrites active..toml +# into its canonical format (entries sorted by ID, content +# hashes added, top-of-file comments stripped). This is +# expected once translators adopt the goi18n workflow; hashes +# are how goi18n detects source drift requiring re-translation. + +.PHONY: i18n-extract i18n-extract-check i18n-merge + +i18n-extract: + go run ./pkg/i18n/cmd/octo-i18n-extract + +i18n-extract-check: + go run ./pkg/i18n/cmd/octo-i18n-extract -check + +i18n-merge: i18n-extract + @command -v goi18n >/dev/null 2>&1 || { \ + echo "goi18n not on PATH — install with:"; \ + echo " go install github.com/nicksnyder/go-i18n/v2/goi18n@v2.6.1"; \ + exit 1; \ + } + # CRITICAL: feed BOTH shared and server marker files to goi18n as source + # inputs. With `-sourceLanguage en-US`, goi18n treats the union of source + # files as the canonical message set and rewrites any existing translation + # file (active.zh-CN.toml) to that set — entries not present in the source + # inputs are removed. Omitting the server marker file destructively wipes + # every err.server.* zh-CN translation from active.zh-CN.toml (verified + # against goi18n@v2.6.1 by PR #186 reviewers; preserving server + # translations is the contract this target must hold). + goi18n merge -sourceLanguage en-US -outdir pkg/i18n/locales \ + tools/i18nmarkers/shared/active.en-US.toml \ + tools/i18nmarkers/server/active.en-US.toml \ + pkg/i18n/locales/active.zh-CN.toml diff --git a/modules/common/api.go b/modules/common/api.go index 4ffc1f79..ff36c76d 100644 --- a/modules/common/api.go +++ b/modules/common/api.go @@ -395,9 +395,10 @@ func (cn *Common) appConfig(c *wkhttp.Context) { } if versionI64 != 0 && int(versionI64) >= appConfigM.Version { c.JSON(http.StatusOK, &appConfigResp{ - Version: appConfigM.Version, - SystemBotUIDs: spacepkg.SystemBotList(), - LocalLoginOff: boolToFlag(cn.systemSettings.LocalLoginOff()), + Version: appConfigM.Version, + SystemBotUIDs: spacepkg.SystemBotList(), + LocalLoginOff: boolToFlag(cn.systemSettings.LocalLoginOff()), + DisableUserCreateSpace: boolToFlag(cn.systemSettings.SpaceDisableUserCreate()), }) return } @@ -433,8 +434,9 @@ func (cn *Common) appConfig(c *wkhttp.Context) { OIDCResetPasswordURL: oidcResetPasswordURL(), OIDCProviders: oidcProviders(), // YUJ-219-A / GH#1283：单一真源下发系统 Bot UID 列表，替代三端硬编码。 - SystemBotUIDs: spacepkg.SystemBotList(), - LocalLoginOff: boolToFlag(cn.systemSettings.LocalLoginOff()), + SystemBotUIDs: spacepkg.SystemBotList(), + LocalLoginOff: boolToFlag(cn.systemSettings.LocalLoginOff()), + DisableUserCreateSpace: boolToFlag(cn.systemSettings.SpaceDisableUserCreate()), }) } @@ -735,6 +737,16 @@ type appConfigResp struct { // 与 app_config.version 解耦：即使客户端命中 version 短路分支，也必须能拿到 // 最新值，否则 admin 切换开关后老客户端会被本地缓存住。和 SystemBotUIDs 同理。 LocalLoginOff int `json:"local_login_off"` + + // DisableUserCreateSpace 控制客户端是否隐藏「创建空间」入口。 + // 来源 system_setting space.disable_user_create,回退到 env + // DM_SPACE_DISABLE_USER_CREATE,默认 0(允许创建)。 + // + // 与 app_config.version 解耦的原因同 LocalLoginOff：admin 在管理台 toggle + // 后老客户端命中 version 短路分支仍必须看到最新值，否则被本地缓存住失去 + // 实时性。后端 POST /v1/space/create 也走同一个 getter 校验,客户端隐藏 + // 与服务端拒绝由单一真源驱动,不存在前后端漂移。 + DisableUserCreateSpace int `json:"disable_user_create_space"` } type oidcProviderResp struct { diff --git a/modules/common/api_test.go b/modules/common/api_test.go index d0b39cfa..6ed86ea1 100644 --- a/modules/common/api_test.go +++ b/modules/common/api_test.go @@ -119,6 +119,71 @@ func TestGetAppConfig_SystemBotUIDsOnVersionShortCircuit(t *testing.T) { assert.Contains(t, body, `"fileHelper"`) } +// appconfig 必须下发 disable_user_create_space：默认 0（缺 system_setting 行且 +// env 未设置）。客户端据此显示/隐藏「创建空间」入口；缺省必须保持开放。 +func TestGetAppConfig_DisableUserCreateSpace_DefaultsZero(t *testing.T) { + t.Setenv("DM_SPACE_DISABLE_USER_CREATE", "") + s, ctx := testutil.NewTestServer() + f := New(ctx) + err := testutil.CleanAllTables(ctx) + assert.NoError(t, err) + err = f.appConfigDB.insert(&appConfigModel{}) + assert.NoError(t, err) + w := httptest.NewRecorder() + req, _ := http.NewRequest("GET", "/v1/common/appconfig", nil) + req.Header.Set("token", testutil.Token) + s.GetRoute().ServeHTTP(w, req) + assert.Equal(t, http.StatusOK, w.Code) + assert.Contains(t, w.Body.String(), `"disable_user_create_space":0`) +} + +// DB 写入 disable_user_create=1 时 appconfig 必须下发 1，admin 在管理台切换 +// 后客户端下次拉配置即可看到入口隐藏 —— 系统级 KV + Reload 路径的实时性保证。 +func TestGetAppConfig_DisableUserCreateSpace_DBOverride(t *testing.T) { + t.Setenv("DM_SPACE_DISABLE_USER_CREATE", "") + s, ctx := testutil.NewTestServer() + f := New(ctx) + err := testutil.CleanAllTables(ctx) + assert.NoError(t, err) + err = f.appConfigDB.insert(&appConfigModel{}) + assert.NoError(t, err) + + settings := EnsureSystemSettings(ctx) + assert.NoError(t, settings.db.upsert("space", "disable_user_create", "1", settingTypeBool, "")) + assert.NoError(t, settings.Reload()) + + w := httptest.NewRecorder() + req, _ := http.NewRequest("GET", "/v1/common/appconfig", nil) + req.Header.Set("token", testutil.Token) + s.GetRoute().ServeHTTP(w, req) + assert.Equal(t, http.StatusOK, w.Code) + assert.Contains(t, w.Body.String(), `"disable_user_create_space":1`) +} + +// version 短路分支同样要下发 disable_user_create_space：老客户端命中版本 +// 短路也必须看到当前开关，否则被缓存住失去"实时调整"的能力（与 LocalLoginOff +// 同样的"system_setting 与 app_config.version 解耦"约束）。 +func TestGetAppConfig_DisableUserCreateSpace_OnVersionShortCircuit(t *testing.T) { + t.Setenv("DM_SPACE_DISABLE_USER_CREATE", "") + s, ctx := testutil.NewTestServer() + f := New(ctx) + err := testutil.CleanAllTables(ctx) + assert.NoError(t, err) + err = f.appConfigDB.insert(&appConfigModel{}) + assert.NoError(t, err) + + settings := EnsureSystemSettings(ctx) + assert.NoError(t, settings.db.upsert("space", "disable_user_create", "1", settingTypeBool, "")) + assert.NoError(t, settings.Reload()) + + w := httptest.NewRecorder() + req, _ := http.NewRequest("GET", "/v1/common/appconfig?version=99999999", nil) + req.Header.Set("token", testutil.Token) + s.GetRoute().ServeHTTP(w, req) + assert.Equal(t, http.StatusOK, w.Code) + assert.Contains(t, w.Body.String(), `"disable_user_create_space":1`) +} + // appconfig 必须下发 local_login_off：默认 0（缺 system_setting 行）。 func TestGetAppConfig_LocalLoginOff_DefaultsZero(t *testing.T) { s, ctx := testutil.NewTestServer() diff --git a/modules/common/system_setting_schema.go b/modules/common/system_setting_schema.go index dce5d298..d257ea26 100644 --- a/modules/common/system_setting_schema.go +++ b/modules/common/system_setting_schema.go @@ -50,6 +50,12 @@ var systemSettingSchema = []settingDef{ {Category: "login", Key: "local_off", Type: settingTypeBool, Description: "是否关闭本地账号登录入口", Effective: func(s *SystemSettings) string { return boolToCanonical(s.LocalLoginOff()) }}, + // Space user-facing creation toggle — admin 关闭后客户端隐藏创建入口, + // 后端 POST /v1/space/create 直接 403。env DM_SPACE_DISABLE_USER_CREATE + // 仍作 fallback,DB 行为单一真源。 + {Category: "space", Key: "disable_user_create", Type: settingTypeBool, Description: "是否关闭普通用户创建空间入口", + Effective: func(s *SystemSettings) string { return boolToCanonical(s.SpaceDisableUserCreate()) }}, + // Email server config — formerly yaml-only (Support.* in config.go). {Category: "support", Key: "email", Type: settingTypeString, Description: "技术支持邮箱（发件人）", Effective: func(s *SystemSettings) string { return s.SupportEmail() }}, diff --git a/modules/common/system_settings.go b/modules/common/system_settings.go index 8ea00ad9..670d1826 100644 --- a/modules/common/system_settings.go +++ b/modules/common/system_settings.go @@ -6,6 +6,7 @@ import ( "os" "regexp" "strconv" + "strings" "sync" "sync/atomic" "time" @@ -427,6 +428,60 @@ func (s *SystemSettings) RawLocalLoginOffFromSnapshot() bool { return s.getBool("login", "local_off", false) } +// envSpaceDisableUserCreate 与 modules/space/api.go:envDisableUserCreateSpace +// 保持同名,镜像在 common 包以避免反向依赖 (space 已 import common)。新增/修改 +// env 解析规则时两处同步,语义就是: 1/true/yes/on (任意大小写,允许前后空格) +// 视为 ON，其余皆 OFF。 +const envSpaceDisableUserCreate = "DM_SPACE_DISABLE_USER_CREATE" + +// SpaceDisableUserCreate reports whether the user-facing「创建空间」入口应被 +// 关闭。完整 fallback 链(按优先级): +// +// 1. DB 行存在且 value 非空 → 走 getBool 解析(1/true/TRUE → true; +// 0/false/FALSE → false; 未知字面量 → false)。**不再回退到 env** —— 与 +// 其他 bool 设置一致,未知字面量等同 "admin 不希望关闭"。 +// 2. DB 行不存在,或 value="" → env DM_SPACE_DISABLE_USER_CREATE +// 3. 都缺失 → false (保持开放) +// +// 注：manager 写接口对 bool 值已做规范化(只接受 0/1/true/false 及大小写 +// 变体),正常路径不会出现未知字面量;此规则覆盖的是有人绕过 API 直接改 DB +// 的边缘场景。 +// +// DB 是单一真源：admin 在管理台显式 toggle 立刻生效（Reload 内存快照）， +// 多实例 60s 内收敛。env 仅作历史部署兼容入口；新部署应直接走 system_setting。 +// +// 与 modules/space/api.go:IsUserCreateDisabled 保持等价语义 —— 后者仍是 +// env-only 的低层解析器,留给没有 ctx 的调用方与 yaml 模式;实际请求路径走本 +// 方法（modules/space/api.go:createSpace）。 +// +// 实现细节：DB 路径委托给 getBool 以与其他 bool 设置共享解析规则,避免双写 +// 字面量集合(reviewer H1)。"DB 行是否存在"由独立 lookup 决定,从而区分 +// "DB 缺行 → env" 与 "DB 值=0 → 强制 false 压制 env" 两个语义。 +func (s *SystemSettings) SpaceDisableUserCreate() bool { + if _, ok := s.lookup("space", "disable_user_create"); ok { + // 走与所有其他 bool 设置一致的字面量解析;未知字面量会落到 fallback=false, + // 与 "DB 显式写了 0" 语义一致 —— 都视为 admin 不希望关闭。 + return s.getBool("space", "disable_user_create", false) + } + return parseSpaceDisableUserCreateEnv(os.Getenv(envSpaceDisableUserCreate)) +} + +// parseSpaceDisableUserCreateEnv 与 modules/space/api.go:IsUserCreateDisabled +// 的解析逻辑保持一致(1/true/yes/on,大小写不敏感,允许前后空格)。两处镜像而 +// 非提到 leaf package,理由同 LocalLoginOff/OIDC: 一个 helper 不值得为它引 +// 入一层新包。修改任何一处时两边同步,否则同一开关在两个出口语义会漂移。 +func parseSpaceDisableUserCreateEnv(v string) bool { + v = strings.TrimSpace(v) + if v == "" { + return false + } + switch strings.ToLower(v) { + case "1", "true", "yes", "on": + return true + } + return false +} + // SupportEmail returns the From address used by the SMTP sender. func (s *SystemSettings) SupportEmail() string { return s.getString("support", "email", s.ctx.GetConfig().Support.Email) diff --git a/modules/common/system_settings_test.go b/modules/common/system_settings_test.go index 5cb807b5..acaee347 100644 --- a/modules/common/system_settings_test.go +++ b/modules/common/system_settings_test.go @@ -223,6 +223,74 @@ func TestSystemSettings_LocalLoginOff_TrueWhenGiteeConfigured(t *testing.T) { assert.True(t, s.LocalLoginOff(), "Gitee OAuth 配置齐备 → 守卫生效") } +// SpaceDisableUserCreate 的 fallback 链：DB → env DM_SPACE_DISABLE_USER_CREATE → false。 +// 默认无 DB 行且无 env 时,返回 false（保持历史行为：用户侧可创建空间）。 +func TestSystemSettings_SpaceDisableUserCreate_DefaultsFalse(t *testing.T) { + s := newTestSystemSettings(t, nil) + t.Setenv("DM_SPACE_DISABLE_USER_CREATE", "") + assert.False(t, s.SpaceDisableUserCreate(), + "DB 未配置且 env 未设置时必须保持开放（false）") +} + +// DB 值优先于 env：admin 在管理台显式关闭时,即便 env 未设置也立即生效。 +func TestSystemSettings_SpaceDisableUserCreate_DBTrueWins(t *testing.T) { + s := newTestSystemSettings(t, nil) + t.Setenv("DM_SPACE_DISABLE_USER_CREATE", "") + require.NoError(t, s.db.upsert("space", "disable_user_create", "1", settingTypeBool, "")) + require.NoError(t, s.Reload()) + assert.True(t, s.SpaceDisableUserCreate(), + "DB=1 必须关闭用户侧创建入口") +} + +// DB 行存在但值为 0 时必须明确返回 false,即使 env 设置为 true 也不再"漏出去"。 +// 这一条用例固化"DB 是单一真源"的语义：admin 在管理台 toggle 回 0 必须能压住 +// 历史 env 配置；否则运维改了配置却看不到效果。 +func TestSystemSettings_SpaceDisableUserCreate_DBFalseOverridesEnv(t *testing.T) { + s := newTestSystemSettings(t, nil) + t.Setenv("DM_SPACE_DISABLE_USER_CREATE", "true") + require.NoError(t, s.db.upsert("space", "disable_user_create", "0", settingTypeBool, "")) + require.NoError(t, s.Reload()) + assert.False(t, s.SpaceDisableUserCreate(), + "DB=0 必须覆盖 env=true（DB 是单一真源）") +} + +// DB 行存在但 value 为空字符串时, lookup 视为"未配置"(与其它所有设置一致), +// 落回 env fallback —— 不是把 disable_user_create 强制 false。这条用例锁定 +// "DB row 空值 == 未配置" 的语义,与其他 bool 设置(register/login/support) +// 共享同一回退规则,避免后续维护者误以为空值压制 env。 +func TestSystemSettings_SpaceDisableUserCreate_DBEmptyValueFallsBackToEnv(t *testing.T) { + s := newTestSystemSettings(t, nil) + t.Setenv("DM_SPACE_DISABLE_USER_CREATE", "true") + require.NoError(t, s.db.upsert("space", "disable_user_create", "", settingTypeBool, "")) + require.NoError(t, s.Reload()) + assert.True(t, s.SpaceDisableUserCreate(), + "DB 值=\"\" 视为未配置, env=true 必须生效") +} + +// 仅设置了 env 而无 DB 行时,getter 必须回退到 env 解析结果,保持对历史部署的 +// 兼容（envDisableUserCreateSpace 是这个开关的原始入口）。 +func TestSystemSettings_SpaceDisableUserCreate_EnvFallback(t *testing.T) { + s := newTestSystemSettings(t, nil) + for _, spelling := range []string{"1", "true", "TRUE", "yes", "on", " true "} { + t.Run(spelling, func(t *testing.T) { + t.Setenv("DM_SPACE_DISABLE_USER_CREATE", spelling) + assert.True(t, s.SpaceDisableUserCreate(), + "env=%q 必须被识别为开启", spelling) + }) + } +} + +func TestSystemSettings_SpaceDisableUserCreate_EnvNegativeSpellings(t *testing.T) { + s := newTestSystemSettings(t, nil) + for _, spelling := range []string{"0", "false", "FALSE", "no", "random"} { + t.Run(spelling, func(t *testing.T) { + t.Setenv("DM_SPACE_DISABLE_USER_CREATE", spelling) + assert.False(t, s.SpaceDisableUserCreate(), + "env=%q 不应被识别为开启", spelling) + }) + } +} + func TestSystemSettings_StringFallsBackOnEmpty(t *testing.T) { s := newTestSystemSettings(t, nil) s.ctx.GetConfig().Support.EmailSmtp = "smtp.yaml.example:465" diff --git a/modules/space/api.go b/modules/space/api.go index 5ce91d2e..0ab03071 100644 --- a/modules/space/api.go +++ b/modules/space/api.go @@ -19,6 +19,7 @@ import ( "github.com/Mininglamp-OSS/octo-lib/pkg/wkevent" "github.com/Mininglamp-OSS/octo-lib/pkg/wkhttp" "github.com/Mininglamp-OSS/octo-server/modules/base/event" + commonmod "github.com/Mininglamp-OSS/octo-server/modules/common" octoredis "github.com/Mininglamp-OSS/octo-server/pkg/redis" spacepkg "github.com/Mininglamp-OSS/octo-server/pkg/space" rd "github.com/go-redis/redis" @@ -121,11 +122,22 @@ func (s *Space) Route(r *wkhttp.WKHttp) { } } -// envDisableUserCreateSpace 全局开关：运维通过环境变量 DM_SPACE_DISABLE_USER_CREATE=true -// 关闭用户侧创建空间入口（POST /v1/space/create）。管理端代建接口不受此开关约束。 +// envDisableUserCreateSpace 全局开关的历史 env 入口：运维通过环境变量 +// DM_SPACE_DISABLE_USER_CREATE=true 关闭用户侧创建空间入口 +// （POST /v1/space/create）。管理端代建接口不受此开关约束。 +// +// 单一真源已迁移到 system_setting 表的 (space, disable_user_create) 行； +// 本 env 仍作 fallback 供没有 DB override 的老部署使用,但 admin 在管理台写入 +// DB 后,DB 值优先。详见 modules/common/system_settings.go:SpaceDisableUserCreate。 const envDisableUserCreateSpace = "DM_SPACE_DISABLE_USER_CREATE" -// IsUserCreateDisabled 是否已通过环境变量关闭用户侧创建空间。 +// IsUserCreateDisabled 是否已通过环境变量关闭用户侧创建空间。env-only 的低层 +// 解析器,语义与 modules/common/system_settings.go:parseSpaceDisableUserCreateEnv +// 保持一致 —— 后者是 system_setting 查询的 env fallback。修改此函数的解析 +// 规则时必须同步那一处,否则同一开关会在两个出口产生漂移。 +// +// 调用方应优先走 (*Space).isUserCreateDisabled 以获得 DB → env 完整链路；本 +// 函数保留是为了向前兼容已有 callers / 测试,以及作为最简 env-only 探测点。 func IsUserCreateDisabled() bool { v := strings.TrimSpace(os.Getenv(envDisableUserCreateSpace)) if v == "" { @@ -138,6 +150,13 @@ func IsUserCreateDisabled() bool { return false } +// isUserCreateDisabled 走 system_setting DB → env 完整 fallback 链, 是 +// createSpace handler 的判断入口。SystemSettings snapshot 由 ticker 自动刷新, +// admin 写 DB → Reload 后 60s 内多实例收敛, 本实例立即生效。 +func (s *Space) isUserCreateDisabled() bool { + return commonmod.EnsureSystemSettings(s.ctx).SpaceDisableUserCreate() +} + // createSpaceParams 创建空间的核心参数。Creator 为目标空间 owner， // 管理端代建时设为被代建用户，业务端为登录用户。 type createSpaceParams struct { @@ -158,7 +177,7 @@ type createSpaceResult struct { // createSpace 创建空间（用户侧入口） func (s *Space) createSpace(c *wkhttp.Context) { - if IsUserCreateDisabled() { + if s.isUserCreateDisabled() { c.ResponseErrorWithStatus(errors.New("管理员已关闭空间创建功能"), http.StatusForbidden) return } diff --git a/modules/space/api_test.go b/modules/space/api_test.go index 20aca515..4fcd2af6 100644 --- a/modules/space/api_test.go +++ b/modules/space/api_test.go @@ -16,8 +16,10 @@ import ( "github.com/Mininglamp-OSS/octo-lib/common" "github.com/Mininglamp-OSS/octo-lib/config" "github.com/Mininglamp-OSS/octo-lib/pkg/util" + "github.com/Mininglamp-OSS/octo-lib/pkg/wkhttp" "github.com/Mininglamp-OSS/octo-lib/server" "github.com/Mininglamp-OSS/octo-lib/testutil" + modulescommon "github.com/Mininglamp-OSS/octo-server/modules/common" "github.com/stretchr/testify/assert" ) @@ -1535,6 +1537,54 @@ func TestCreateSpace_AllowedByDefault(t *testing.T) { assert.Equal(t, 2, mem.Role) } +// system_setting 写入 space.disable_user_create=1 后, createSpace 必须返回 403, +// 不依赖任何环境变量。这是「admin 在管理台实时关闭用户侧创建」核心路径的 +// 守卫用例 —— 离开 env 之后,DB 行 + Reload 立刻让本实例生效,多实例由 60s +// 自动 reload 收敛(参见 SystemSettings.StartAutoReload)。 +func TestCreateSpace_DisabledBySystemSetting(t *testing.T) { + s, _, err := setup(t) + assert.NoError(t, err) + // 显式清空 env, 证明开关纯由 DB 驱动 + t.Setenv(envDisableUserCreateSpace, "") + + // 直接 DB 写入 + Reload, 模拟 manager API 的写路径但避开 admin token 与 + // 路由准备 — 这条用例的关注点是 "DB → SystemSettings → createSpace 拒绝" + // 这条链路, 不是 manager API 本身(后者在 common 包已有单测覆盖)。 + _, err = testCtx.DB().InsertInto("system_setting"). + Pair("category", "space"). + Pair("key_name", "disable_user_create"). + Pair("value", "1"). + Pair("value_type", "bool"). + Pair("description", ""). + Exec() + assert.NoError(t, err) + settings := modulescommon.EnsureSystemSettings(testCtx) + assert.NoError(t, settings.Reload()) + defer func() { + _, _ = testCtx.DB().DeleteFrom("system_setting"). + Where("category=? AND key_name=?", "space", "disable_user_create"). + Exec() + _ = settings.Reload() + }() + + body := util.ToJson(map[string]interface{}{ + "name": "p2-blocked-by-db", + "join_mode": 0, + }) + w := httptest.NewRecorder() + req, _ := http.NewRequest("POST", "/v1/space/create", bytes.NewReader([]byte(body))) + req.Header.Set("token", testutil.Token) + s.GetRoute().ServeHTTP(w, req) + + assert.Equal(t, http.StatusForbidden, w.Code, "DB 开关 ON 时应返回 403, body=%s", w.Body.String()) + assert.Contains(t, w.Body.String(), "已关闭") + + var count int + _, err = testCtx.DB().SelectBySql("SELECT COUNT(*) FROM space WHERE name=?", "p2-blocked-by-db").Load(&count) + assert.NoError(t, err) + assert.Equal(t, 0, count, "DB 开关 ON 时不应写入任何 space 记录") +} + func TestCreateSpace_DisabledByEnv(t *testing.T) { s, _, err := setup(t) assert.NoError(t, err) @@ -1807,3 +1857,101 @@ func TestRejectJoinApply_DoesNotConsumeInvite(t *testing.T) { assert.Equal(t, 2, updated.Status) assert.Equal(t, testutil.UID, updated.ReviewerUID) } + +// TestE2E_DisableUserCreateSpace_FullChain 串起完整的 admin 实时调控链路: +// +// manager POST /v1/manager/common/system_setting (写 disable_user_create=1) +// → 客户端 GET /v1/common/appconfig (看到 disable_user_create_space=1) +// → 用户 POST /v1/space/create (403) +// → manager POST 写回 0 +// → 用户 POST /v1/space/create (200) +// +// 这条 e2e 守住 "DB 单一真源 + Reload 即时生效 + 前后端用同一 getter" 的整条链路, +// 任一节点漂移(写路径未触发 Reload、appconfig 漏字段、createSpace 走老 env-only +// 路径)都会让本用例失败。 +func TestE2E_DisableUserCreateSpace_FullChain(t *testing.T) { + srv, _, err := setup(t) + assert.NoError(t, err) + t.Setenv(envDisableUserCreateSpace, "") + t.Setenv("OCTO_MASTER_KEY", "0123456789abcdef0123456789abcdef") + + // CleanAllTables 清空了 app_config,/v1/common/appconfig 没拿到行会 400。 + // 这里灌一行默认 app_config(其余 NOT NULL 列在 schema 里都有 DEFAULT), + // 让 appconfig handler 走到我们要验证的字段下发路径。 + _, err = testCtx.DB().InsertInto("app_config").Pair("version", 1).Exec() + assert.NoError(t, err) + + // 给 testutil.Token 升 super admin 角色,以便调用 manager 写接口。 + // CleanAllTables 不会清缓存里的 token 行,但 setup 内已重置一次,这里覆盖 + // 上层角色到 SuperAdmin。还原也走 cache.Set,无副作用。 + cfg := testCtx.GetConfig() + tokenKey := cfg.Cache.TokenCachePrefix + testutil.Token + origTokenVal, _ := testCtx.Cache().Get(tokenKey) + assert.NoError(t, testCtx.Cache().Set(tokenKey, + testutil.UID+"@test@"+string(wkhttp.SuperAdmin))) + defer func() { _ = testCtx.Cache().Set(tokenKey, origTokenVal) }() + + defer func() { + // 不论用例分支如何退出,把 system_setting 行清掉避免污染后续测试。 + _, _ = testCtx.DB().DeleteFrom("system_setting"). + Where("category=? AND key_name=?", "space", "disable_user_create"). + Exec() + _ = modulescommon.EnsureSystemSettings(testCtx).Reload() + }() + + writeSetting := func(value string) { + t.Helper() + body := util.ToJson(map[string]interface{}{ + "items": []map[string]string{{ + "category": "space", + "key": "disable_user_create", + "value": value, + }}, + }) + w := httptest.NewRecorder() + req, _ := http.NewRequest("POST", + "/v1/manager/common/system_setting", + bytes.NewReader([]byte(body))) + req.Header.Set("token", testutil.Token) + srv.GetRoute().ServeHTTP(w, req) + assert.Equal(t, http.StatusOK, w.Code, + "manager 写 disable_user_create=%s 应 200, body=%s", value, w.Body.String()) + } + + getAppconfig := func() string { + t.Helper() + w := httptest.NewRecorder() + req, _ := http.NewRequest("GET", "/v1/common/appconfig", nil) + srv.GetRoute().ServeHTTP(w, req) + assert.Equal(t, http.StatusOK, w.Code, "appconfig 应 200, body=%s", w.Body.String()) + return w.Body.String() + } + + createSpace := func(name string) int { + t.Helper() + body := util.ToJson(map[string]interface{}{ + "name": name, + "join_mode": 0, + }) + w := httptest.NewRecorder() + req, _ := http.NewRequest("POST", "/v1/space/create", + bytes.NewReader([]byte(body))) + req.Header.Set("token", testutil.Token) + srv.GetRoute().ServeHTTP(w, req) + return w.Code + } + + // --- Step 1: 关闭 --- + writeSetting("1") + assert.Contains(t, getAppconfig(), `"disable_user_create_space":1`, + "manager 写入后 appconfig 必须立刻下发 1") + assert.Equal(t, http.StatusForbidden, createSpace("e2e-off"), + "开关 ON 时 createSpace 必须 403") + + // --- Step 2: 重新打开 --- + writeSetting("0") + assert.Contains(t, getAppconfig(), `"disable_user_create_space":0`, + "manager 写回 0 后 appconfig 必须立刻下发 0") + assert.Equal(t, http.StatusOK, createSpace("e2e-on"), + "开关 OFF 时 createSpace 必须 200") +} diff --git a/modules/voice_adapter/adapter.go b/modules/voice_adapter/adapter.go index 0a35f9f5..405de471 100644 --- a/modules/voice_adapter/adapter.go +++ b/modules/voice_adapter/adapter.go @@ -1,6 +1,7 @@ package voice_adapter import ( + "encoding/json" "errors" "net/http" "os" @@ -39,6 +40,7 @@ func (a *VoiceAdapter) Route(r *wkhttp.WKHttp) { auth.PUT("/local-config", a.putLocalConfig) auth.GET("/local-config", a.getLocalConfig) auth.DELETE("/local-config", a.deleteLocalConfig) + auth.POST("/local-config/reset", a.resetLocalConfig) } } @@ -265,15 +267,35 @@ func (a *VoiceAdapter) getLocalConfig(c *wkhttp.Context) { c.JSON(http.StatusOK, resp) } -func (a *VoiceAdapter) deleteLocalConfig(c *wkhttp.Context) { +func (a *VoiceAdapter) resetLocalConfig(c *wkhttp.Context) { loginUID := c.GetLoginUID() + c.Request.Body = http.MaxBytesReader(c.Writer, c.Request.Body, 64*1024) + + var req struct { + Enabled *bool `json:"enabled"` + } + if err := c.ShouldBindJSON(&req); err != nil { + var maxErr *http.MaxBytesError + if errors.As(err, &maxErr) { + c.JSON(http.StatusRequestEntityTooLarge, gin.H{"status": http.StatusRequestEntityTooLarge, "msg": "request body too large"}) + return + } + c.JSON(http.StatusBadRequest, gin.H{"status": http.StatusBadRequest, "msg": "invalid request body"}) + return + } + spaceID := getSpaceID(c) if spaceID == "" { c.JSON(http.StatusBadRequest, gin.H{"status": http.StatusBadRequest, "msg": "X-Space-ID header is required"}) return } + if req.Enabled == nil { + c.JSON(http.StatusBadRequest, gin.H{"status": http.StatusBadRequest, "msg": "enabled is required"}) + return + } + isMember, err := space.CheckMembership(a.ctx.DB(), spaceID, loginUID) if err != nil { a.Error("check space membership failed", zap.Error(err)) @@ -285,13 +307,58 @@ func (a *VoiceAdapter) deleteLocalConfig(c *wkhttp.Context) { return } - err = a.client.DeleteLocalConfig(c.Request.Context(), loginUID, "space", spaceID) + err = a.client.ResetLocalConfig(c.Request.Context(), loginUID, "space", spaceID, *req.Enabled) if err != nil { var svcErr *SpeechServiceError if errors.As(err, &svcErr) && svcErr.StatusCode >= 400 && svcErr.StatusCode < 500 { c.JSON(svcErr.StatusCode, gin.H{"status": svcErr.StatusCode, "msg": svcErr.Body}) return } + a.Error("reset local config failed", zap.Error(err)) + c.JSON(http.StatusBadGateway, gin.H{"status": http.StatusBadGateway, "msg": "speech service unavailable"}) + return + } + c.JSON(http.StatusOK, gin.H{"status": http.StatusOK, "msg": "ok"}) +} + +func (a *VoiceAdapter) deleteLocalConfig(c *wkhttp.Context) { + loginUID := c.GetLoginUID() + + spaceID := getSpaceID(c) + if spaceID == "" { + c.JSON(http.StatusBadRequest, gin.H{"status": http.StatusBadRequest, "msg": "X-Space-ID header is required"}) + return + } + + isMember, err := space.CheckMembership(a.ctx.DB(), spaceID, loginUID) + if err != nil { + a.Error("check space membership failed", zap.Error(err)) + c.JSON(http.StatusInternalServerError, gin.H{"status": http.StatusInternalServerError, "msg": "check space membership failed"}) + return + } + if !isMember { + c.JSON(http.StatusForbidden, gin.H{"status": http.StatusForbidden, "msg": "not a member of this space"}) + return + } + + err = a.client.DeleteLocalConfig(c.Request.Context(), loginUID, "space", spaceID) + if err != nil { + var svcErr *SpeechServiceError + if errors.As(err, &svcErr) { + if svcErr.StatusCode == 404 { + var parsed struct { + Status int `json:"status"` + } + if json.Unmarshal([]byte(svcErr.Body), &parsed) == nil && parsed.Status == 404 { + c.JSON(http.StatusOK, gin.H{"status": http.StatusOK, "msg": "ok"}) + return + } + } + if svcErr.StatusCode >= 400 && svcErr.StatusCode < 500 { + c.JSON(svcErr.StatusCode, gin.H{"status": svcErr.StatusCode, "msg": svcErr.Body}) + return + } + } a.Error("delete local config failed", zap.Error(err)) c.JSON(http.StatusBadGateway, gin.H{"status": http.StatusBadGateway, "msg": "speech service unavailable"}) return diff --git a/modules/voice_adapter/adapter_test.go b/modules/voice_adapter/adapter_test.go index 3171be04..6d4dd8eb 100644 --- a/modules/voice_adapter/adapter_test.go +++ b/modules/voice_adapter/adapter_test.go @@ -939,6 +939,91 @@ func TestDeleteLocalConfigHandler_5xxReturnsBadGateway(t *testing.T) { } } +func TestDeleteLocalConfigHandler_Speech404ConfigNotFound(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + w.Write([]byte(`{"msg":"config not found","status":404}`)) + })) + defer srv.Close() + + ctx, mock, cleanup := newTestContextWithMockDB(t) + defer cleanup() + mock.ExpectQuery("SELECT COUNT").WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(1)) + + a := newTestAdapterWithDB(srv.URL, ctx) + gin.SetMode(gin.TestMode) + rec := httptest.NewRecorder() + gc, _ := gin.CreateTestContext(rec) + gc.Request = httptest.NewRequest(http.MethodDelete, "/v1/voice/local-config", nil) + gc.Request.Header.Set("X-Space-ID", "space1") + gc.Set("uid", "user1") + wkCtx := &wkhttp.Context{Context: gc} + a.deleteLocalConfig(wkCtx) + + if rec.Code != http.StatusOK { + t.Fatalf("expected 200, got %d", rec.Code) + } + var body map[string]interface{} + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("decode response: %v", err) + } + if body["msg"] != "ok" { + t.Errorf("expected msg='ok', got %v", body["msg"]) + } +} + +func TestDeleteLocalConfigHandler_Speech404InvalidJSON(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + w.Write([]byte(`not json at all`)) + })) + defer srv.Close() + + ctx, mock, cleanup := newTestContextWithMockDB(t) + defer cleanup() + mock.ExpectQuery("SELECT COUNT").WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(1)) + + a := newTestAdapterWithDB(srv.URL, ctx) + gin.SetMode(gin.TestMode) + rec := httptest.NewRecorder() + gc, _ := gin.CreateTestContext(rec) + gc.Request = httptest.NewRequest(http.MethodDelete, "/v1/voice/local-config", nil) + gc.Request.Header.Set("X-Space-ID", "space1") + gc.Set("uid", "user1") + wkCtx := &wkhttp.Context{Context: gc} + a.deleteLocalConfig(wkCtx) + + if rec.Code != http.StatusNotFound { + t.Fatalf("expected 404, got %d", rec.Code) + } +} + +func TestDeleteLocalConfigHandler_Speech404WrongStatus(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + w.Write([]byte(`{"msg":"error","status":500}`)) + })) + defer srv.Close() + + ctx, mock, cleanup := newTestContextWithMockDB(t) + defer cleanup() + mock.ExpectQuery("SELECT COUNT").WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(1)) + + a := newTestAdapterWithDB(srv.URL, ctx) + gin.SetMode(gin.TestMode) + rec := httptest.NewRecorder() + gc, _ := gin.CreateTestContext(rec) + gc.Request = httptest.NewRequest(http.MethodDelete, "/v1/voice/local-config", nil) + gc.Request.Header.Set("X-Space-ID", "space1") + gc.Set("uid", "user1") + wkCtx := &wkhttp.Context{Context: gc} + a.deleteLocalConfig(wkCtx) + + if rec.Code != http.StatusNotFound { + t.Fatalf("expected 404, got %d", rec.Code) + } +} + func TestGetConfig_NonMember(t *testing.T) { ctx, mock, cleanup := newTestContextWithMockDB(t) defer cleanup() @@ -1021,6 +1106,229 @@ func TestPutLocalConfig_BodyTooLarge(t *testing.T) { } } +func TestResetLocalConfig_Success(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + t.Errorf("expected POST, got %s", r.Method) + } + if r.URL.Path != "/v1/speech/local-config/reset" { + t.Errorf("unexpected path: %s", r.URL.Path) + } + var body map[string]interface{} + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + t.Errorf("decode body: %v", err) + } + if body["subject_id"] != "user1" { + t.Errorf("expected subject_id=user1, got %v", body["subject_id"]) + } + if body["scope_type"] != "space" { + t.Errorf("expected scope_type=space, got %v", body["scope_type"]) + } + if body["scope_id"] != "space1" { + t.Errorf("expected scope_id=space1, got %v", body["scope_id"]) + } + if body["enabled"] != true { + t.Errorf("expected enabled=true, got %v", body["enabled"]) + } + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"status":200,"msg":"ok"}`)) + })) + defer srv.Close() + + ctx, mock, cleanup := newTestContextWithMockDB(t) + defer cleanup() + mock.ExpectQuery("SELECT COUNT").WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(1)) + + a := newTestAdapterWithDB(srv.URL, ctx) + gin.SetMode(gin.TestMode) + rec := httptest.NewRecorder() + gc, _ := gin.CreateTestContext(rec) + reqBody := `{"enabled":true}` + gc.Request = httptest.NewRequest(http.MethodPost, "/v1/voice/local-config/reset", bytes.NewBufferString(reqBody)) + gc.Request.Header.Set("Content-Type", "application/json") + gc.Request.Header.Set("X-Space-ID", "space1") + gc.Set("uid", "user1") + wkCtx := &wkhttp.Context{Context: gc} + a.resetLocalConfig(wkCtx) + + if rec.Code != http.StatusOK { + t.Fatalf("expected 200, got %d", rec.Code) + } + var body map[string]interface{} + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("decode response: %v", err) + } + if body["msg"] != "ok" { + t.Errorf("expected msg='ok', got %v", body["msg"]) + } +} + +func TestResetLocalConfig_MissingSpaceID(t *testing.T) { + a := newTestAdapter("http://unused") + gin.SetMode(gin.TestMode) + rec := httptest.NewRecorder() + gc, _ := gin.CreateTestContext(rec) + reqBody := `{"enabled":true}` + gc.Request = httptest.NewRequest(http.MethodPost, "/v1/voice/local-config/reset", bytes.NewBufferString(reqBody)) + gc.Request.Header.Set("Content-Type", "application/json") + gc.Set("uid", "user1") + ctx := &wkhttp.Context{Context: gc} + a.resetLocalConfig(ctx) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d", rec.Code) + } + var body map[string]interface{} + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("decode response: %v", err) + } + if body["msg"] != "X-Space-ID header is required" { + t.Errorf("expected msg='X-Space-ID header is required', got %v", body["msg"]) + } +} + +func TestResetLocalConfig_MissingEnabled(t *testing.T) { + ctx, _, cleanup := newTestContextWithMockDB(t) + defer cleanup() + + a := newTestAdapterWithDB("http://unused", ctx) + gin.SetMode(gin.TestMode) + rec := httptest.NewRecorder() + gc, _ := gin.CreateTestContext(rec) + reqBody := `{}` + gc.Request = httptest.NewRequest(http.MethodPost, "/v1/voice/local-config/reset", bytes.NewBufferString(reqBody)) + gc.Request.Header.Set("Content-Type", "application/json") + gc.Request.Header.Set("X-Space-ID", "space1") + gc.Set("uid", "user1") + wkCtx := &wkhttp.Context{Context: gc} + a.resetLocalConfig(wkCtx) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d", rec.Code) + } + var body map[string]interface{} + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("decode response: %v", err) + } + if body["msg"] != "enabled is required" { + t.Errorf("expected msg='enabled is required', got %v", body["msg"]) + } +} + +func TestResetLocalConfig_NotMember(t *testing.T) { + ctx, mock, cleanup := newTestContextWithMockDB(t) + defer cleanup() + mock.ExpectQuery("SELECT COUNT").WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(0)) + + a := newTestAdapterWithDB("http://unused", ctx) + gin.SetMode(gin.TestMode) + rec := httptest.NewRecorder() + gc, _ := gin.CreateTestContext(rec) + reqBody := `{"enabled":true}` + gc.Request = httptest.NewRequest(http.MethodPost, "/v1/voice/local-config/reset", bytes.NewBufferString(reqBody)) + gc.Request.Header.Set("Content-Type", "application/json") + gc.Request.Header.Set("X-Space-ID", "space1") + gc.Set("uid", "user1") + wkCtx := &wkhttp.Context{Context: gc} + a.resetLocalConfig(wkCtx) + + if rec.Code != http.StatusForbidden { + t.Fatalf("expected 403, got %d", rec.Code) + } + var body map[string]interface{} + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("decode response: %v", err) + } + if body["msg"] != "not a member of this space" { + t.Errorf("expected msg='not a member of this space', got %v", body["msg"]) + } +} + +func TestResetLocalConfig_SpeechError(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + w.Write([]byte("internal error")) + })) + defer srv.Close() + + ctx, mock, cleanup := newTestContextWithMockDB(t) + defer cleanup() + mock.ExpectQuery("SELECT COUNT").WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(1)) + + a := newTestAdapterWithDB(srv.URL, ctx) + gin.SetMode(gin.TestMode) + rec := httptest.NewRecorder() + gc, _ := gin.CreateTestContext(rec) + reqBody := `{"enabled":false}` + gc.Request = httptest.NewRequest(http.MethodPost, "/v1/voice/local-config/reset", bytes.NewBufferString(reqBody)) + gc.Request.Header.Set("Content-Type", "application/json") + gc.Request.Header.Set("X-Space-ID", "space1") + gc.Set("uid", "user1") + wkCtx := &wkhttp.Context{Context: gc} + a.resetLocalConfig(wkCtx) + + if rec.Code != http.StatusBadGateway { + t.Fatalf("expected 502, got %d", rec.Code) + } +} + +func TestResetLocalConfig_BodyTooLarge(t *testing.T) { + ctx, _, cleanup := newTestContextWithMockDB(t) + defer cleanup() + + a := newTestAdapterWithDB("http://unused", ctx) + gin.SetMode(gin.TestMode) + rec := httptest.NewRecorder() + gc, _ := gin.CreateTestContext(rec) + largeBody := `{"enabled":true,"extra":"` + strings.Repeat("a", 65*1024) + `"}` + gc.Request = httptest.NewRequest(http.MethodPost, "/v1/voice/local-config/reset", bytes.NewBufferString(largeBody)) + gc.Request.Header.Set("Content-Type", "application/json") + gc.Request.Header.Set("X-Space-ID", "space1") + gc.Set("uid", "user1") + wkCtx := &wkhttp.Context{Context: gc} + a.resetLocalConfig(wkCtx) + + if rec.Code != http.StatusRequestEntityTooLarge { + t.Fatalf("expected 413, got %d", rec.Code) + } + var body map[string]interface{} + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("decode response: %v", err) + } + if body["msg"] != "request body too large" { + t.Errorf("expected msg='request body too large', got %v", body["msg"]) + } +} + +func TestResetLocalConfig_MembershipDBError(t *testing.T) { + ctx, mock, cleanup := newTestContextWithMockDB(t) + defer cleanup() + mock.ExpectQuery("SELECT COUNT").WillReturnError(sqlmock.ErrCancelled) + + a := newTestAdapterWithDB("http://unused", ctx) + gin.SetMode(gin.TestMode) + rec := httptest.NewRecorder() + gc, _ := gin.CreateTestContext(rec) + reqBody := `{"enabled":true}` + gc.Request = httptest.NewRequest(http.MethodPost, "/v1/voice/local-config/reset", bytes.NewBufferString(reqBody)) + gc.Request.Header.Set("Content-Type", "application/json") + gc.Request.Header.Set("X-Space-ID", "space1") + gc.Set("uid", "user1") + wkCtx := &wkhttp.Context{Context: gc} + a.resetLocalConfig(wkCtx) + + if rec.Code != http.StatusInternalServerError { + t.Fatalf("expected 500, got %d", rec.Code) + } + var body map[string]interface{} + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("decode response: %v", err) + } + if body["msg"] != "check space membership failed" { + t.Errorf("expected msg='check space membership failed', got %v", body["msg"]) + } +} + func TestGetContext_QuerySpaceIDIgnored(t *testing.T) { ctx, _, cleanup := newTestContextWithMockDB(t) defer cleanup() diff --git a/modules/voice_adapter/client.go b/modules/voice_adapter/client.go index 7f8005a8..56057a6e 100644 --- a/modules/voice_adapter/client.go +++ b/modules/voice_adapter/client.go @@ -263,6 +263,39 @@ func (c *SpeechClient) GetLocalConfig(ctx context.Context, subjectID, scopeType, return result, nil } +func (c *SpeechClient) ResetLocalConfig(ctx context.Context, subjectID, scopeType, scopeID string, enabled bool) error { + payload := map[string]interface{}{ + "subject_id": subjectID, + "scope_type": scopeType, + "scope_id": scopeID, + "enabled": enabled, + } + + body, err := json.Marshal(payload) + if err != nil { + return fmt.Errorf("marshal request: %w", err) + } + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, c.baseURL+"/v1/speech/local-config/reset", bytes.NewReader(body)) + if err != nil { + return fmt.Errorf("create request: %w", err) + } + req.Header.Set("Authorization", "Bearer "+c.apiKey) + req.Header.Set("Content-Type", "application/json") + + resp, err := c.client.Do(req) + if err != nil { + return fmt.Errorf("do request: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + respBody, _ := io.ReadAll(resp.Body) + return &SpeechServiceError{StatusCode: resp.StatusCode, Body: string(respBody)} + } + return nil +} + func (c *SpeechClient) DeleteLocalConfig(ctx context.Context, subjectID, scopeType, scopeID string) error { params := url.Values{} params.Set("subject_id", subjectID) diff --git a/pkg/i18n/cmd/octo-i18n-extract/extract.go b/pkg/i18n/cmd/octo-i18n-extract/extract.go new file mode 100644 index 00000000..65a67683 --- /dev/null +++ b/pkg/i18n/cmd/octo-i18n-extract/extract.go @@ -0,0 +1,220 @@ +package main + +import ( + "fmt" + "go/ast" + "go/parser" + "go/token" + "os" + "path/filepath" + "sort" + "strconv" + "strings" +) + +// Marker is the minimal go-i18n message marker emitted per registered Code. +// We only carry ID + DefaultMessage because the AST extractor is strictly a +// source-of-truth dump for translators (D18); HTTPStatus / SafeDetailKeys / +// Internal don't belong in the runtime TOML. +type Marker struct { + // ID is the stable i18n key (e.g. "err.shared.auth.required"). + ID string + // DefaultMessage is the en-US source text the marker exposes as `other`. + DefaultMessage string + // Pos is the file:line where the Register call was found; used purely + // for human-readable error messages on duplicate / non-literal failures. + Pos string +} + +// recognizedFuncNames is the closed set of call expressions the extractor +// treats as a Code registration. Both shapes appear in-repo today: +// +// - pkg/i18n/codes/shared.go uses package-local `Register(Code{...})` +// - pkg/errcode/server.go uses a wrapper `register(codes.Code{...})` +// +// Adding a new wrapper requires updating this set AND adding a test fixture; +// silently accepting any *register* name would let typos pass through. +var recognizedFuncNames = map[string]struct{}{ + "Register": {}, + "register": {}, +} + +// ExtractFromDir walks `root` and extracts a Marker for every Code{...} +// composite literal passed to one of recognizedFuncNames. +// +// Strictness rationale (these conditions fail the run, not warn): +// - ID / DefaultMessage MUST be basic string literals; computed values +// defeat the static guarantee that markers match codes.Register input. +// - Duplicate IDs MUST fail; downstream goi18n merge produces undefined +// output on collisions. +// +// Test files (`*_test.go`) and directories whose name begins with `.` or `_` +// are skipped (the latter mirrors `go build`'s convention so test fixtures +// under `_fixtures/` don't pollute the marker set). +func ExtractFromDir(root string) ([]Marker, error) { + fset := token.NewFileSet() + var markers []Marker + + walkErr := filepath.Walk(root, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if info.IsDir() { + name := info.Name() + if path != root && (strings.HasPrefix(name, ".") || strings.HasPrefix(name, "_")) { + return filepath.SkipDir + } + return nil + } + if !strings.HasSuffix(path, ".go") || strings.HasSuffix(path, "_test.go") { + return nil + } + file, err := parser.ParseFile(fset, path, nil, parser.SkipObjectResolution) + if err != nil { + return fmt.Errorf("parse %s: %w", path, err) + } + fileMarkers, err := extractFromFile(fset, file) + if err != nil { + return err + } + markers = append(markers, fileMarkers...) + return nil + }) + if walkErr != nil { + return nil, walkErr + } + + if err := checkDuplicates(markers); err != nil { + return nil, err + } + + sort.SliceStable(markers, func(i, j int) bool { return markers[i].ID < markers[j].ID }) + return markers, nil +} + +func extractFromFile(fset *token.FileSet, file *ast.File) ([]Marker, error) { + var ( + out []Marker + ferr error + ) + ast.Inspect(file, func(n ast.Node) bool { + // Once ferr is set we stop creating new state, but ast.Inspect's + // `return false` only skips the current subtree — siblings still + // trigger the visitor, which re-checks ferr and short-circuits. Net + // effect: first error wins, no further work happens, traversal walks + // the rest of the file harmlessly. Acceptable for tool-binary use. + if ferr != nil { + return false + } + call, ok := n.(*ast.CallExpr) + if !ok { + return true + } + if !isRecognizedRegisterCall(call) || len(call.Args) != 1 { + return true + } + lit, ok := call.Args[0].(*ast.CompositeLit) + if !ok || !isCodeType(lit.Type) { + return true + } + id, msg, err := readCodeLiteral(lit) + if err != nil { + ferr = fmt.Errorf("%s: %w", fset.Position(call.Pos()), err) + return false + } + // Missing ID or DefaultMessage is a hard error, not a silent skip. + // Register() also panics at runtime, but relying on that means a + // typo'd Code literal disappears from the marker set and surfaces + // only as a recall-check mismatch in main.go with no source + // position. Failing here matches the rest of this file's strictness + // rationale and gives reviewers an actionable file:line. + if id == "" || msg == "" { + ferr = fmt.Errorf("%s: Code literal is missing ID or DefaultMessage; both are required for a marker", fset.Position(call.Pos())) + return false + } + out = append(out, Marker{ + ID: id, + DefaultMessage: msg, + Pos: fset.Position(call.Pos()).String(), + }) + return true + }) + return out, ferr +} + +func isRecognizedRegisterCall(call *ast.CallExpr) bool { + switch fn := call.Fun.(type) { + case *ast.Ident: + _, ok := recognizedFuncNames[fn.Name] + return ok + case *ast.SelectorExpr: + _, ok := recognizedFuncNames[fn.Sel.Name] + return ok + } + return false +} + +// isCodeType returns true when the composite literal's type is `Code` or a +// selector ending in `.Code`. Anything else (e.g. `Other{}` inside a fake +// Register call) is ignored — false positives would leak unrelated structs +// into the marker output. +func isCodeType(t ast.Expr) bool { + switch tt := t.(type) { + case *ast.Ident: + return tt.Name == "Code" + case *ast.SelectorExpr: + return tt.Sel.Name == "Code" + } + return false +} + +func readCodeLiteral(lit *ast.CompositeLit) (id, msg string, err error) { + for _, elt := range lit.Elts { + kv, ok := elt.(*ast.KeyValueExpr) + if !ok { + continue + } + key, ok := kv.Key.(*ast.Ident) + if !ok { + continue + } + switch key.Name { + case "ID": + s, ok := basicStringLit(kv.Value) + if !ok { + return "", "", fmt.Errorf("Code.ID must be a string literal") + } + id = s + case "DefaultMessage": + s, ok := basicStringLit(kv.Value) + if !ok { + return "", "", fmt.Errorf("Code.DefaultMessage must be a string literal (id=%q)", id) + } + msg = s + } + } + return id, msg, nil +} + +func basicStringLit(e ast.Expr) (string, bool) { + lit, ok := e.(*ast.BasicLit) + if !ok || lit.Kind != token.STRING { + return "", false + } + s, err := strconv.Unquote(lit.Value) + if err != nil { + return "", false + } + return s, true +} + +func checkDuplicates(ms []Marker) error { + seen := make(map[string]string, len(ms)) + for _, m := range ms { + if prev, ok := seen[m.ID]; ok { + return fmt.Errorf("duplicate Code.ID %q registered at %s and %s", m.ID, prev, m.Pos) + } + seen[m.ID] = m.Pos + } + return nil +} diff --git a/pkg/i18n/cmd/octo-i18n-extract/extract_test.go b/pkg/i18n/cmd/octo-i18n-extract/extract_test.go new file mode 100644 index 00000000..525d376b --- /dev/null +++ b/pkg/i18n/cmd/octo-i18n-extract/extract_test.go @@ -0,0 +1,223 @@ +package main + +import ( + "os" + "path/filepath" + "sort" + "testing" +) + +// writeFile is a tiny helper that creates a fake Go source under a temp dir +// and returns its path. Tests use this to build representative fixtures of +// the codes.Register / errcode.register call shapes that ExtractFromDir must +// recognize without compiling the fixtures. +func writeFile(t *testing.T, dir, name, body string) string { + t.Helper() + p := filepath.Join(dir, name) + if err := os.WriteFile(p, []byte(body), 0o644); err != nil { + t.Fatalf("write %s: %v", p, err) + } + return p +} + +func TestExtractFromDir_RegisterPatterns(t *testing.T) { + cases := []struct { + name string + src string + want []Marker + }{ + { + name: "codes_Register direct", + src: `package fake +import "net/http" +func _() { + Register(Code{ + ID: "err.shared.auth.required", + HTTPStatus: http.StatusUnauthorized, + DefaultMessage: "Please log in to continue.", + }) +}`, + want: []Marker{{ID: "err.shared.auth.required", DefaultMessage: "Please log in to continue."}}, + }, + { + name: "errcode_register wrapper + codes.Code", + src: `package fake +import ( + "net/http" + "github.com/Mininglamp-OSS/octo-server/pkg/i18n/codes" +) +var ErrX = register(codes.Code{ + ID: "err.server.thread.not_found", + HTTPStatus: http.StatusNotFound, + DefaultMessage: "Thread not found.", +})`, + want: []Marker{{ID: "err.server.thread.not_found", DefaultMessage: "Thread not found."}}, + }, + { + name: "multiple in one file", + src: `package fake +func _() { + Register(Code{ID: "err.shared.a", DefaultMessage: "A"}) + Register(Code{ID: "err.shared.b", DefaultMessage: "B"}) +}`, + want: []Marker{ + {ID: "err.shared.a", DefaultMessage: "A"}, + {ID: "err.shared.b", DefaultMessage: "B"}, + }, + }, + { + name: "ignores non-Code composite literals", + src: `package fake +type Other struct{ ID, DefaultMessage string } +func _() { + Register(Other{ID: "ignored", DefaultMessage: "x"}) +}`, + want: nil, + }, + { + name: "ignores Register call without composite literal", + src: `package fake +func _() { + var c Code + Register(c) +}`, + want: nil, + }, + } + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "fake.go", tc.src) + got, err := ExtractFromDir(dir) + if err != nil { + t.Fatalf("ExtractFromDir: %v", err) + } + sort.Slice(got, func(i, j int) bool { return got[i].ID < got[j].ID }) + sort.Slice(tc.want, func(i, j int) bool { return tc.want[i].ID < tc.want[j].ID }) + if len(got) != len(tc.want) { + t.Fatalf("len got=%d want=%d (got=%v)", len(got), len(tc.want), got) + } + for i := range got { + if got[i].ID != tc.want[i].ID || got[i].DefaultMessage != tc.want[i].DefaultMessage { + t.Errorf("[%d] got=%+v want=%+v", i, got[i], tc.want[i]) + } + } + }) + } +} + +func TestExtractFromDir_RejectsNonStringLiterals(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "bad.go", `package fake +const msg = "hi" +func _() { + Register(Code{ID: "err.shared.x", DefaultMessage: msg}) +}`) + _, err := ExtractFromDir(dir) + if err == nil { + t.Fatalf("expected error for non-literal DefaultMessage, got nil") + } +} + +func TestExtractFromDir_RejectsDuplicateID(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "dup.go", `package fake +func _() { + Register(Code{ID: "err.shared.dup", DefaultMessage: "a"}) + Register(Code{ID: "err.shared.dup", DefaultMessage: "b"}) +}`) + _, err := ExtractFromDir(dir) + if err == nil { + t.Fatalf("expected error for duplicate ID, got nil") + } +} + +func TestExtractFromDir_SkipsTestFiles(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "x_test.go", `package fake +func _() { + Register(Code{ID: "err.shared.testonly", DefaultMessage: "T"}) +}`) + got, err := ExtractFromDir(dir) + if err != nil { + t.Fatalf("ExtractFromDir: %v", err) + } + if len(got) != 0 { + t.Fatalf("expected test file to be skipped, got %v", got) + } +} + +// TestExtractFromDir_RejectsEmptyFields locks the strict-fail upgrade: a +// Code literal missing ID or DefaultMessage is a typo, not a deliberate +// skip signal, and must surface with a file:line position rather than +// silently disappearing into a recall-check mismatch later. +func TestExtractFromDir_RejectsEmptyFields(t *testing.T) { + cases := []struct { + name string + src string + }{ + { + name: "empty_id", + src: `package fake +func _() { + Register(Code{ID: "", DefaultMessage: "x"}) +}`, + }, + { + name: "empty_default_message", + src: `package fake +func _() { + Register(Code{ID: "err.shared.x", DefaultMessage: ""}) +}`, + }, + { + name: "id_only", + src: `package fake +func _() { + Register(Code{ID: "err.shared.x"}) +}`, + }, + } + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "bad.go", tc.src) + _, err := ExtractFromDir(dir) + if err == nil { + t.Fatal("expected strict-fail on empty field, got nil") + } + }) + } +} + +// TestExtractFromDir_SkipsHiddenAndUnderscoreDirs codifies the convention +// that test fixtures dropped under `.hidden/` or `_fixtures/` are off-limits +// to extraction (matching `go build`'s dir-skip behavior). Without this +// fixture the convention is only described in a comment and could regress. +func TestExtractFromDir_SkipsHiddenAndUnderscoreDirs(t *testing.T) { + dir := t.TempDir() + // Real source under root — should be extracted. + writeFile(t, dir, "real.go", `package fake +func _() { + Register(Code{ID: "err.shared.real", DefaultMessage: "R"}) +}`) + for _, sub := range []string{"_fixtures", ".hidden"} { + subDir := filepath.Join(dir, sub) + if err := os.MkdirAll(subDir, 0o755); err != nil { + t.Fatalf("mkdir %s: %v", subDir, err) + } + writeFile(t, subDir, "leak.go", `package fake +func _() { + Register(Code{ID: "err.shared.LEAK_`+sub+`", DefaultMessage: "leak"}) +}`) + } + got, err := ExtractFromDir(dir) + if err != nil { + t.Fatalf("ExtractFromDir: %v", err) + } + if len(got) != 1 || got[0].ID != "err.shared.real" { + t.Fatalf("expected only real.go to extract, got %v", got) + } +} diff --git a/pkg/i18n/cmd/octo-i18n-extract/main.go b/pkg/i18n/cmd/octo-i18n-extract/main.go new file mode 100644 index 00000000..c466b393 --- /dev/null +++ b/pkg/i18n/cmd/octo-i18n-extract/main.go @@ -0,0 +1,219 @@ +// Command octo-i18n-extract is the source-of-truth AST extractor for the +// i18n message marker pipeline (D18 in `i18n 最终方案.md` v7.1). +// +// What it does: +// +// 1. Walk one or more Go source roots (default: pkg/i18n/codes pkg/errcode) +// and parse every `Register(Code{...})` / `register(codes.Code{...})` +// composite literal to recover (ID, DefaultMessage) pairs. +// 2. Route markers by ID prefix: +// - err.shared.* → /active.en-US.toml +// - err.server.* / msg.* → /active.en-US.toml +// 3. Cross-check against the registered set obtained by importing the codes +// and errcode packages. If `len(AST markers) != len(codes.All())` or the +// ID sets differ, exit non-zero — this is the 100% recall guarantee that +// Phase 2 module PRs depend on (TODOS §0.8). +// 4. With `-check`, write nothing; instead report whether any marker file +// on disk diverges from what would be written (CI use). +// +// Exit codes: 0 (clean), 1 (extraction error), 2 (recall mismatch), +// 3 (-check found diff). +// +// Typical invocations: +// +// go run ./pkg/i18n/cmd/octo-i18n-extract # rewrite markers +// go run ./pkg/i18n/cmd/octo-i18n-extract -check # CI verification +package main + +import ( + "flag" + "fmt" + "os" + "sort" + "strings" + + "github.com/Mininglamp-OSS/octo-server/pkg/i18n/codes" + // Side-effect imports: these packages register their Codes in init(). + // Listing them explicitly is what makes codes.All() complete; do NOT + // rely on transitive imports — extractor is a tool binary and must + // pin its own truth set. + _ "github.com/Mininglamp-OSS/octo-server/pkg/errcode" +) + +const ( + exitOK = 0 + exitExtractError = 1 + exitRecallMismatch = 2 + exitCheckDiff = 3 +) + +func main() { + var ( + sharedDir string + serverDir string + roots stringsFlag + check bool + ) + flag.StringVar(&sharedDir, "shared-dir", "tools/i18nmarkers/shared", "output directory for err.shared.* markers") + flag.StringVar(&serverDir, "server-dir", "tools/i18nmarkers/server", "output directory for err.server.* / msg.* markers") + flag.Var(&roots, "root", "Go source root to scan (repeatable; default: pkg/i18n/codes, pkg/errcode)") + flag.BoolVar(&check, "check", false, "verify on-disk markers match expected; exit 3 on diff, write nothing") + flag.Parse() + + if len(roots) == 0 { + roots = stringsFlag{"pkg/i18n/codes", "pkg/errcode"} + } + + markers, err := collectMarkers(roots) + if err != nil { + fmt.Fprintf(os.Stderr, "extract: %v\n", err) + os.Exit(exitExtractError) + } + registered := make([]string, 0, len(codes.All())) + for _, c := range codes.All() { + registered = append(registered, c.ID) + } + if err := verifyRecall(markers, registered); err != nil { + fmt.Fprintf(os.Stderr, "recall: %v\n", err) + os.Exit(exitRecallMismatch) + } + + groups, err := GroupByPrefix(markers) + if err != nil { + fmt.Fprintf(os.Stderr, "group: %v\n", err) + os.Exit(exitExtractError) + } + + // Iterate in a fixed order so log output (and `-check` diff hints) is + // deterministic across runs — Go map iteration would otherwise swap the + // "wrote/unchanged ... → ..." lines, hurting log diffing. + targetOrder := []string{"shared", "server"} + targets := map[string]string{ + "shared": joinPath(sharedDir, "active.en-US.toml"), + "server": joinPath(serverDir, "active.en-US.toml"), + } + + exit := exitOK + for _, group := range targetOrder { + path := targets[group] + ms := groups[group] + if check { + diff, err := checkOnDisk(path, ms) + if err != nil { + fmt.Fprintf(os.Stderr, "check %s: %v\n", path, err) + os.Exit(exitExtractError) + } + if diff { + fmt.Fprintf(os.Stderr, "diff: %s is stale (re-run `make i18n-extract`)\n", path) + exit = exitCheckDiff + } + continue + } + changed, err := WriteMarkerFile(path, ms) + if err != nil { + fmt.Fprintf(os.Stderr, "write %s: %v\n", path, err) + os.Exit(exitExtractError) + } + if changed { + fmt.Printf("wrote %d markers → %s\n", len(ms), path) + } else { + fmt.Printf("unchanged %d markers → %s\n", len(ms), path) + } + } + os.Exit(exit) +} + +// collectMarkers walks each root in turn and merges markers, checking for +// duplicate IDs across roots (a Code registered in two packages is always a +// bug — `codes.Register` would panic at runtime, but during static extraction +// the panic never fires). +func collectMarkers(roots []string) ([]Marker, error) { + var all []Marker + seen := make(map[string]string) + for _, r := range roots { + ms, err := ExtractFromDir(r) + if err != nil { + return nil, fmt.Errorf("root %s: %w", r, err) + } + for _, m := range ms { + if prev, ok := seen[m.ID]; ok { + return nil, fmt.Errorf("duplicate Code.ID %q across roots: %s and %s", m.ID, prev, m.Pos) + } + seen[m.ID] = m.Pos + all = append(all, m) + } + } + sort.SliceStable(all, func(i, j int) bool { return all[i].ID < all[j].ID }) + return all, nil +} + +// verifyRecall is the 100% guarantee: the AST-extracted set must match the +// runtime-registered set exactly. Diffs surface as a sorted list of missing +// and extra IDs to make CI failures actionable. +// +// `registered` is passed in as a slice (rather than calling codes.All() +// directly) so this function is unit-testable with synthetic inputs — the +// recall guarantee is the load-bearing safety property of this binary and +// deserves direct coverage independent of the live registry's state. +func verifyRecall(markers []Marker, registered []string) error { + astIDs := make(map[string]struct{}, len(markers)) + for _, m := range markers { + astIDs[m.ID] = struct{}{} + } + regIDs := make(map[string]struct{}, len(registered)) + for _, id := range registered { + regIDs[id] = struct{}{} + } + + var missing, extra []string + for id := range regIDs { + if _, ok := astIDs[id]; !ok { + missing = append(missing, id) + } + } + for id := range astIDs { + if _, ok := regIDs[id]; !ok { + extra = append(extra, id) + } + } + if len(missing) == 0 && len(extra) == 0 { + return nil + } + sort.Strings(missing) + sort.Strings(extra) + return fmt.Errorf( + "AST vs codes.All() mismatch: ast=%d registered=%d\n missing-from-AST: %v\n extra-only-in-AST: %v", + len(astIDs), len(regIDs), missing, extra, + ) +} + +// checkOnDisk returns true when the file at path differs from the rendering +// of ms (or does not exist). It never writes. +func checkOnDisk(path string, ms []Marker) (bool, error) { + want := RenderTOML(ms) + got, err := os.ReadFile(path) + if err != nil { + if os.IsNotExist(err) { + return true, nil + } + return false, err + } + return string(got) != want, nil +} + +func joinPath(dir, name string) string { + if dir == "" { + return name + } + if strings.HasSuffix(dir, "/") { + return dir + name + } + return dir + "/" + name +} + +// stringsFlag is a tiny -repeatable flag implementation; standard library has +// no built-in for `flag.Var` slice-of-string accumulation. +type stringsFlag []string + +func (s *stringsFlag) String() string { return strings.Join(*s, ",") } +func (s *stringsFlag) Set(v string) error { *s = append(*s, v); return nil } diff --git a/pkg/i18n/cmd/octo-i18n-extract/main_test.go b/pkg/i18n/cmd/octo-i18n-extract/main_test.go new file mode 100644 index 00000000..b3ae0d50 --- /dev/null +++ b/pkg/i18n/cmd/octo-i18n-extract/main_test.go @@ -0,0 +1,167 @@ +package main + +// Unit coverage for the load-bearing safety properties in main.go that the +// initial PR only exercised indirectly via `make i18n-extract`. Reviewer +// feedback on PR #186 (yujiawei P2) called out that verifyRecall, +// checkOnDisk, and collectMarkers' cross-root duplicate path deserve +// direct tests because they are what makes the 100% recall guarantee +// enforceable rather than aspirational. + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestVerifyRecall(t *testing.T) { + cases := []struct { + name string + markers []Marker + registered []string + wantErr bool + wantSubstr []string // each substring must appear in the error message + }{ + { + name: "both_empty", + markers: nil, + registered: nil, + wantErr: false, + }, + { + name: "exact_match", + markers: []Marker{{ID: "err.shared.a"}, {ID: "err.shared.b"}}, + registered: []string{"err.shared.b", "err.shared.a"}, + wantErr: false, + }, + { + name: "missing_from_ast", + markers: []Marker{{ID: "err.shared.a"}}, + registered: []string{"err.shared.a", "err.shared.b"}, + wantErr: true, + wantSubstr: []string{"missing-from-AST", "err.shared.b"}, + }, + { + name: "extra_in_ast", + markers: []Marker{{ID: "err.shared.a"}, {ID: "err.shared.z"}}, + registered: []string{"err.shared.a"}, + wantErr: true, + wantSubstr: []string{"extra-only-in-AST", "err.shared.z"}, + }, + { + name: "both_missing_and_extra", + markers: []Marker{{ID: "err.shared.a"}, {ID: "err.shared.z"}}, + registered: []string{"err.shared.a", "err.shared.b"}, + wantErr: true, + wantSubstr: []string{"missing-from-AST", "err.shared.b", "extra-only-in-AST", "err.shared.z"}, + }, + } + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + err := verifyRecall(tc.markers, tc.registered) + if (err != nil) != tc.wantErr { + t.Fatalf("verifyRecall err=%v wantErr=%v", err, tc.wantErr) + } + if err == nil { + return + } + for _, s := range tc.wantSubstr { + if !strings.Contains(err.Error(), s) { + t.Errorf("error message missing %q\n full: %s", s, err.Error()) + } + } + }) + } +} + +func TestCheckOnDisk(t *testing.T) { + dir := t.TempDir() + ms := []Marker{{ID: "err.shared.x", DefaultMessage: "X"}} + + // State 1: file absent → diff + absent := filepath.Join(dir, "absent.toml") + diff, err := checkOnDisk(absent, ms) + if err != nil { + t.Fatalf("checkOnDisk(absent): %v", err) + } + if !diff { + t.Error("absent file must report diff=true") + } + + // State 2: file matches → no diff + matching := filepath.Join(dir, "match.toml") + if err := os.WriteFile(matching, []byte(RenderTOML(ms)), 0o644); err != nil { + t.Fatalf("seed match file: %v", err) + } + diff, err = checkOnDisk(matching, ms) + if err != nil { + t.Fatalf("checkOnDisk(match): %v", err) + } + if diff { + t.Error("matching file must report diff=false") + } + + // State 3: file differs → diff + stale := filepath.Join(dir, "stale.toml") + if err := os.WriteFile(stale, []byte("# stale unrelated content\n"), 0o644); err != nil { + t.Fatalf("seed stale file: %v", err) + } + diff, err = checkOnDisk(stale, ms) + if err != nil { + t.Fatalf("checkOnDisk(stale): %v", err) + } + if !diff { + t.Error("stale file must report diff=true") + } +} + +func TestCollectMarkers_CrossRootDuplicateID(t *testing.T) { + root1 := t.TempDir() + root2 := t.TempDir() + writeFile(t, root1, "a.go", `package fake +func _() { + Register(Code{ID: "err.shared.dup", DefaultMessage: "from root1"}) +}`) + writeFile(t, root2, "b.go", `package fake +func _() { + Register(Code{ID: "err.shared.dup", DefaultMessage: "from root2"}) +}`) + + _, err := collectMarkers([]string{root1, root2}) + if err == nil { + t.Fatal("expected cross-root duplicate to fail, got nil") + } + // Error message must point at both occurrences so reviewers can locate them. + for _, want := range []string{"err.shared.dup", root1, root2} { + if !strings.Contains(err.Error(), want) { + t.Errorf("cross-root error missing %q\n full: %s", want, err.Error()) + } + } +} + +func TestCollectMarkers_MergesDistinctRoots(t *testing.T) { + root1 := t.TempDir() + root2 := t.TempDir() + writeFile(t, root1, "a.go", `package fake +func _() { + Register(Code{ID: "err.shared.a", DefaultMessage: "A"}) +}`) + writeFile(t, root2, "b.go", `package fake +func _() { + Register(Code{ID: "err.server.b", DefaultMessage: "B"}) +}`) + + ms, err := collectMarkers([]string{root1, root2}) + if err != nil { + t.Fatalf("collectMarkers: %v", err) + } + if len(ms) != 2 { + t.Fatalf("len=%d want 2", len(ms)) + } + // Sorted output is part of the contract — main.go relies on it for + // stable downstream rendering and recall diffs. + if ms[0].ID >= ms[1].ID { + t.Errorf("collectMarkers output not sorted: %v", ms) + } +} diff --git a/pkg/i18n/cmd/octo-i18n-extract/write.go b/pkg/i18n/cmd/octo-i18n-extract/write.go new file mode 100644 index 00000000..59e35e2f --- /dev/null +++ b/pkg/i18n/cmd/octo-i18n-extract/write.go @@ -0,0 +1,112 @@ +package main + +import ( + "bytes" + "errors" + "fmt" + "os" + "path/filepath" + "sort" + "strconv" + "strings" +) + +// Prefix-to-group routing (D18). err.shared.* lives in the SDK and follows +// pkg/i18n/locales/; everything else (err.server.* and the reserved msg.* +// namespace) is server-private and ships under the repo-root locales/. +// +// The mapping is closed: unknown prefixes are a hard error. That guarantees +// that introducing a third namespace forces an explicit routing decision +// (and a Code.ID regex update in pkg/i18n/codes/registry.go). +// +// NOTE: `msg.` is a Phase 2 placeholder. pkg/i18n/codes/registry.go's +// idPattern currently restricts IDs to `err.(shared|server).*`, so any +// attempt to Register a `msg.*` Code panics at startup and never reaches +// extraction. The routing entry is intentionally pre-wired so that when +// Phase 2 widens the registry regex, this table doesn't need a parallel +// edit. Any future maintainer touching one side must update both. +var prefixGroup = map[string]string{ + "err.shared.": "shared", + "err.server.": "server", + "msg.": "server", +} + +// GroupByPrefix routes markers into their target group ("shared" or "server"). +// Returns an error if any marker's ID does not match a known prefix. +func GroupByPrefix(ms []Marker) (map[string][]Marker, error) { + out := make(map[string][]Marker) + for _, m := range ms { + group, ok := groupFor(m.ID) + if !ok { + return nil, fmt.Errorf("marker %q: unrecognized ID prefix (expected one of err.shared./err.server./msg.)", m.ID) + } + out[group] = append(out[group], m) + } + for g := range out { + ms := out[g] + sort.SliceStable(ms, func(i, j int) bool { return ms[i].ID < ms[j].ID }) + out[g] = ms + } + return out, nil +} + +func groupFor(id string) (string, bool) { + for prefix, g := range prefixGroup { + if strings.HasPrefix(id, prefix) { + return g, true + } + } + return "", false +} + +// RenderTOML produces a deterministic go-i18n marker file. Format matches +// what `goi18n merge` expects as a source: one `["id"]` table per Code with +// a single `other = "..."` key, alphabetized by ID, LF line endings, trailing +// newline. Stable output is a precondition for the CI extract-diff check. +func RenderTOML(ms []Marker) string { + sorted := append([]Marker(nil), ms...) + sort.SliceStable(sorted, func(i, j int) bool { return sorted[i].ID < sorted[j].ID }) + + var buf bytes.Buffer + buf.WriteString(markerHeader) + for i, m := range sorted { + if i > 0 { + buf.WriteByte('\n') + } + // strconv.Quote handles \n, \t, embedded quotes and unicode safely; + // go-i18n's TOML parser accepts standard basic strings. + fmt.Fprintf(&buf, "[%s]\nother = %s\n", strconv.Quote(m.ID), strconv.Quote(m.DefaultMessage)) + } + return buf.String() +} + +const markerHeader = `# AUTO-GENERATED by pkg/i18n/cmd/octo-i18n-extract. DO NOT EDIT. +# +# Source-of-truth marker file: one entry per codes.Register / errcode.register +# call in the repo. Regenerate with ` + "`make i18n-extract`" + `; the merge into +# active..toml is performed by goi18n merge (see Makefile). +# +# CI rejects PRs that change codes.Register input without re-running extract. + +` + +// WriteMarkerFile writes content (RenderTOML output) to path, creating the +// parent directory if needed. Returns changed=true only when on-disk bytes +// differ from the new payload, enabling cheap idempotency checks in CI. +func WriteMarkerFile(path string, ms []Marker) (changed bool, err error) { + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + return false, fmt.Errorf("mkdir %s: %w", filepath.Dir(path), err) + } + want := []byte(RenderTOML(ms)) + got, err := os.ReadFile(path) + if err != nil && !errors.Is(err, os.ErrNotExist) { + return false, fmt.Errorf("read %s: %w", path, err) + } + if bytes.Equal(got, want) { + return false, nil + } + if err := os.WriteFile(path, want, 0o644); err != nil { + return false, fmt.Errorf("write %s: %w", path, err) + } + return true, nil +} diff --git a/pkg/i18n/cmd/octo-i18n-extract/write_test.go b/pkg/i18n/cmd/octo-i18n-extract/write_test.go new file mode 100644 index 00000000..d926912b --- /dev/null +++ b/pkg/i18n/cmd/octo-i18n-extract/write_test.go @@ -0,0 +1,97 @@ +package main + +import ( + "os" + "path/filepath" + "testing" +) + +func TestGroupByPrefix(t *testing.T) { + in := []Marker{ + {ID: "err.shared.auth.required", DefaultMessage: "a"}, + {ID: "err.server.thread.not_found", DefaultMessage: "b"}, + {ID: "msg.notify.invite", DefaultMessage: "c"}, + {ID: "err.shared.internal", DefaultMessage: "d"}, + } + got, err := GroupByPrefix(in) + if err != nil { + t.Fatalf("GroupByPrefix: %v", err) + } + if n := len(got["shared"]); n != 2 { + t.Errorf("shared group n=%d want 2", n) + } + if n := len(got["server"]); n != 2 { + t.Errorf("server group n=%d want 2 (server + msg)", n) + } +} + +func TestGroupByPrefix_RejectsUnknown(t *testing.T) { + _, err := GroupByPrefix([]Marker{{ID: "weird.foo", DefaultMessage: "x"}}) + if err == nil { + t.Fatal("expected error for unknown prefix") + } +} + +func TestRenderTOML_StableAndIdempotent(t *testing.T) { + ms := []Marker{ + {ID: "err.shared.b", DefaultMessage: "B with \"quotes\""}, + {ID: "err.shared.a", DefaultMessage: "A\nnewline"}, + } + out1 := RenderTOML(ms) + out2 := RenderTOML(ms) + if out1 != out2 { + t.Fatalf("RenderTOML not deterministic:\n--1--\n%s\n--2--\n%s", out1, out2) + } + // Sorted: "a" must appear before "b". + if got := out1; !before(got, "err.shared.a", "err.shared.b") { + t.Fatalf("expected sorted output, got:\n%s", got) + } + // Escape sanity: literal newline / quote must not survive raw. + if contains(out1, "B with \"quotes\"") { + t.Errorf("quotes not escaped:\n%s", out1) + } +} + +func TestWriteMarkerFile_IdempotentNoChange(t *testing.T) { + dir := t.TempDir() + ms := []Marker{{ID: "err.shared.x", DefaultMessage: "X"}} + path := filepath.Join(dir, "active.en-US.toml") + changed, err := WriteMarkerFile(path, ms) + if err != nil { + t.Fatalf("first write: %v", err) + } + if !changed { + t.Errorf("first write should report changed=true") + } + changed, err = WriteMarkerFile(path, ms) + if err != nil { + t.Fatalf("second write: %v", err) + } + if changed { + t.Errorf("second write should report changed=false (idempotent)") + } + b, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read: %v", err) + } + if len(b) == 0 { + t.Error("output file is empty") + } +} + +func before(s, a, b string) bool { + i := indexOf(s, a) + j := indexOf(s, b) + return i >= 0 && j >= 0 && i < j +} + +func indexOf(s, sub string) int { + for i := 0; i+len(sub) <= len(s); i++ { + if s[i:i+len(sub)] == sub { + return i + } + } + return -1 +} + +func contains(s, sub string) bool { return indexOf(s, sub) >= 0 } diff --git a/tools/i18nmarkers/server/active.en-US.toml b/tools/i18nmarkers/server/active.en-US.toml new file mode 100644 index 00000000..e0220bc8 --- /dev/null +++ b/tools/i18nmarkers/server/active.en-US.toml @@ -0,0 +1,61 @@ +# AUTO-GENERATED by pkg/i18n/cmd/octo-i18n-extract. DO NOT EDIT. +# +# Source-of-truth marker file: one entry per codes.Register / errcode.register +# call in the repo. Regenerate with `make i18n-extract`; the merge into +# active..toml is performed by goi18n merge (see Makefile). +# +# CI rejects PRs that change codes.Register input without re-running extract. + +["err.server.thread.creator_cannot_leave"] +other = "Thread creator cannot leave the thread." + +["err.server.thread.deleted"] +other = "Thread has been deleted." + +["err.server.thread.group_md_content_empty"] +other = "GROUP.md content must not be empty." + +["err.server.thread.group_md_content_too_large"] +other = "GROUP.md content exceeds the maximum size." + +["err.server.thread.group_md_not_found"] +other = "Thread GROUP.md not found." + +["err.server.thread.group_no_invalid"] +other = "Invalid group number." + +["err.server.thread.name_invalid"] +other = "Thread name is required and must not exceed 100 characters." + +["err.server.thread.not_active"] +other = "Thread is not active." + +["err.server.thread.not_found"] +other = "Thread not found." + +["err.server.thread.not_group_member"] +other = "You are not a group member." + +["err.server.thread.permission_denied"] +other = "You do not have permission to perform this action." + +["err.server.thread.request_invalid"] +other = "Invalid request." + +["err.server.thread.setting_invalid"] +other = "Invalid thread setting." + +["err.server.thread.short_id_invalid"] +other = "Invalid thread short ID." + +["err.server.thread.source_message_invalid"] +other = "Invalid source message payload." + +["err.server.thread.status_changed"] +other = "Thread status changed concurrently." + +["err.server.thread.status_invalid"] +other = "Invalid thread status." + +["err.server.thread.store_failed"] +other = "Thread storage operation failed." diff --git a/tools/i18nmarkers/shared/active.en-US.toml b/tools/i18nmarkers/shared/active.en-US.toml new file mode 100644 index 00000000..381b7f0c --- /dev/null +++ b/tools/i18nmarkers/shared/active.en-US.toml @@ -0,0 +1,34 @@ +# AUTO-GENERATED by pkg/i18n/cmd/octo-i18n-extract. DO NOT EDIT. +# +# Source-of-truth marker file: one entry per codes.Register / errcode.register +# call in the repo. Regenerate with `make i18n-extract`; the merge into +# active..toml is performed by goi18n merge (see Makefile). +# +# CI rejects PRs that change codes.Register input without re-running extract. + +["err.shared.auth.forbidden"] +other = "You do not have permission to perform this action." + +["err.shared.auth.required"] +other = "Please log in to continue." + +["err.shared.auth.token_expired"] +other = "Authentication token has expired." + +["err.shared.auth.token_invalid"] +other = "Authentication token is invalid." + +["err.shared.auth.token_missing"] +other = "Authentication token is required." + +["err.shared.internal"] +other = "Internal server error." + +["err.shared.not_found"] +other = "Resource not found." + +["err.shared.param.invalid"] +other = "Invalid request parameter." + +["err.shared.rate.limited"] +other = "Too many requests, please try again later."