From 3fd610213df8ad06454ea4c53a9a1fbab3eccb4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=99=88=E5=85=B4=E7=82=80?= Date: Thu, 6 Aug 2026 19:44:55 +0800 Subject: [PATCH 1/3] fix(apps): detect the no-database failure by code or message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The recovery flow for "db command against an app that has no database" keyed on one business code (500002759). The server has since renumbered that case to 400002465, which silently disabled the flow: users now see the raw internal message about workspace / app-id mapping and lose the cloud-development recovery steps entirely. Nothing catches the regression. There is no compile error, the unit tests compare against the same constant they set, and the dry-run E2E does not exercise a real response — the failure only shows up against a server that has already renumbered. Detect on code OR message instead. Both known codes are kept, plus narrow lowercase markers of the server's internal wording. The two channels have opposite failure modes: a code is precise but gets renumbered, a message survives renumbering but breaks on rewording or localization. Requiring either to match means one channel changing degrades nothing, and only a simultaneous change of both regresses. Markers stay deliberately narrow. "no db branch" in particular must not also swallow env-pull's "invalid db branch" case, which needs its own hint; a comment records that widening them requires a test proving the neighbours still pass through. Classification and the cause chain are untouched: the helper still mutates the problem in place and returns the same error value. --- shortcuts/apps/common.go | 69 ++++++++++++++++++++++++++++++----- shortcuts/apps/common_test.go | 42 +++++++++++++++++++++ 2 files changed, 101 insertions(+), 10 deletions(-) diff --git a/shortcuts/apps/common.go b/shortcuts/apps/common.go index cd0572b976..fb10e6eecb 100644 --- a/shortcuts/apps/common.go +++ b/shortcuts/apps/common.go @@ -22,14 +22,22 @@ const apiBasePath = "/open-apis/spark/v1" // lark-apps SKILL.md ("app_id 获取"); the hint stays lean and does not repeat it. const appIDListHint = "verify --app-id is correct and you have access to the app; list your apps with `lark-cli apps +list`" -// appNoDatabaseCode is the Spark business code returned when a db command runs -// against an app that has not initialized a database yet. The raw server -// message for this code carries internal workspace terminology, so the CLI +// appNoDatabaseCode / appNoDatabaseLegacyCode are the Spark business codes seen +// when a db command runs against an app that has not initialized a database yet. +// The raw server message carries internal workspace terminology, so the CLI // rewrites it into a user-facing explanation and attaches a recoverable // cloud-development next step (see appNoDatabaseMessage / appNoDatabaseHint). -// The numeric code — not the message text — is the stable discriminator an -// agent harness keys on to enter the recovery flow. -const appNoDatabaseCode = 500002759 +// +// Two codes, not one: the server renumbered this case from 500002759 to +// 400002465 when the domain moved its client-class errors into the 4xx band. +// Keying on a single literal made the recovery flow disappear silently on the +// day that shipped — no compile error, no failing unit test (they assert against +// the same constant), and the dry-run E2E never reaches a live server. Hence +// isAppNoDatabaseError matches code OR message; see that function. +const ( + appNoDatabaseCode = 400002465 // current + appNoDatabaseLegacyCode = 500002759 // pre-4xx renumber; kept so older servers still match +) // appNoDatabaseMessage is the user-facing explanation for appNoDatabaseCode. // It deliberately drops internal workspace / db-branch terms. @@ -48,10 +56,10 @@ const appNoDatabaseHint = "ask the user whether to add a database through Miaoda // is filled in. Mirrors drive.appendDriveExportRecoveryHint. err==nil and // untyped errors pass through unchanged. // -// Special-case appNoDatabaseCode (500002759, db command on an app with no -// database yet): rewrite the message to a user-facing explanation and force the +// Special-case the "app has no database yet" failure (see isAppNoDatabaseError): +// rewrite the message to a user-facing explanation and force the // cloud-development recovery hint, since the raw upstream message uses internal -// terms and any generic hint would be less actionable. This code is only +// terms and any generic hint would be less actionable. That failure is only // produced by db endpoints, so the override is safe to check for every apps // command that funnels through here. func withAppsHint(err error, hint string) error { @@ -60,7 +68,7 @@ func withAppsHint(err error, hint string) error { } // p points at the embedded Problem, so the mutation is reflected in err. if p, ok := errs.ProblemOf(err); ok { - if p.Code == appNoDatabaseCode { + if isAppNoDatabaseError(p) { p.Message = appNoDatabaseMessage p.Hint = appNoDatabaseHint return err @@ -73,6 +81,47 @@ func withAppsHint(err error, hint string) error { return err } +// appNoDatabaseMessageMarkers are lowercase substrings of the raw server message +// for the no-database failure, used as a fallback when the business code is not +// one the CLI knows. They quote the server's internal vocabulary — "db branch", +// "workspace id ... app id" — which is exactly why the message gets rewritten for +// users. +// +// Deliberately narrow. A looser marker such as "workspace" alone would swallow +// neighbouring db failures that need their own hint; "no db branch" in particular +// must not also match env-pull's "invalid db branch" case +// (isEnvPullDevDBNotInitializedError). Widen only with a test proving the +// neighbours still pass through. +var appNoDatabaseMessageMarkers = []string{ + "get workspace id failed by app id", + "no db branch", +} + +// isAppNoDatabaseError reports whether a typed failure is "this app has no +// database yet", matching on business code OR raw server message. +// +// Why both: the code is the precise signal but not a stable one — the server has +// already renumbered this case once (500002759 → 400002465), and a code-only +// check fails open, silently dropping the recovery flow with nothing in CI to +// catch it. The message is the reverse trade: it survives renumbering but breaks +// on rewording or localization. Requiring either to match means one channel +// changing degrades nothing, and only a simultaneous change of both regresses. +func isAppNoDatabaseError(p *errs.Problem) bool { + if p == nil { + return false + } + if p.Code == appNoDatabaseCode || p.Code == appNoDatabaseLegacyCode { + return true + } + message := strings.ToLower(p.Message) + for _, marker := range appNoDatabaseMessageMarkers { + if strings.Contains(message, marker) { + return true + } + } + return false +} + // validateRealAppID checks that --app-id is a real app ID (app_ prefix). // meta_token values are rejected with a hint to resolve via +get first. func validateRealAppID(appID string) error { diff --git a/shortcuts/apps/common_test.go b/shortcuts/apps/common_test.go index e4b09cfe16..b767c8b5dc 100644 --- a/shortcuts/apps/common_test.go +++ b/shortcuts/apps/common_test.go @@ -92,6 +92,48 @@ func TestWithAppsHint(t *testing.T) { } }) + // The server has renumbered this case before (500002759 → 400002465). Both the + // legacy code and an unrecognized future code must still enter the recovery + // flow, so detection is code-OR-message rather than a single literal. + t.Run("legacy no-database code still enters the recovery flow", func(t *testing.T) { + in := errs.NewAPIError(errs.SubtypeUnknown, "workspace has no db branch"). + WithCode(appNoDatabaseLegacyCode) + p, _ := errs.ProblemOf(withAppsHint(in, "generic db hint")) + if p.Message != appNoDatabaseMessage || p.Hint != appNoDatabaseHint { + t.Errorf("legacy code not detected: Message=%q Hint=%q", p.Message, p.Hint) + } + }) + + t.Run("unknown code is detected by server message", func(t *testing.T) { + // Codes the CLI has never seen: only the raw message identifies the case. + for _, msg := range []string{ + "get workspace id failed by app id", + "Get Workspace Id Failed By App Id", // case-insensitive + "workspace ws_x has no db branch", // substring, not whole-string + } { + in := errs.NewAPIError(errs.SubtypeUnknown, msg).WithCode(999999999) + p, _ := errs.ProblemOf(withAppsHint(in, "generic db hint")) + if p.Message != appNoDatabaseMessage || p.Hint != appNoDatabaseHint { + t.Errorf("message %q not detected: Message=%q Hint=%q", msg, p.Message, p.Hint) + } + } + }) + + t.Run("unrelated failure keeps the caller hint", func(t *testing.T) { + // Guards against an over-broad matcher hijacking neighbouring db errors: + // "invalid db branch" (env-pull's dev-branch case) must NOT be swallowed. + for _, msg := range []string{"invalid db branch: dev", "数据表格不存在", "permission denied"} { + in := errs.NewAPIError(errs.SubtypeUnknown, msg).WithCode(400002469) + p, _ := errs.ProblemOf(withAppsHint(in, "generic db hint")) + if p.Message != msg { + t.Errorf("message %q was rewritten to %q; matcher is too broad", msg, p.Message) + } + if p.Hint != "generic db hint" { + t.Errorf("message %q got hint %q, want the caller hint", msg, p.Hint) + } + } + }) + t.Run("no-database code overrides even a preexisting upstream hint", func(t *testing.T) { // An upstream hint must NOT win here: the recovery flow is more actionable. in := errs.NewAPIError(errs.SubtypeUnknown, "internal msg"). From b877e53d3c5b3cb294a986cfa6fb1d921be7034d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=99=88=E5=85=B4=E7=82=80?= Date: Thu, 6 Aug 2026 20:19:00 +0800 Subject: [PATCH 2/3] test(apps): assert the full typed-error contract in no-database cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback: the new subtests checked only Message and Hint, so a change that reclassified the failure — or replaced the error value and dropped the cause chain — would still have passed. Each case now asserts Category, Subtype and Code are untouched by the rewrite, and that the helper returns the same error value. Inputs use a concrete subtype rather than Unknown, so a clobbered classification is actually observable. One new case wraps a cause and asserts errors.Is still finds it through the rewrite. Also covers the predicate's defensive nil guard, which withAppsHint cannot reach on its own (ProblemOf returns ok=false for untyped errors), closing the two uncovered lines the coverage report flagged. Both withAppsHint and isAppNoDatabaseError are now at 100%. --- shortcuts/apps/common_test.go | 69 ++++++++++++++++++++++++++++++++--- 1 file changed, 63 insertions(+), 6 deletions(-) diff --git a/shortcuts/apps/common_test.go b/shortcuts/apps/common_test.go index b767c8b5dc..8d964ef7f1 100644 --- a/shortcuts/apps/common_test.go +++ b/shortcuts/apps/common_test.go @@ -95,27 +95,72 @@ func TestWithAppsHint(t *testing.T) { // The server has renumbered this case before (500002759 → 400002465). Both the // legacy code and an unrecognized future code must still enter the recovery // flow, so detection is code-OR-message rather than a single literal. + // assertClassificationIntact checks the discriminators the error envelope keys + // on. Rewriting Message/Hint must never change how the failure is classified, + // and the helper must hand back the same error value so the cause chain + // survives — a replacement error would otherwise pass a Message-only check. + assertClassificationIntact := func(t *testing.T, label string, in error, out error, wantSubtype errs.Subtype, wantCode int) { + t.Helper() + if out != in { + t.Fatalf("%s: returned a different error value: got %p, want original %p", label, out, in) + } + p, ok := errs.ProblemOf(out) + if !ok { + t.Fatalf("%s: returned error is not typed: %T", label, out) + } + if p.Category != errs.CategoryAPI { + t.Errorf("%s: Category = %q, want %q", label, p.Category, errs.CategoryAPI) + } + if p.Subtype != wantSubtype { + t.Errorf("%s: Subtype = %q, want %q", label, p.Subtype, wantSubtype) + } + if p.Code != wantCode { + t.Errorf("%s: Code = %d, want %d", label, p.Code, wantCode) + } + } + t.Run("legacy no-database code still enters the recovery flow", func(t *testing.T) { - in := errs.NewAPIError(errs.SubtypeUnknown, "workspace has no db branch"). + in := errs.NewAPIError(errs.SubtypeNotFound, "workspace has no db branch"). WithCode(appNoDatabaseLegacyCode) - p, _ := errs.ProblemOf(withAppsHint(in, "generic db hint")) + out := withAppsHint(in, "generic db hint") + p, _ := errs.ProblemOf(out) if p.Message != appNoDatabaseMessage || p.Hint != appNoDatabaseHint { t.Errorf("legacy code not detected: Message=%q Hint=%q", p.Message, p.Hint) } + assertClassificationIntact(t, "legacy code", in, out, errs.SubtypeNotFound, appNoDatabaseLegacyCode) }) t.Run("unknown code is detected by server message", func(t *testing.T) { // Codes the CLI has never seen: only the raw message identifies the case. + // A concrete subtype (not Unknown) proves classification survives the rewrite. for _, msg := range []string{ "get workspace id failed by app id", "Get Workspace Id Failed By App Id", // case-insensitive "workspace ws_x has no db branch", // substring, not whole-string } { - in := errs.NewAPIError(errs.SubtypeUnknown, msg).WithCode(999999999) - p, _ := errs.ProblemOf(withAppsHint(in, "generic db hint")) + in := errs.NewAPIError(errs.SubtypeNotFound, msg).WithCode(999999999) + out := withAppsHint(in, "generic db hint") + p, _ := errs.ProblemOf(out) if p.Message != appNoDatabaseMessage || p.Hint != appNoDatabaseHint { t.Errorf("message %q not detected: Message=%q Hint=%q", msg, p.Message, p.Hint) } + assertClassificationIntact(t, "message "+msg, in, out, errs.SubtypeNotFound, 999999999) + } + }) + + t.Run("no-database rewrite preserves the wrapped cause", func(t *testing.T) { + // The recovery flow replaces Message wholesale; if it ever swapped the error + // value instead of mutating in place, callers would silently lose errors.Is. + cause := errors.New("upstream transport failure") + in := errs.NewAPIError(errs.SubtypeNotFound, "workspace has no db branch"). + WithCode(appNoDatabaseCode).WithCause(cause) + out := withAppsHint(in, "generic db hint") + if !errors.Is(out, cause) { + t.Errorf("cause chain lost: errors.Is(out, cause) = false") + } + p, _ := errs.ProblemOf(out) + if p.Message != appNoDatabaseMessage { + t.Errorf("Message = %q, want %q", p.Message, appNoDatabaseMessage) } }) @@ -123,14 +168,16 @@ func TestWithAppsHint(t *testing.T) { // Guards against an over-broad matcher hijacking neighbouring db errors: // "invalid db branch" (env-pull's dev-branch case) must NOT be swallowed. for _, msg := range []string{"invalid db branch: dev", "数据表格不存在", "permission denied"} { - in := errs.NewAPIError(errs.SubtypeUnknown, msg).WithCode(400002469) - p, _ := errs.ProblemOf(withAppsHint(in, "generic db hint")) + in := errs.NewAPIError(errs.SubtypeNotFound, msg).WithCode(400002469) + out := withAppsHint(in, "generic db hint") + p, _ := errs.ProblemOf(out) if p.Message != msg { t.Errorf("message %q was rewritten to %q; matcher is too broad", msg, p.Message) } if p.Hint != "generic db hint" { t.Errorf("message %q got hint %q, want the caller hint", msg, p.Hint) } + assertClassificationIntact(t, "unrelated "+msg, in, out, errs.SubtypeNotFound, 400002469) } }) @@ -148,3 +195,13 @@ func TestWithAppsHint(t *testing.T) { } }) } + +// TestIsAppNoDatabaseError_NilProblem covers the defensive nil guard, which +// withAppsHint itself cannot reach (ProblemOf returns ok=false for an untyped +// error, so the predicate is never called with nil from there). The guard exists +// because the predicate is package-level and a future caller could pass nil. +func TestIsAppNoDatabaseError_NilProblem(t *testing.T) { + if isAppNoDatabaseError(nil) { + t.Error("isAppNoDatabaseError(nil) = true, want false") + } +} From 329b571ec0ee9ae08b1c7cb8270370c16f951a1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=99=88=E5=85=B4=E7=82=80?= Date: Fri, 7 Aug 2026 12:01:50 +0800 Subject: [PATCH 3/3] fix(errclass): classify the db-domain business codes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three codes reaching the Apps db commands were absent from the Spark table, so BuildAPIError fell through to the CategoryAPI + SubtypeUnknown catch-all and the envelope carried no usable classification. "App has no database yet" registers as Validation / FailedPrecondition: the app resolves fine and the request is well-formed, but a prerequisite the caller must create first is missing, so retrying unchanged can never succeed. This moves its exit code from 1 to 2 — "fix the state" rather than "the call failed" — and a test pins that so a future reclassification has to be deliberate. Two codes cover it because the server renumbered the case into the 4xx band; the legacy one stays for older servers. "Table does not exist" registers as API / NotFound, an ordinary missing-resource lookup with no exit-code change. SubtypeNotFound has no APIHint default, which matters here: the Apps layer fills its command-scoped hint only when the classifier left Hint empty, so a context-free default would displace the more actionable one. A test guards that too. --- internal/errclass/codemeta_spark.go | 4 +++ internal/errclass/codemeta_spark_test.go | 37 ++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/internal/errclass/codemeta_spark.go b/internal/errclass/codemeta_spark.go index ff8556fb1f..ce6a24ed98 100644 --- a/internal/errclass/codemeta_spark.go +++ b/internal/errclass/codemeta_spark.go @@ -23,6 +23,10 @@ var sparkCodeMeta = map[int]CodeMeta{ 3344039: {Category: errs.CategoryAPI, Subtype: errs.SubtypeInvalidParameters}, // invalid role description 3344040: {Category: errs.CategoryAPI, Subtype: errs.SubtypeInvalidParameters}, // unsupported member type 3344041: {Category: errs.CategoryAPI, Subtype: errs.SubtypeInvalidParameters}, // invalid member ID + + 400002465: {Category: errs.CategoryValidation, Subtype: errs.SubtypeFailedPrecondition}, // app has no database yet + 500002759: {Category: errs.CategoryValidation, Subtype: errs.SubtypeFailedPrecondition}, // app has no database yet, pre-4xx renumber + 400002469: {Category: errs.CategoryAPI, Subtype: errs.SubtypeNotFound}, // table does not exist } func init() { mergeCodeMeta(sparkCodeMeta, "spark") } diff --git a/internal/errclass/codemeta_spark_test.go b/internal/errclass/codemeta_spark_test.go index ab583aeb5d..343ad056d7 100644 --- a/internal/errclass/codemeta_spark_test.go +++ b/internal/errclass/codemeta_spark_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/output" ) func TestLookupCodeMetaSparkRoleCodes(t *testing.T) { @@ -30,6 +31,9 @@ func TestLookupCodeMetaSparkRoleCodes(t *testing.T) { {3344039, errs.CategoryAPI, errs.SubtypeInvalidParameters}, {3344040, errs.CategoryAPI, errs.SubtypeInvalidParameters}, {3344041, errs.CategoryAPI, errs.SubtypeInvalidParameters}, + {400002465, errs.CategoryValidation, errs.SubtypeFailedPrecondition}, + {500002759, errs.CategoryValidation, errs.SubtypeFailedPrecondition}, + {400002469, errs.CategoryAPI, errs.SubtypeNotFound}, } for _, tt := range tests { @@ -57,3 +61,36 @@ func TestLookupCodeMetaSparkRoleCodes(t *testing.T) { }) } } + +// TestSparkNoDatabaseCodesExitCode pins the exit code these codes route to; +// classifying them as Validation moves it from 1 to 2. +func TestSparkNoDatabaseCodesExitCode(t *testing.T) { + for _, code := range []int{400002465, 500002759} { + err := BuildAPIError(map[string]any{ + "code": code, + "msg": "get workspace id failed by app id", + }, ClassifyContext{Identity: "user"}) + if got := output.ExitCodeOf(err); got != 2 { + t.Errorf("code %d exit = %d, want 2 (validation: fix state, do not retry)", code, got) + } + } +} + +// TestSparkTableNotFoundLeavesHintToCaller keeps Hint empty here: the Apps layer +// only fills its command-scoped hint when the classifier left one empty. +func TestSparkTableNotFoundLeavesHintToCaller(t *testing.T) { + err := BuildAPIError(map[string]any{ + "code": 400002469, + "msg": "数据表格不存在", + }, ClassifyContext{Identity: "user"}) + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("BuildAPIError = %#v, want typed problem", err) + } + if p.Hint != "" { + t.Errorf("Hint = %q, want empty so the command-scoped hint still applies", p.Hint) + } + if got := output.ExitCodeOf(err); got != 1 { + t.Errorf("exit = %d, want 1 (unchanged: an ordinary API lookup failure)", got) + } +}