diff --git a/shortcuts/slides/slides_add_slide.go b/shortcuts/slides/slides_add_slide.go index d20ade334c..99b05c4786 100644 --- a/shortcuts/slides/slides_add_slide.go +++ b/shortcuts/slides/slides_add_slide.go @@ -53,6 +53,7 @@ var SlidesAddSlide = common.Shortcut{ {Name: "slide", Desc: "one complete XML document", Required: true, Input: []string{common.File, common.Stdin}}, {Name: "before-slide-id", Desc: "insert before this slide_id (default: append after the last page)"}, {Name: "revision-id", Type: "int", Default: "-1", Desc: "presentation revision (-1 = latest; pass a specific number for optimistic locking)"}, + noLintFlag(), }, Tips: []string{ " placeholders resolve against the current directory, not the directory of the --slide file, and are deduplicated per call: a page-by-page loop re-uploads a shared image once per page, so upload it once with slides +media-upload and reuse the file_token instead.", @@ -139,7 +140,7 @@ var SlidesAddSlide = common.Shortcut{ )). Desc(fmt.Sprintf("[%d/%d] Add page%s", step, total, descSuffix)). Params(addSlideQuery(runtime)). - Body(addSlideBody(slideXML, runtime.Str("before-slide-id"))) + Body(addSlideBody(slideXML, runtime.Str("before-slide-id"), runtime)) return dry.Set("images_to_upload", len(placeholders)) }, @@ -181,7 +182,7 @@ var SlidesAddSlide = common.Shortcut{ validate.EncodePathSegment(presentationID), ), addSlideQuery(runtime), - addSlideBody(slideXML, beforeSlideID), + addSlideBody(slideXML, beforeSlideID, runtime), ) if err != nil { if len(placeholders) > 0 { @@ -189,7 +190,10 @@ var SlidesAddSlide = common.Shortcut{ // a retry silently uploads a second copy of every file. err = appendSlidesProgressHint(err, fmt.Sprintf("%d image(s) were uploaded before the page failed; re-running will upload them again", len(placeholders))) } - return enrichSlidesReplaceError(err) + // Lint first: it names the actual finding, and enrichSlidesReplaceError + // only fills an empty hint, so its generic checklist stays out of the + // way when the backend already said what was wrong. + return enrichSlidesReplaceError(enrichSlidesLintError(err)) } slideID := common.GetString(data, "slide_id") @@ -234,12 +238,12 @@ func addSlideQuery(runtime *common.RuntimeContext) map[string]interface{} { // addSlideBody builds the request body shared by dry-run and execute. // before_slide_id is omitted when empty: the backend appends to the end only // if the key is absent, and an empty string is rejected as an unknown slide. -func addSlideBody(slideXML, beforeSlideID string) map[string]interface{} { +func addSlideBody(slideXML, beforeSlideID string, runtime *common.RuntimeContext) map[string]interface{} { body := map[string]interface{}{ "slide": map[string]interface{}{"content": slideXML}, } if id := strings.TrimSpace(beforeSlideID); id != "" { body["before_slide_id"] = id } - return body + return withLintXML(body, runtime) } diff --git a/shortcuts/slides/slides_create.go b/shortcuts/slides/slides_create.go index 56b5684676..3486e43a13 100644 --- a/shortcuts/slides/slides_create.go +++ b/shortcuts/slides/slides_create.go @@ -23,6 +23,12 @@ const ( ) // SlidesCreate creates a new Lark Slides presentation with bot auto-grant. +// +// The presentation is created first as an empty shell, then the pages are added +// one at a time. Each page is linted on its own way in, so a bad page is refused +// with the findings for that page — and the run stops there, leaving the +// presentation and the pages added before it. The error says so, so the caller +// knows what exists and where the run stopped. var SlidesCreate = common.Shortcut{ Service: "slides", Command: "+create", @@ -44,6 +50,7 @@ var SlidesCreate = common.Shortcut{ // by the framework, so it is not spelled out in Desc. {Name: "slides", Desc: "slide content JSON array (each element is a XML string, max 10; for more pages, create first then add them one at a time with slides +add-slide). placeholders are auto-uploaded and replaced with file_token.", Input: []string{common.File, common.Stdin}}, {Name: "slide", Type: "string_array", Desc: "one complete XML document, or @path to read one from a file; repeat once per page (max 10) and the CLI assembles the array for you, so no JSON escaping is needed. placeholders are handled as with --slides. Mutually exclusive with --slides."}, + noLintFlag(), }, Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { slides, param, err := createSlideContents(runtime) @@ -66,16 +73,25 @@ var SlidesCreate = common.Shortcut{ createBody := map[string]interface{}{ "xml_presentation": map[string]interface{}{"content": buildPresentationXML(title)}, } + placeholders := extractImagePlaceholderPaths(slides) + + // The note belongs to the create step, which is what the grant follows. + // Adding it at the end instead would land on whichever step happened to + // be last and overwrite that step's own description. + botNote := "" + if runtime.IsBot() { + botNote = " After creation succeeds in bot mode, the CLI will also try to grant the current CLI user full_access on the new presentation." + } dry := common.NewDryRunAPI() if len(slides) == 0 { dry.Desc("Create empty presentation"). POST("/open-apis/slides_ai/v1/xml_presentations"). + Desc(strings.TrimSpace(botNote)). Body(createBody) } else { n := len(slides) - placeholders := extractImagePlaceholderPaths(slides) total := n + 1 + len(placeholders) descSuffix := "" @@ -84,7 +100,7 @@ var SlidesCreate = common.Shortcut{ } dry.Desc(fmt.Sprintf("Create presentation%s + add %d slide(s)", descSuffix, n)). POST("/open-apis/slides_ai/v1/xml_presentations"). - Desc(fmt.Sprintf("[1/%d] Create presentation", total)). + Desc(fmt.Sprintf("[1/%d] Create presentation.%s", total, botNote)). Body(createBody) // Upload steps come right after creation so they can use the new @@ -101,15 +117,11 @@ var SlidesCreate = common.Shortcut{ for i, slideXML := range slides { dry.POST("/open-apis/slides_ai/v1/xml_presentations//slide"). Desc(fmt.Sprintf("[%d/%d] Add slide %d%s", slideStepStart+i, total, i+1, slideDescSuffix)). - Body(map[string]interface{}{ - "slide": map[string]interface{}{"content": slideXML}, - }) + Params(createSlideQuery()). + Body(createSlideBody(slideXML, runtime)) } } - if runtime.IsBot() { - dry.Desc("After creation succeeds in bot mode, the CLI will also try to grant the current CLI user full_access on the new presentation.") - } return dry }, Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { @@ -121,8 +133,10 @@ var SlidesCreate = common.Shortcut{ if err != nil { return err } + placeholders := extractImagePlaceholderPaths(slides) - // Step 1: Create presentation + // Step 1: Create presentation. The shell carries no page, so there is + // nothing here for the server to lint and no lint switch to send. data, err := runtime.CallAPITyped( "POST", "/open-apis/slides_ai/v1/xml_presentations", @@ -158,7 +172,6 @@ var SlidesCreate = common.Shortcut{ // Step 1.5: Upload any @path placeholders, then rewrite slide XML // with the resulting file_tokens. Uploads run after creation so // they can use the new presentation_id as parent_node. - placeholders := extractImagePlaceholderPaths(slides) if len(placeholders) > 0 { tokens, uploaded, err := uploadSlidesPlaceholders(runtime, presentationID, placeholders, param) if err != nil { @@ -170,6 +183,9 @@ var SlidesCreate = common.Shortcut{ result["images_uploaded"] = uploaded } + // Each page is linted on its way in, so the first refusal stops the + // run with the pages before it already on the server — which is what + // the progress hint on the error spells out. slideURL := fmt.Sprintf( "/open-apis/slides_ai/v1/xml_presentations/%s/slide", validate.EncodePathSegment(presentationID), @@ -181,13 +197,11 @@ var SlidesCreate = common.Shortcut{ slideData, err := runtime.CallAPITyped( "POST", slideURL, - map[string]interface{}{"revision_id": -1}, - map[string]interface{}{ - "slide": map[string]interface{}{"content": slideXML}, - }, + createSlideQuery(), + createSlideBody(slideXML, runtime), ) if err != nil { - return appendSlidesProgressHint(err, fmt.Sprintf("adding slide %d/%d failed; presentation %s was created, %d slide(s) added before failure", i+1, len(slides), presentationID, i)) + return appendSlidesProgressHint(enrichSlidesLintError(err), fmt.Sprintf("adding slide %d/%d failed; presentation %s was created, %d slide(s) added before failure", i+1, len(slides), presentationID, i)) } sid := common.GetString(slideData, "slide_id") if sid != "" { @@ -352,6 +366,26 @@ func effectiveTitle(title string) string { return title } +// createSlideQuery builds the query for the per-page calls +create makes after +// the presentation exists. revision_id is pinned to -1 (latest) rather than +// exposed: the deck was created by this same command a moment ago, so there is +// no earlier revision a caller could sensibly target. +func createSlideQuery() map[string]interface{} { + return map[string]interface{}{"revision_id": -1} +} + +// createSlideBody builds the per-page body shared by dry-run and execute, so +// the two cannot drift on the lint switch the way two literals would. +// +// The presentation-create call has no body of its own to stamp: it sends the +// title-only shell from buildPresentationXML, and there is no +// page in it for the server to lint. +func createSlideBody(slideXML string, runtime *common.RuntimeContext) map[string]interface{} { + return withLintXML(map[string]interface{}{ + "slide": map[string]interface{}{"content": slideXML}, + }, runtime) +} + // buildPresentationXML builds the minimal XML for a new empty presentation. func buildPresentationXML(title string) string { escapedTitle := xmlEscape(title) diff --git a/shortcuts/slides/slides_create_test.go b/shortcuts/slides/slides_create_test.go index 87249740a2..d676dfb727 100644 --- a/shortcuts/slides/slides_create_test.go +++ b/shortcuts/slides/slides_create_test.go @@ -298,42 +298,7 @@ func TestSlidesCreateWithSlides(t *testing.T) { t.Parallel() f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) - reg.Register(&httpmock.Stub{ - Method: "POST", - URL: "/open-apis/slides_ai/v1/xml_presentations", - Body: map[string]interface{}{ - "code": 0, - "msg": "ok", - "data": map[string]interface{}{ - "xml_presentation_id": "pres_with_slides", - "revision_id": 1, - }, - }, - }) - reg.Register(&httpmock.Stub{ - Method: "POST", - URL: "/open-apis/slides_ai/v1/xml_presentations/pres_with_slides/slide", - Body: map[string]interface{}{ - "code": 0, - "msg": "ok", - "data": map[string]interface{}{ - "slide_id": "slide_001", - "revision_id": 2, - }, - }, - }) - reg.Register(&httpmock.Stub{ - Method: "POST", - URL: "/open-apis/slides_ai/v1/xml_presentations/pres_with_slides/slide", - Body: map[string]interface{}{ - "code": 0, - "msg": "ok", - "data": map[string]interface{}{ - "slide_id": "slide_002", - "revision_id": 3, - }, - }, - }) + createStubPresentation(t, reg, "pres_with_slides", 2) slidesJSON := `["",""]` err := runSlidesCreateShortcut(t, f, stdout, []string{ @@ -354,14 +319,18 @@ func TestSlidesCreateWithSlides(t *testing.T) { if !ok || len(slideIDs) != 2 { t.Fatalf("slide_ids = %v, want 2 elements", data["slide_ids"]) } - if slideIDs[0] != "slide_001" || slideIDs[1] != "slide_002" { - t.Fatalf("slide_ids = %v, want [slide_001, slide_002]", slideIDs) + if slideIDs[0] != "s_1" || slideIDs[1] != "s_2" { + t.Fatalf("slide_ids = %v, want [s_1, s_2]", slideIDs) } if data["slides_added"] != float64(2) { t.Fatalf("slides_added = %v, want 2", data["slides_added"]) } } +// TestSlidesCreatePreservesSchemaIssues keeps the advisories from every call +// that produced them. Each page is judged by its own call, so the per-page +// findings are collected under slide_issues with the page they belong to, and +// the presentation-level ones stay separate. func TestSlidesCreatePreservesSchemaIssues(t *testing.T) { t.Parallel() @@ -412,6 +381,70 @@ func TestSlidesCreatePreservesSchemaIssues(t *testing.T) { } } +// TestSlidesCreateRefusedPageSaysWhatLanded is what a lint refusal mid-deck has +// to tell the caller. The pages go in one at a time, so page 2 is refused with +// page 1 already on the server and a presentation that exists — the run cannot +// undo that, so the error has to name the presentation and say how far it got, +// or the caller retries into a duplicate deck. +func TestSlidesCreateRefusedPageSaysWhatLanded(t *testing.T) { + t.Parallel() + + f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/slides_ai/v1/xml_presentations", + Body: map[string]interface{}{ + "code": 0, + "data": map[string]interface{}{"xml_presentation_id": "pres_refused", "revision_id": 1}, + }, + }) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/slides_ai/v1/xml_presentations/pres_refused/slide", + Body: map[string]interface{}{"code": 0, "data": map[string]interface{}{"slide_id": "s_1", "revision_id": 2}}, + }) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/slides_ai/v1/xml_presentations/pres_refused/slide", + Body: map[string]interface{}{"code": 4000153, "msg": lintBlockMessage}, + }) + + page1 := `` + page2 := `` + err := runSlidesCreateShortcut(t, f, stdout, []string{ + "+create", + "--title", "Partial", + "--slide", page1, + "--slide", page2, + "--as", "user", + }) + if err == nil { + t.Fatal("expected the refused page to surface, got nil") + } + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("expected a typed errs.* error, got %v", err) + } + if p.Category != errs.CategoryAPI { + t.Fatalf("category = %q, want %q", p.Category, errs.CategoryAPI) + } + // The lint report reaches the caller verbatim, so the findings are there to + // read and parse the same way `lark-cli api` would deliver them. + if p.Message != lintBlockMessage { + t.Fatalf("message = %q, want the lint report verbatim", p.Message) + } + // Both halves of the hint matter: how to fix the page, and what already + // exists so the retry adds the rest instead of starting over. + if !strings.Contains(p.Hint, lintRemediationHint) { + t.Fatalf("hint = %q, want the lint remediation wording", p.Hint) + } + for _, want := range []string{"pres_refused", "slide 2/2", "1 slide(s) added"} { + if !strings.Contains(p.Hint, want) { + t.Fatalf("hint lost %q, got: %s", want, p.Hint) + } + } +} + // TestSlidesCreateWithSlidesPartialFailure verifies error reporting when a slide fails to create. func TestSlidesCreateWithSlidesPartialFailure(t *testing.T) { t.Parallel() @@ -481,14 +514,10 @@ func TestSlidesCreateWithSlidesPartialFailure(t *testing.T) { // The presentation was created but a slide add failed; the recovery hint // carries the partial-progress context (which presentation exists, how many // slides landed) so the caller can resume without recreating. - if !strings.Contains(p.Hint, "pres_partial") { - t.Fatalf("hint should contain presentation ID, got: %s", p.Hint) - } - if !strings.Contains(p.Hint, "slide 2/2") { - t.Fatalf("hint should indicate slide 2/2 failed, got: %s", p.Hint) - } - if !strings.Contains(p.Hint, "1 slide(s) added") { - t.Fatalf("hint should report 1 slide added before failure, got: %s", p.Hint) + for _, want := range []string{"pres_partial", "slide 2/2", "1 slide(s) added"} { + if !strings.Contains(p.Hint, want) { + t.Fatalf("hint lost %q, got: %s", want, p.Hint) + } } } diff --git a/shortcuts/slides/slides_lint_error.go b/shortcuts/slides/slides_lint_error.go new file mode 100644 index 0000000000..bf53a0e645 --- /dev/null +++ b/shortcuts/slides/slides_lint_error.go @@ -0,0 +1,120 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package slides + +import ( + "encoding/json" + "fmt" + "strings" + + "github.com/larksuite/cli/errs" +) + +// lintBlockedCode is the code the slide engine answers a layout-lint refusal +// with. It is the engine's own, not a generic validation code, so it identifies +// the refusal on its own and nothing about the message has to be guessed at. +const lintBlockedCode = 4000153 + +// lintBlockDetail is the part of the lint report this file reads. The message is +// the report the lint tool produced, forwarded whole, and the report is large: +// a per-level split, a per-slide grouping, and a set of measurements that differ +// from rule to rule. None of that is decoded here because nothing here would do +// anything with it — it reaches the caller through the message itself, which is +// left verbatim. What is decoded is the two facts the hint is built from. +type lintBlockDetail struct { + Summary lintBlockSummary `json:"summary"` + // SchemaIssues is the schema-sanitize report, which the engine merges into + // the lint report as its own top-level field so the message stays parseable + // as a whole. + SchemaIssues string `json:"schema_issues"` +} + +// lintBlockSummary is the report's own verdict over the page. Only error_count +// is read: it is the count that refused the write, and it is the same line the +// lint script draws when it is run by hand — it exits non-zero on error_count +// and says nothing about warnings or infos. A report can therefore arrive with +// warnings in it and still be a refusal that only the errors caused, so +// counting findings instead would overstate what has to be fixed. +type lintBlockSummary struct { + ErrorCount int `json:"error_count"` +} + +// lintRemediationHint names the escape hatch. The backend cannot write this line +// itself: --no-lint is a CLI flag and the server has never heard of it. +// The opening says "the page" and not "nothing": on +create the refusal can +// arrive after the presentation and some of its pages already exist, and a +// blanket "nothing was written" would contradict the progress hint sitting next +// to it. One page is the unit the gate refuses on every path, so it is what this +// says. +const lintRemediationHint = "the page was not written. Fix the error-level findings in the message and retry;" + + " if the lint is wrong about a page that has to ship as-is, re-run with --no-lint" + +// enrichSlidesLintError adds what the CLI knows about a layout-lint refusal, and +// leaves every other error alone. +// +// The refusal arrives as the lint report in the message field and it stays there +// verbatim. The same refusal reaches callers two ways: through these shortcuts, +// and through `lark-cli api` calling the endpoint directly, where nothing +// rewrites it. Rendering the report to prose here would mean the same refusal +// has two different message formats depending on which command produced it, so a +// caller could not parse one field one way. Verbatim also matches +// ERROR_CONTRACT.md's "propagate typed errors unchanged": the server named the +// offending element, the page it sits on, and how to fix it. +// +// What the backend cannot say is added as a hint instead: how many findings +// refused the page, that the page did not land, and --no-lint, which is a CLI +// flag the server has never heard of. That is the one thing this layer knows and +// the report does not, which is the only reason this enrichment exists at all. +// +// Detection is by code. Every other error is returned untouched, so the helper is +// safe on write paths whose backend does not lint. +func enrichSlidesLintError(err error) error { + if err == nil { + return nil + } + p, ok := errs.ProblemOf(err) + if !ok || p.Code != lintBlockedCode { + return err + } + // Reuses the progress-hint helper for its append-preserving-classification + // behaviour, not for its orchestration meaning. + return appendSlidesProgressHint(err, lintBlockHint(parseLintBlockDetail(p.Message))) +} + +// parseLintBlockDetail reads the two fields the hint needs out of the report. A +// message that does not decode is not an error here: the code already said this +// is a refusal, and the hint is worth writing even without a count in it — the +// escape hatch is the half of it the caller cannot get anywhere else. +func parseLintBlockDetail(message string) lintBlockDetail { + var detail lintBlockDetail + if err := json.Unmarshal([]byte(strings.TrimSpace(message)), &detail); err != nil { + return lintBlockDetail{} + } + return detail +} + +// lintBlockHint summarises the refusal without restating it. The findings +// themselves stay in the message, so this says only what a caller needs before +// deciding whether to read them: how many refused the write, and what to do next. +// +// It deliberately does not name a page. Every write path submits one page, so +// the slide_number on each finding is its position inside that submission and is +// always 1 — which is not the page the caller is looking for. On +create it is +// actively wrong: the progress hint sitting next to this one says "adding slide +// 2/3 failed", and a "on slide 1" next to it sends the reader to the wrong page. +// The real page number has exactly one source, and it is that other line. +// +// The schema findings get their own note because they travel in their own field +// and are easy to miss inside a long report. +func lintBlockHint(detail lintBlockDetail) string { + head := "xml lint blocked" + if detail.Summary.ErrorCount > 0 { + head = fmt.Sprintf("%s: %d error(s)", head, detail.Summary.ErrorCount) + } + parts := []string{head} + if detail.SchemaIssues != "" { + parts = append(parts, "the message also carries schema_issues") + } + return strings.Join(parts, ", ") + ". " + lintRemediationHint +} diff --git a/shortcuts/slides/slides_lint_error_test.go b/shortcuts/slides/slides_lint_error_test.go new file mode 100644 index 0000000000..15ff7a4306 --- /dev/null +++ b/shortcuts/slides/slides_lint_error_test.go @@ -0,0 +1,301 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package slides + +import ( + "strings" + "testing" + + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/cmdutil" + "github.com/larksuite/cli/internal/httpmock" +) + +// lintBlockMessage is the report the backend forwards when the layout lint +// refuses a write — the same document the lint script writes when it is run by +// hand. Written out as literal JSON rather than marshalled from the parser's own +// structs so a change to the server's field names has to be noticed here instead +// of being absorbed silently by both sides at once. +// +// It carries a warning alongside the two errors, because that is the report a +// real refusal produces and it is what proves the hint counts the errors rather +// than the findings. The second error carries a measurement and a +// related_objects pair, which this parser has no field for and which differ from +// rule to rule — they are here because the verbatim assertion below is what +// proves the CLI does not have to model a finding in order to deliver one. +const lintBlockMessage = `{"schema_version":"2.0","tool":"xml_lint","file":null,` + + `"slide_size":{"width":960,"height":540},` + + `"summary":{"slide_count":1,"error_count":2,"warning_count":1,"info_count":0,` + + `"status":"blocked","release_ready":false,"screenshot_review_required":true},` + + `"document":{"errors":[],"warnings":[],"infos":[]},` + + `"slides":[{"slide_number":1,"status":"blocked","element_count":7,"errors":[` + + `{"level":"error","code":"text_overflow","slide_number":1,"path":"presentation/slide/data/shape[2]",` + + `"message":"text is 340pt tall in a 180pt shape","hint":"shorten the text or raise height"},` + + `{"level":"error","code":"bbox_overlap","slide_number":1,` + + `"message":"element ends at x=1080, slide is 960 wide",` + + `"measurement":{"intersection_area":94.848,"width":9.88,"height":9.6},` + + `"related_objects":[{"element_id":"bhT","xml_path":"slide[1]/data/shape[3]",` + + `"bbox":{"x":36,"y":36,"width":9.88,"height":9.6}}]}],` + + `"warnings":[{"level":"warning","code":"sparse_slide_content","slide_number":1,` + + `"message":"coverage 12.8% below 15.0%"}],"infos":[]}]}` + +// lintBlockedError builds the error the API layer produces for a refusal, which +// is a typed error carrying the engine's code and the report as its message. +func lintBlockedError(message string) error { + return errs.NewAPIError(errs.SubtypeInvalidParameters, "%s", message).WithCode(lintBlockedCode) +} + +// TestLintErrorKeepsTheReportVerbatimAndAddsTheEscapeHatch pins the division of +// labour: the finding is the server's to word and travels untouched, the flag is +// the CLI's to mention and travels in the hint. +// +// The message is asserted byte-for-byte because the same refusal also reaches +// callers through `lark-cli api`, where nothing rewrites it. Rendering it here +// would give one refusal two message formats depending on which command produced +// it, and a caller could not parse the field one way. +func TestLintErrorKeepsTheReportVerbatimAndAddsTheEscapeHatch(t *testing.T) { + t.Parallel() + + f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/slides_ai/v1/xml_presentations/pres_lint/slide", + Body: map[string]interface{}{"code": lintBlockedCode, "msg": lintBlockMessage}, + }) + + err := runSlidesShortcut(t, f, stdout, SlidesAddSlide, []string{ + "+add-slide", + "--presentation", "pres_lint", + "--slide", testPageXML, + "--as", "user", + }) + if err == nil { + t.Fatal("expected the lint block to surface as an error") + } + problem, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("err = %v, want typed problem metadata", err) + } + if problem.Category != errs.CategoryAPI { + t.Fatalf("Category = %q, want %q: the backend rejection must survive enrichment", + problem.Category, errs.CategoryAPI) + } + // Byte-for-byte, not "contains": every field of every finding reaches the + // caller in the shape the server chose, so the same parse works whether the + // refusal came through a shortcut or through `lark-cli api`. + if problem.Message != lintBlockMessage { + t.Fatalf("Message = %q, want the server document verbatim:\n%q", problem.Message, lintBlockMessage) + } + // The hint carries only what the document cannot say for itself. + for _, want := range []string{ + "xml lint blocked: 2 error(s)", + "--no-lint", + } { + if !strings.Contains(problem.Hint, want) { + t.Fatalf("Hint lost %q\ngot: %q", want, problem.Hint) + } + } + // The hint summarises, it does not restate: repeating the findings would put + // two copies of the same list in one error, and the copies drift. + if strings.Contains(problem.Hint, "text is 340pt tall") { + t.Fatalf("Hint = %q, want a summary rather than a second copy of the findings", problem.Hint) + } +} + +// TestLintErrorCountsOnlyTheErrorsThatRefusedThePage covers a report spread +// across levels. Only errors refuse a write — the same line the lint script +// draws when it is run by hand — so a report that also carries a warning and an +// info is still a one-error refusal, and saying "3" would send the caller +// hunting for two more blockers that are not there. +func TestLintErrorCountsOnlyTheErrorsThatRefusedThePage(t *testing.T) { + t.Parallel() + + msg := `{"schema_version":"2.0","tool":"xml_lint",` + + `"summary":{"slide_count":3,"error_count":1,"warning_count":1,"info_count":1,` + + `"status":"blocked","release_ready":false,"screenshot_review_required":true},` + + `"schema_issues":"dropped unknown attribute shadow on shape[1]",` + + `"slides":[{"slide_number":1,"status":"blocked","errors":[` + + `{"level":"error","code":"out_of_bounds","message":"element ends at x=1080, slide is 960 wide"}],` + + `"warnings":[{"level":"warning","code":"sparse_slide_content","message":"coverage 12.8% below 15.0%"}],` + + `"infos":[{"level":"info","code":"table_resolved_size_mismatch",` + + `"message":"table is 12pt narrower than declared"}]}]}` + + enriched := enrichSlidesLintError(lintBlockedError(msg)) + problem, ok := errs.ProblemOf(enriched) + if !ok { + t.Fatalf("err = %v, want typed problem metadata", enriched) + } + if problem.Message != msg { + t.Fatalf("Message = %q, want the server document verbatim", problem.Message) + } + for _, want := range []string{ + // One error refused the page; the warning and the info came along. + "xml lint blocked: 1 error(s)", + // schema_issues travels in its own field, so a caller reading the message + // top to bottom can miss it. The hint says it is there. + "the message also carries schema_issues", + } { + if !strings.Contains(problem.Hint, want) { + t.Fatalf("Hint lost %q\ngot: %q", want, problem.Hint) + } + } + if strings.Contains(problem.Hint, "3 ") { + t.Fatalf("Hint = %q, want the error count rather than the finding count", problem.Hint) + } +} + +// TestLintErrorLeavesOtherFailuresAlone guards the detection. The helper runs on +// every write path including ones the backend does not lint, so a false positive +// would staple "re-run with --no-lint" onto unrelated failures — advice that does +// nothing and sends the caller down the wrong path. +func TestLintErrorLeavesOtherFailuresAlone(t *testing.T) { + t.Parallel() + + f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/slides_ai/v1/xml_presentations/pres_other/slide", + Body: map[string]interface{}{"code": 3350001, "msg": "invalid slide xml"}, + }) + + err := runSlidesShortcut(t, f, stdout, SlidesAddSlide, []string{ + "+add-slide", + "--presentation", "pres_other", + "--slide", testPageXML, + "--as", "user", + }) + if err == nil { + t.Fatal("expected the backend rejection to surface") + } + problem, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("err = %v, want typed problem metadata", err) + } + if strings.Contains(problem.Hint, "--no-lint") { + t.Fatalf("Hint = %q, want no lint advice on a non-lint failure", problem.Hint) + } + // The 3350001 checklist still lands, so lint enrichment running first has not + // displaced the existing fallback. + if !strings.Contains(problem.Hint, "block_id not found in current slide") { + t.Fatalf("Hint = %q, want the existing 3350001 checklist", problem.Hint) + } +} + +// TestLintErrorClaimsOnlyItsOwnCode is the other half of the detection. The +// backend reports other failures through the same field as JSON, and one of them +// could one day look like a lint report; the code is what says this refusal came +// from the lint gate, so a message alone must not be enough to claim it. +func TestLintErrorClaimsOnlyItsOwnCode(t *testing.T) { + t.Parallel() + + for name, msg := range map[string]string{ + "a report under a foreign code": lintBlockMessage, + "node server shape": `{"code":"SCHEMA_INVALID","errors":[{"line":3,"detail":"unexpected element"}]}`, + "not json": "invalid param", + } { + t.Run(name, func(t *testing.T) { + original := errs.NewAPIError(errs.SubtypeInvalidParameters, "%s", msg).WithCode(4001000) + enriched := enrichSlidesLintError(original) + problem, ok := errs.ProblemOf(enriched) + if !ok { + t.Fatalf("err = %v, want typed problem metadata", enriched) + } + if problem.Message != msg { + t.Fatalf("Message = %q, want it passed through unchanged", problem.Message) + } + if strings.Contains(problem.Hint, "--no-lint") { + t.Fatalf("Hint = %q, want no lint advice on a message this helper does not own", problem.Hint) + } + }) + } +} + +// TestLintErrorStillHelpsWhenTheReportCannotBeRead covers the code arriving with +// a message this parser cannot decode. The count is the half of the hint that +// needs the report; the escape hatch is the half the caller cannot get anywhere +// else, so dropping the whole hint over a missing number would withhold the only +// thing this layer knows. +func TestLintErrorStillHelpsWhenTheReportCannotBeRead(t *testing.T) { + t.Parallel() + + enriched := enrichSlidesLintError(lintBlockedError("xml lint blocked")) + problem, ok := errs.ProblemOf(enriched) + if !ok { + t.Fatalf("err = %v, want typed problem metadata", enriched) + } + if !strings.Contains(problem.Hint, "--no-lint") { + t.Fatalf("Hint = %q, want the escape hatch even without a readable report", problem.Hint) + } + if strings.Contains(problem.Hint, "error(s)") { + t.Fatalf("Hint = %q, want no invented count when the report did not parse", problem.Hint) + } +} + +// TestLintErrorSurvivesTheImageProgressHint covers the two enrichers landing on +// the same error. Both write to Hint, and a caller that uploaded images needs +// both facts: the pages were rejected, and the uploads already happened. +func TestLintErrorSurvivesTheImageProgressHint(t *testing.T) { + t.Parallel() + + err := lintBlockedError(lintBlockMessage) + enriched := appendSlidesProgressHint(enrichSlidesLintError(err), "2 image(s) were uploaded before the page failed") + + problem, ok := errs.ProblemOf(enriched) + if !ok { + t.Fatalf("err = %v, want typed problem metadata", enriched) + } + if !strings.Contains(problem.Hint, "--no-lint") { + t.Fatalf("Hint = %q, want the lint escape hatch", problem.Hint) + } + if !strings.Contains(problem.Hint, "2 image(s) were uploaded") { + t.Fatalf("Hint = %q, want the upload progress note", problem.Hint) + } +} + +// TestLintErrorScopesTheRefusalToThePage pins the wording against the case that +// exposed it. On +create the refusal can arrive after the deck and some of its +// pages are already on the server, and the orchestration hint says exactly that +// — so a lint hint claiming nothing was written puts two contradictory +// statements in the same message and the caller cannot tell which is true. +func TestLintErrorScopesTheRefusalToThePage(t *testing.T) { + t.Parallel() + + err := lintBlockedError(lintBlockMessage) + // The order +create produces: the lint enricher first, then the progress + // note about what already landed. + enriched := appendSlidesProgressHint(enrichSlidesLintError(err), + "adding slide 2/3 failed; presentation pres_abc was created, 1 slide(s) added before failure") + + problem, ok := errs.ProblemOf(enriched) + if !ok { + t.Fatalf("err = %v, want typed problem metadata", enriched) + } + if strings.Contains(problem.Hint, "nothing was written") { + t.Fatalf("Hint = %q, want no blanket claim that nothing was written: the same hint reports a created presentation and added slides", problem.Hint) + } + if !strings.Contains(problem.Hint, "the page was not written") { + t.Fatalf("Hint = %q, want the refusal scoped to the page the gate rejected", problem.Hint) + } + if !strings.Contains(problem.Hint, "was created") { + t.Fatalf("Hint = %q, want the orchestration progress preserved alongside it", problem.Hint) + } + // The findings in lintBlockMessage carry slide_number 1, because slide_number + // counts inside the submission and every write path submits one page. Naming + // it here would put "on slide 1" next to "adding slide 2/3 failed" and send + // the caller to a page that linted clean. The page number has one source, and + // it is the progress note. + if strings.Contains(problem.Hint, "on slide") { + t.Fatalf("Hint = %q, want no page named by the lint half: slide_number is the position inside the submitted page, not the deck", problem.Hint) + } +} + +// TestLintErrorPassesNilThrough keeps the helper safe to fold into a return +// expression on a path that may not have failed. +func TestLintErrorPassesNilThrough(t *testing.T) { + t.Parallel() + + if got := enrichSlidesLintError(nil); got != nil { + t.Fatalf("enrichSlidesLintError(nil) = %v, want nil", got) + } +} diff --git a/shortcuts/slides/slides_lint_param.go b/shortcuts/slides/slides_lint_param.go new file mode 100644 index 0000000000..54d9313b9a --- /dev/null +++ b/shortcuts/slides/slides_lint_param.go @@ -0,0 +1,66 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package slides + +import "github.com/larksuite/cli/shortcuts/common" + +// The server-side XML lint switch, carried by every shortcut that writes slide +// content: +create, +add-slide, +update-slide and +replace-slide. +// +// The backend lints the page the write produces and refuses it when that page +// fails. Note "produces", not "is handed": +replace-slide submits fragments, and +// the subject of the lint is the assembled page, so a fragment that is valid on +// its own is still refused when it pushes a neighbour off the canvas. The CLI +// asks for the check on every call, so a caller who never touches the flag gets +// the checked path; --no-lint is the escape hatch for the case the lint is wrong +// about a page that must still ship. +// +// Sent explicitly in both directions rather than omitted when on: the parameter +// is newer than internal/registry/meta_data.json, so the server-side default is +// not something this CLI can read anywhere, and a request that states the value +// means the same thing before and after that default ever changes. +const noLintFlagName = "no-lint" + +// lintXMLBodyKey is the wire name the switch binds to, and it travels in the +// request body rather than the query string. +// +// That is not a style choice. A query parameter has to be declared in the +// gateway's own api meta before it is bound to a field, and the published +// definition of these endpoints lists only xml_presentation_id, revision_id +// and idempotency_key. Until that publish happens the gateway drops an +// undeclared parameter on the floor: the request succeeds, the field arrives +// unset, and the server reads it as "lint not requested". Verified against a +// live backend — pages that asked to be linted were written unlinted, with no +// error anywhere to say so. Body fields ride along with the JSON already being +// sent and need no separate registration. +// +// snake_case matches every other body key these endpoints take: slide, +// before_slide_id, parts, comment. +// +// Worth knowing when this is next touched: a wrong name here still fails +// silently, for the same reason it did in the query. Tests and --dry-run only +// prove what the CLI sent, not what was read. +const lintXMLBodyKey = "lint_xml" + +// noLintFlag is the shared flag definition, so the three commands cannot drift +// apart on the name or the wording. +func noLintFlag() common.Flag { + return common.Flag{ + Name: noLintFlagName, + Type: "bool", + Desc: "submit the XML without the server-side lint; by default the server lints every page and rejects the write when it fails", + } +} + +// withLintXML stamps the switch onto a request body and returns it, so the body +// builders stay one-liners and dry-run and execute cannot disagree about the +// value. A nil map is allocated rather than rejected, so a caller with nothing +// else to send still gets a well-formed body. +func withLintXML(body map[string]interface{}, runtime *common.RuntimeContext) map[string]interface{} { + if body == nil { + body = map[string]interface{}{} + } + body[lintXMLBodyKey] = !runtime.Bool(noLintFlagName) + return body +} diff --git a/shortcuts/slides/slides_lint_param_test.go b/shortcuts/slides/slides_lint_param_test.go new file mode 100644 index 0000000000..c2ddc5daf2 --- /dev/null +++ b/shortcuts/slides/slides_lint_param_test.go @@ -0,0 +1,425 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package slides + +import ( + "encoding/json" + "io" + "net/http" + "net/url" + "strconv" + "strings" + "testing" + + "github.com/larksuite/cli/internal/cmdutil" + "github.com/larksuite/cli/internal/httpmock" + "github.com/larksuite/cli/shortcuts/common" +) + +// The lint switch is asserted on the wire rather than on the body-builder +// return value. What matters is what the backend receives, and only a real +// request proves the switch reached it: a builder test would still pass if a +// command stopped calling its own builder. +// +// Where it sits on the wire is asserted too, not just its value. The switch +// used to ride the query string, where the gateway dropped it silently because +// its api meta never declared the parameter — the request succeeded and the +// page went in unlinted. A test that only read the value would have been just +// as green then as it is now. + +// captureLintBody records the decoded request body of a stub. +func captureLintBody(t *testing.T, into *map[string]interface{}) func(*http.Request) { + t.Helper() + return func(req *http.Request) { + raw, err := io.ReadAll(req.Body) + if err != nil { + t.Errorf("read request body: %v", err) + return + } + var decoded map[string]interface{} + if err := json.Unmarshal(raw, &decoded); err != nil { + t.Errorf("decode request body %q: %v", raw, err) + return + } + *into = decoded + } +} + +// assertLintSwitch checks the switch is in the body, carries the expected +// value, and left the query alone. +func assertLintSwitch(t *testing.T, body map[string]interface{}, query url.Values, want bool) { + t.Helper() + got, ok := body[lintXMLBodyKey] + if !ok { + t.Fatalf("body = %v, want a %s key", body, lintXMLBodyKey) + } + if got != want { + t.Fatalf("body[%s] = %v, want %v", lintXMLBodyKey, got, want) + } + if q := query.Get(lintXMLBodyKey); q != "" { + t.Fatalf("query carried %s = %q; the gateway does not bind it there, so a value in the query is a silent no-op", + lintXMLBodyKey, q) + } +} + +// TestAddSlideLintXMLTravels pins both directions on +add-slide. +func TestAddSlideLintXMLTravels(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + extra []string + want bool + }{ + {name: "default lints", want: true}, + {name: "--no-lint disables", extra: []string{"--no-lint"}, want: false}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + var body map[string]interface{} + var query url.Values + capture := captureLintBody(t, &body) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/slides_ai/v1/xml_presentations/pres_abc/slide", + Body: map[string]interface{}{"code": 0, "data": map[string]interface{}{"slide_id": "slide_001"}}, + OnMatch: func(req *http.Request) { + query = req.URL.Query() + capture(req) + }, + }) + + args := append([]string{ + "+add-slide", + "--presentation", "pres_abc", + "--slide", testPageXML, + "--as", "user", + }, tc.extra...) + if err := runSlidesShortcut(t, f, stdout, SlidesAddSlide, args); err != nil { + t.Fatalf("unexpected error: %v", err) + } + assertLintSwitch(t, body, query, tc.want) + // The switch is added to the body, not substituted for it. + if _, ok := body["slide"]; !ok { + t.Fatalf("body = %v, want the slide payload alongside the switch", body) + } + }) + } +} + +// TestUpdateSlideLintXMLTravels pins both directions on +update-slide, and that +// the switch does not displace the parts payload. +func TestUpdateSlideLintXMLTravels(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + extra []string + want bool + }{ + {name: "default lints", want: true}, + {name: "--no-lint disables", extra: []string{"--no-lint"}, want: false}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + var body map[string]interface{} + var query url.Values + capture := captureLintBody(t, &body) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/slides_ai/v1/xml_presentations/pres_abc/slide/replace", + Body: map[string]interface{}{"code": 0, "data": map[string]interface{}{"revision_id": 9}}, + OnMatch: func(req *http.Request) { + query = req.URL.Query() + capture(req) + }, + }) + + args := append([]string{ + "+update-slide", + "--presentation", "pres_abc", + "--slide-id", "bUn", + "--content", testPageXML, + "--as", "user", + }, tc.extra...) + if err := runSlidesShortcut(t, f, stdout, SlidesUpdateSlide, args); err != nil { + t.Fatalf("unexpected error: %v", err) + } + assertLintSwitch(t, body, query, tc.want) + if _, ok := body["parts"]; !ok { + t.Fatalf("body = %v, want the parts payload alongside the switch", body) + } + // slide_id genuinely is a query parameter here, and the gateway does + // declare it. Moving the switch must not have disturbed it. + if got := query.Get("slide_id"); got != "bUn" { + t.Fatalf("slide_id = %q, want bUn", got) + } + }) + } +} + +// TestCreateLintXMLTravels pins both directions on +create. +// +// +create makes the presentation first and then sends the pages one at a time, +// so the switch has to reach every page call — a deck where only the first page +// is linted is the failure this guards against. The create call itself sends the +// title-only shell, which has no page in it to lint, so it carries no switch at +// all. +func TestCreateLintXMLTravels(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + extra []string + want bool + }{ + {name: "default lints", want: true}, + {name: "--no-lint disables", extra: []string{"--no-lint"}, want: false}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + var createBody map[string]interface{} + type slideCall struct { + body map[string]interface{} + query url.Values + } + var slideCalls []slideCall + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/slides_ai/v1/xml_presentations", + Body: map[string]interface{}{"code": 0, "data": map[string]interface{}{ + "xml_presentation_id": "pres_lint", + "revision_id": 1, + }}, + OnMatch: captureLintBody(t, &createBody), + }) + for i := 0; i < 2; i++ { + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/slides_ai/v1/xml_presentations/pres_lint/slide", + Body: map[string]interface{}{"code": 0, "data": map[string]interface{}{ + "slide_id": "s_1", + "revision_id": i + 2, + }}, + OnMatch: func(req *http.Request) { + call := slideCall{query: req.URL.Query()} + captureLintBody(t, &call.body)(req) + slideCalls = append(slideCalls, call) + }, + }) + } + + args := append([]string{ + "+create", + "--title", "Lint", + "--slide", testPageXML, + "--slide", testPageXML, + "--as", "user", + }, tc.extra...) + if err := runSlidesCreateShortcut(t, f, stdout, args); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // The shell holds the title and nothing else, so there is nothing for a + // switch on it to decide. + presentation, _ := createBody["xml_presentation"].(map[string]interface{}) + content, _ := presentation["content"].(string) + if strings.Contains(content, ""}]`, + "--as", "user", + }, tc.extra...) + if err := runSlidesShortcut(t, f, stdout, SlidesReplaceSlide, args); err != nil { + t.Fatalf("unexpected error: %v", err) + } + assertLintSwitch(t, body, query, tc.want) + if _, ok := body["parts"]; !ok { + t.Fatalf("body = %v, want the parts payload alongside the switch", body) + } + }) + } +} + +// TestReplacePagesLintXMLTravels pins both directions on +replace-pages. +// +// This path deletes the old page once the new one lands, so an unlinted create +// here is the only write that can destroy existing content on the strength of a +// page nobody checked. The switch has to reach every item, not just the first, +// for the same reason as on +create. +func TestReplacePagesLintXMLTravels(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + extra []string + want bool + }{ + {name: "default lints", want: true}, + {name: "--no-lint disables", extra: []string{"--no-lint"}, want: false}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + type slideCall struct { + body map[string]interface{} + query url.Values + } + var createCalls []slideCall + for i := 0; i < 2; i++ { + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/slides_ai/v1/xml_presentations/pres_abc/slide", + Body: map[string]interface{}{"code": 0, "data": map[string]interface{}{ + "slide_id": "new_1", + "revision_id": i*2 + 11, + }}, + OnMatch: func(req *http.Request) { + call := slideCall{query: req.URL.Query()} + captureLintBody(t, &call.body)(req) + createCalls = append(createCalls, call) + }, + }) + reg.Register(&httpmock.Stub{ + Method: "DELETE", + URL: "/open-apis/slides_ai/v1/xml_presentations/pres_abc/slide", + Body: map[string]interface{}{"code": 0, "data": map[string]interface{}{ + "revision_id": i*2 + 12, + }}, + }) + } + + pages := `[{"slide_id":"old1","content":` + strconv.Quote(testPageXML) + + `},{"slide_id":"old2","content":` + strconv.Quote(testPageXML) + `}]` + args := append([]string{ + "+replace-pages", + "--presentation", "pres_abc", + "--pages", pages, + "--as", "user", + }, tc.extra...) + if err := runSlidesShortcut(t, f, stdout, SlidesReplacePages, args); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(createCalls) != 2 { + t.Fatalf("create calls = %d, want 2", len(createCalls)) + } + for _, call := range createCalls { + assertLintSwitch(t, call.body, call.query, tc.want) + // The switch is added to the body, not substituted for it. + if _, ok := call.body["before_slide_id"]; !ok { + t.Fatalf("body = %v, want before_slide_id alongside the switch", call.body) + } + } + }) + } +} + +// TestLintXMLFlagIsOnEverySlideContentWriter guards the set. A new writer that +// forgets the flag ships pages the backend never checked, and the omission is +// invisible until something renders wrong. +func TestLintXMLFlagIsOnEverySlideContentWriter(t *testing.T) { + t.Parallel() + + for _, sc := range []struct { + name string + want bool + have []string + }{ + {name: "+create", want: true, have: lintFlagNames(SlidesCreate.Flags)}, + {name: "+add-slide", want: true, have: lintFlagNames(SlidesAddSlide.Flags)}, + {name: "+update-slide", want: true, have: lintFlagNames(SlidesUpdateSlide.Flags)}, + // +replace-slide sends fragments rather than a whole , which is + // exactly why it needs the flag: the lint's subject is the page they + // assemble into, so this is the path where a fragment that is correct + // on its own can still break the page. + {name: "+replace-slide", want: true, have: lintFlagNames(SlidesReplaceSlide.Flags)}, + // +replace-pages deletes the old page once the new one lands, so it is + // the one writer where skipping the lint can cost content that already + // existed. + {name: "+replace-pages", want: true, have: lintFlagNames(SlidesReplacePages.Flags)}, + } { + if got := flagListHas(sc.have, noLintFlagName); got != sc.want { + t.Fatalf("%s has --%s = %v, want %v", sc.name, noLintFlagName, got, sc.want) + } + } +} + +func lintFlagNames(flags []common.Flag) []string { + names := make([]string, 0, len(flags)) + for _, f := range flags { + names = append(names, f.Name) + } + return names +} + +func flagListHas(haystack []string, needle string) bool { + for _, s := range haystack { + if s == needle { + return true + } + } + return false +} diff --git a/shortcuts/slides/slides_replace_pages.go b/shortcuts/slides/slides_replace_pages.go index 39e9d53b01..1688fc38ba 100644 --- a/shortcuts/slides/slides_replace_pages.go +++ b/shortcuts/slides/slides_replace_pages.go @@ -33,6 +33,7 @@ var SlidesReplacePages = common.Shortcut{ {Name: "pages", Desc: "JSON array of page replacements (each: {slide_id, content}); supports @file or -", Required: true, Input: []string{common.File, common.Stdin}}, {Name: "continue-on-error", Type: "bool", Desc: "continue with later pages after a create/delete failure; default false"}, {Name: "validate-only", Type: "bool", Desc: "validate input and build the create/delete plan without write calls"}, + noLintFlag(), }, Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { ref, err := parsePresentationRef(runtime.Str("presentation")) @@ -56,7 +57,7 @@ var SlidesReplacePages = common.Shortcut{ if err != nil { return dry.Set("error", err.Error()) } - appendReplacePagesDryRunCalls(dry, resolved) + appendReplacePagesDryRunCalls(dry, resolved, runtime) return dry. Set("xml_presentation_id", resolved.PresentationID). Set("pages_count", len(resolved.Plan)). @@ -83,6 +84,17 @@ var SlidesReplacePages = common.Shortcut{ results := make([]replacePageResult, 0, len(resolved.Plan)) for i, item := range resolved.Plan { result, err := replaceOnePage(runtime, resolved.PresentationID, item, revisionID) + if err != nil { + // The lint refusal is enriched here for the same reason as on every + // other write path: --no-lint is a CLI flag the backend cannot name. + // It happens before the --continue-on-error branch below because a + // refused page needs the same recovery information either way; the + // batch that keeps going only ever sees the per-item record, so + // enriching inside the returning branch would have hidden the flag + // from the callers most likely to have several pages to fix. + err = enrichSlidesLintError(err) + recordReplacePageError(&result, err) + } results = append(results, result) if result.RevisionID != nil { revisionID = *result.RevisionID @@ -91,6 +103,11 @@ var SlidesReplacePages = common.Shortcut{ if runtime.Bool("continue-on-error") { continue } + // The progress hint wrapping the error carries the page, which the + // lint hint deliberately does not — inside one item the report + // always says slide 1. It is appended only on the returning path: + // the per-item records already say which items ran and how each + // ended, so the batch that keeps going has no use for a position. return appendSlidesProgressHint(err, fmt.Sprintf("slides +replace-pages stopped at item %d/%d; %d page(s) completed before failure; old page is kept when create failed", i+1, len(resolved.Plan), countReplacedPages(results))) } } @@ -136,10 +153,27 @@ type replacePageResult struct { NewSlideID string Status string Error string + // ErrorCode and ErrorHint carry the typed metadata that Error alone drops: + // Problem.Error() renders the message and nothing else, so an item recorded + // from the string would arrive without the code that identifies it or the + // hint that says how to recover from it. + ErrorCode int + ErrorHint string Issues interface{} RevisionID *int } +// recordReplacePageError is the one place a failed item is written down, so the +// stop-on-error path and the --continue-on-error path cannot record a failure +// differently. Untyped errors keep only their message, which is all they have. +func recordReplacePageError(result *replacePageResult, err error) { + result.Error = err.Error() + if p, ok := errs.ProblemOf(err); ok { + result.ErrorCode = p.Code + result.ErrorHint = p.Hint + } +} + func prepareReplacePages(runtime *common.RuntimeContext) (*replacePagesPrepared, error) { ref, err := parsePresentationRef(runtime.Str("presentation")) if err != nil { @@ -240,16 +274,13 @@ func buildReplacePagesPlan(pages []replacePageInput) ([]replacePagePlanItem, err return plan, nil } -func appendReplacePagesDryRunCalls(dry *common.DryRunAPI, resolved *replacePagesPrepared) { +func appendReplacePagesDryRunCalls(dry *common.DryRunAPI, resolved *replacePagesPrepared, runtime *common.RuntimeContext) { dry.Desc("Batch replace pages in-place: create each new page before old page, then delete old page (not atomic)") for i, item := range resolved.Plan { dry.POST(fmt.Sprintf("/open-apis/slides_ai/v1/xml_presentations/%s/slide", validate.EncodePathSegment(resolved.PresentationID))). Desc(fmt.Sprintf("[%d/%d] Create replacement before old slide %s", i*2+1, len(resolved.Plan)*2, item.OldSlideID)). Params(map[string]interface{}{"revision_id": ""}). - Body(map[string]interface{}{ - "slide": map[string]interface{}{"content": item.Content}, - "before_slide_id": item.OldSlideID, - }) + Body(replacePageCreateBody(item, runtime)) dry.DELETE(fmt.Sprintf("/open-apis/slides_ai/v1/xml_presentations/%s/slide", validate.EncodePathSegment(resolved.PresentationID))). Desc(fmt.Sprintf("[%d/%d] Delete old slide %s after create succeeds", i*2+2, len(resolved.Plan)*2, item.OldSlideID)). Params(map[string]interface{}{ @@ -259,6 +290,20 @@ func appendReplacePagesDryRunCalls(dry *common.DryRunAPI, resolved *replacePages } } +// replacePageCreateBody builds the create request body shared by dry-run and +// execute, so the previewed request and the sent one cannot drift apart on +// whether the page gets linted. +func replacePageCreateBody(item replacePagePlanItem, runtime *common.RuntimeContext) map[string]interface{} { + return withLintXML(map[string]interface{}{ + "slide": map[string]interface{}{"content": item.Content}, + "before_slide_id": item.OldSlideID, + }, runtime) +} + +// replaceOnePage reports how far the item got through Status and returns the +// failure itself untouched. Recording it belongs to the caller, which is where +// the error is enriched — writing the message here as well would have produced +// a per-item record built from a different error than the one the caller sees. func replaceOnePage(runtime *common.RuntimeContext, presentationID string, item replacePagePlanItem, revisionID int) (replacePageResult, error) { result := replacePageResult{ OldSlideID: item.OldSlideID, @@ -269,21 +314,16 @@ func replaceOnePage(runtime *common.RuntimeContext, presentationID string, item "POST", slideURL, map[string]interface{}{"revision_id": revisionID}, - map[string]interface{}{ - "slide": map[string]interface{}{"content": item.Content}, - "before_slide_id": item.OldSlideID, - }, + replacePageCreateBody(item, runtime), ) if err != nil { result.Status = "create_failed" - result.Error = err.Error() return result, err } newSlideID := common.GetString(createData, "slide_id") if newSlideID == "" { err := errs.NewInternalError(errs.SubtypeInvalidResponse, "slide.create returned no slide_id for replacement of slide_id %q", item.OldSlideID) result.Status = "create_failed" - result.Error = err.Error() return result, err } result.NewSlideID = newSlideID @@ -306,7 +346,6 @@ func replaceOnePage(runtime *common.RuntimeContext, presentationID string, item ) if err != nil { result.Status = "delete_failed" - result.Error = err.Error() return result, err } if rev, ok := revisionFromData(deleteData); ok { @@ -342,6 +381,12 @@ func replacePageResultsOutput(results []replacePageResult) []map[string]interfac if result.Error != "" { m["error"] = result.Error } + if result.ErrorCode != 0 { + m["error_code"] = result.ErrorCode + } + if result.ErrorHint != "" { + m["hint"] = result.ErrorHint + } if result.Issues != nil { m["issues"] = result.Issues } diff --git a/shortcuts/slides/slides_replace_pages_test.go b/shortcuts/slides/slides_replace_pages_test.go index c7322aedd9..ffa0443311 100644 --- a/shortcuts/slides/slides_replace_pages_test.go +++ b/shortcuts/slides/slides_replace_pages_test.go @@ -200,6 +200,100 @@ func TestReplacePagesContinueOnErrorReturnsPartialFailure(t *testing.T) { } } +// TestReplacePagesContinueOnErrorKeepsLintRecoveryPerItem pins the one thing +// --continue-on-error changes about a refusal: nothing. A batch that keeps going +// reports its failures only through the per-item records, so if the enrichment +// lived on the returning path the caller with the most pages to fix would be the +// one told least about how to fix them. +// +// The report travels verbatim in the item for the same reason it does in the +// error: it is the server's document and one parse has to work on it wherever it +// surfaced. The code and the hint travel beside it because Problem.Error() is +// the message alone, so an item built from the string would name neither the +// refusal nor the flag that gets past it. +func TestReplacePagesContinueOnErrorKeepsLintRecoveryPerItem(t *testing.T) { + t.Parallel() + + f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/slides_ai/v1/xml_presentations/pres_abc/slide", + Body: map[string]interface{}{"code": lintBlockedCode, "msg": lintBlockMessage}, + }) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/slides_ai/v1/xml_presentations/pres_abc/slide", + Body: map[string]interface{}{ + "code": 0, + "data": map[string]interface{}{"slide_id": "new2", "revision_id": 11}, + }, + }) + reg.Register(&httpmock.Stub{ + Method: "DELETE", + URL: "/open-apis/slides_ai/v1/xml_presentations/pres_abc/slide", + Body: map[string]interface{}{ + "code": 0, + "data": map[string]interface{}{"revision_id": 12}, + }, + }) + + pages := `[ + {"slide_id":"old1","content":""}, + {"slide_id":"old2","content":""} + ]` + err := runSlidesShortcut(t, f, stdout, SlidesReplacePages, []string{ + "+replace-pages", + "--presentation", "pres_abc", + "--pages", pages, + "--continue-on-error", + "--as", "user", + }) + var pfErr *output.PartialFailureError + if !errors.As(err, &pfErr) { + t.Fatalf("err = %T %v, want *output.PartialFailureError", err, err) + } + + env := decodeReplacePagesEnvelope(t, stdout) + results, _ := env.Data["results"].([]interface{}) + if len(results) != 2 { + t.Fatalf("results len = %d, want 2", len(results)) + } + refused, _ := results[0].(map[string]interface{}) + if refused["status"] != "create_failed" { + t.Fatalf("refused item status = %v, want create_failed", refused["status"]) + } + if refused["error_code"] != float64(lintBlockedCode) { + t.Fatalf("refused item error_code = %v, want %d: the item cannot be recognised as a lint refusal without it", + refused["error_code"], lintBlockedCode) + } + if refused["error"] != lintBlockMessage { + t.Fatalf("refused item error = %v, want the server report verbatim", refused["error"]) + } + hint, _ := refused["hint"].(string) + for _, want := range []string{"xml lint blocked: 2 error(s)", "--no-lint"} { + if !strings.Contains(hint, want) { + t.Fatalf("refused item hint lost %q\ngot: %q", want, hint) + } + } + // The position belongs to the stop-on-error path. Here every item has its own + // record, so a "stopped at item 1/2" would describe a batch that did not stop. + if strings.Contains(hint, "stopped at item") { + t.Fatalf("refused item hint = %q, want no stop-on-error progress line", hint) + } + // A refusal is not a partial write: the page the batch moved on from must not + // look like it left a new page behind. + if _, ok := refused["new_slide_id"]; ok { + t.Fatalf("refused item = %#v, want no new_slide_id", refused) + } + survivor, _ := results[1].(map[string]interface{}) + if survivor["status"] != "replaced" || survivor["new_slide_id"] != "new2" { + t.Fatalf("second result = %#v, want replaced with new2", survivor) + } + if _, ok := survivor["error_code"]; ok { + t.Fatalf("second result = %#v, want no error_code on a page that landed", survivor) + } +} + func TestReplacePagesContinueOnErrorDeleteFailureIncludesNewSlideID(t *testing.T) { t.Parallel() diff --git a/shortcuts/slides/slides_replace_slide.go b/shortcuts/slides/slides_replace_slide.go index 1b98a0fe82..7d90db745b 100644 --- a/shortcuts/slides/slides_replace_slide.go +++ b/shortcuts/slides/slides_replace_slide.go @@ -34,6 +34,8 @@ const maxReplaceParts = 200 // it triggers 3350001. // 4. On 3350001 errors it enriches the hint with context-specific guidance // so AI agents can self-correct. +// 5. It asks the backend to lint the page these parts produce, and renders the +// refusal when the lint blocks the write. --no-lint opts out. // // `str_replace` is intentionally NOT exposed: product direction is that // slide edits go through structural (block-level) operations only. The backend @@ -53,6 +55,7 @@ var SlidesReplaceSlide = common.Shortcut{ {Name: "parts", Desc: "JSON array of replace parts; accepts replace/insert action aliases, target_id for block_id, and block/content/shape/element for the action's XML payload; max 200", Required: true, Input: []string{common.File, common.Stdin}}, {Name: "revision-id", Type: "int", Default: "-1", Desc: "presentation revision (-1 = latest; pass a specific number for optimistic locking)"}, {Name: "tid", Desc: "transaction id for concurrent-edit locking (usually empty)"}, + noLintFlag(), }, Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { ref, err := parsePresentationRef(runtime.Str("presentation")) @@ -103,7 +106,7 @@ var SlidesReplaceSlide = common.Shortcut{ if tid := runtime.Str("tid"); tid != "" { query["tid"] = tid } - body := map[string]interface{}{"parts": injected} + body := replaceSlideBody(injected, runtime) dry := common.NewDryRunAPI() presentationID := ref.Token @@ -155,11 +158,14 @@ var SlidesReplaceSlide = common.Shortcut{ if tid := strings.TrimSpace(runtime.Str("tid")); tid != "" { query["tid"] = tid } - body := map[string]interface{}{"parts": injected} + body := replaceSlideBody(injected, runtime) data, err := runtime.CallAPITyped("POST", slideReplaceAPIPath(presentationID), query, body) if err != nil { - return enrichSlidesReplaceError(err) + // Lint first: enrichSlidesReplaceError only fills an empty hint, so + // running it second leaves the specific lint finding in place and + // the generic 3350001 checklist for everything else. + return enrichSlidesReplaceError(enrichSlidesLintError(err)) } result := map[string]interface{}{ @@ -183,12 +189,27 @@ var SlidesReplaceSlide = common.Shortcut{ if raw, ok := data["failed_reason"]; ok { result["failed_reason"] = raw } + // issues points the other way from failed_reason: the parts were applied + // and committed, and the backend still had something to say about the page + // they produced. A finding serious enough to refuse the write leaves as an + // error carrying the same report, so whatever arrives here describes a page + // that is already stored. + if raw, ok := data["issues"]; ok { + result["issues"] = raw + } runtime.Out(result, nil) return nil }, } +// replaceSlideBody builds the request body shared by dry-run and execute, so the +// two cannot disagree about the lint switch — the failure mode being a --dry-run +// that shows a linted request and an execute that sends an unlinted one. +func replaceSlideBody(parts []map[string]interface{}, runtime *common.RuntimeContext) map[string]interface{} { + return withLintXML(map[string]interface{}{"parts": parts}, runtime) +} + // replacePart is the normalized (post-JSON) representation of one entry in the // parts array. Fields are nullable so we can tell "not provided" from "empty". type replacePart struct { diff --git a/shortcuts/slides/slides_replace_slide_test.go b/shortcuts/slides/slides_replace_slide_test.go index 23a06993df..5cee37a9e3 100644 --- a/shortcuts/slides/slides_replace_slide_test.go +++ b/shortcuts/slides/slides_replace_slide_test.go @@ -614,6 +614,47 @@ func TestReplaceSlidePassThroughFailureFields(t *testing.T) { } } +// TestReplaceSlidePassesThroughIssues covers the field pointing the other way +// from failed_reason: the parts were applied and committed, and the backend +// still had something to say about the page they produced. A finding serious +// enough to refuse the write leaves as an error carrying the same report, so +// what arrives here always describes a page that is already stored. +func TestReplaceSlidePassesThroughIssues(t *testing.T) { + t.Parallel() + + f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/slide/replace", + Body: map[string]interface{}{ + "code": 0, + "data": map[string]interface{}{ + "revision_id": 4, + "issues": "[issue=text_overflows_container id=bgD level=warning]", + }, + }, + }) + + parts := `[{"action":"block_replace","block_id":"bxx","replacement":""}]` + err := runSlidesShortcut(t, f, stdout, SlidesReplaceSlide, []string{ + "+replace-slide", + "--presentation", "pres_abc", + "--slide-id", "s", + "--parts", parts, + "--as", "user", + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + data := decodeShortcutData(t, stdout) + if issues, _ := data["issues"].(string); issues != "[issue=text_overflows_container id=bgD level=warning]" { + t.Fatalf("issues = %#v, want the backend report passed through verbatim", data["issues"]) + } + if _, ok := data["failed_reason"]; ok { + t.Fatalf("failed_reason should stay absent on a write that succeeded: %#v", data) + } +} + func TestReplaceSlide3350001ErrorEnrichment(t *testing.T) { t.Parallel() diff --git a/shortcuts/slides/slides_update_slide.go b/shortcuts/slides/slides_update_slide.go index 9ee1304573..34e98abf59 100644 --- a/shortcuts/slides/slides_update_slide.go +++ b/shortcuts/slides/slides_update_slide.go @@ -99,6 +99,7 @@ var updateSlideFlags = []common.Flag{ {Name: "content", Aliases: contentFlagAliases, Desc: "the page's full target XML, one root; elements omitted here are removed from the page", Required: true, Input: []string{common.File, common.Stdin}}, {Name: "revision-id", Type: "int", Default: "-1", Desc: "revision to apply against; -1 (default) means latest. Pinning an older revision rebuilds the page from that snapshot and discards newer edits to it"}, {Name: "tid", Desc: "transaction id for concurrent-edit locking (usually empty)"}, + noLintFlag(), } func updateSlideValidate(_ context.Context, runtime *common.RuntimeContext) error { @@ -187,7 +188,7 @@ func updateSlideDryRun(_ context.Context, runtime *common.RuntimeContext) *commo dry.POST(slideReplaceAPIPath(presentationID)). Desc(fmt.Sprintf("[%d/%d] Replace slide%s", step, total, descSuffix)). Params(updateSlideQuery(runtime, slideID)). - Body(map[string]interface{}{"parts": updateSlideParts(slideID, content)}) + Body(updateSlideBody(slideID, content, runtime)) return dry.Set("slide_id", slideID). Set("content_bytes", len(content)). Set("images_to_upload", len(placeholders)) @@ -233,14 +234,17 @@ func updateSlideExecute(_ context.Context, runtime *common.RuntimeContext) error data, err := runtime.CallAPITyped("POST", slideReplaceAPIPath(presentationID), updateSlideQuery(runtime, slideID), - map[string]interface{}{"parts": updateSlideParts(slideID, content)}) + updateSlideBody(slideID, content, runtime)) if err != nil { if len(placeholders) > 0 { // The images are already in the deck's media store; say so, or a // retry silently uploads a second copy of every file. err = appendSlidesProgressHint(err, fmt.Sprintf("%d image(s) were uploaded before the slide failed; re-running will upload them again", len(placeholders))) } - return enrichUpdateSlideError(err) + // .../slide/replace is gated, and the subject is the page the write + // produces rather than the payload sent, so a page that is invalid only + // in combination is caught here too. + return enrichUpdateSlideError(enrichSlidesLintError(err)) } // A single part carries the whole page, so any failed_reason means the page @@ -263,6 +267,14 @@ func updateSlideExecute(_ context.Context, runtime *common.RuntimeContext) error if v, ok := data["revision_id"]; ok { result["revision_id"] = v } + // issues is advisory and only ever arrives on a page that was written: a + // finding serious enough to refuse the write leaves as an error carrying the + // same report, and the failed_reason branch above has already returned. Passed + // through untouched, as +add-slide and +create do, so a caller reads one field + // however it wrote the page. + if issues, ok := data["issues"]; ok { + result["issues"] = issues + } runtime.Out(result, nil) return nil } @@ -316,6 +328,12 @@ func updateSlideQuery(runtime *common.RuntimeContext, slideID string) map[string return query } +// updateSlideBody builds the request body shared by dry-run and execute, so the +// two cannot disagree about the lint switch. +func updateSlideBody(slideID, content string, runtime *common.RuntimeContext) map[string]interface{} { + return withLintXML(map[string]interface{}{"parts": updateSlideParts(slideID, content)}, runtime) +} + // updateSlideParts builds the one part the command ever sends. block_id is the // page id, which is what makes the backend swap the whole out. func updateSlideParts(slideID, content string) []map[string]interface{} { diff --git a/shortcuts/slides/slides_update_slide_test.go b/shortcuts/slides/slides_update_slide_test.go index 66b84c5a6b..a63193c130 100644 --- a/shortcuts/slides/slides_update_slide_test.go +++ b/shortcuts/slides/slides_update_slide_test.go @@ -419,6 +419,71 @@ func TestUpdateSlideRevisionAndTIDTravel(t *testing.T) { } } +// TestUpdateSlidePassesThroughIssues keeps the backend's lint report visible on +// a page that was written. A finding serious enough to refuse the write comes +// back as an error carrying the same report, so anything arriving here describes +// a page that is already stored — dropping it would leave the caller believing +// the deck says exactly what they wrote. +func TestUpdateSlidePassesThroughIssues(t *testing.T) { + t.Parallel() + + f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/slides_ai/v1/xml_presentations/pres_abc/slide/replace", + Body: map[string]interface{}{"code": 0, "data": map[string]interface{}{ + "revision_id": 9, + "issues": "[issue=text_overflows_container id=v5 level=warning]", + }}, + }) + + if err := runSlidesShortcut(t, f, stdout, SlidesUpdateSlide, []string{ + "+update-slide", + "--presentation", "pres_abc", + "--slide-id", "pYw", + "--content", testPageXML, + "--as", "user", + }); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + data := decodeShortcutData(t, stdout) + if issues, _ := data["issues"].(string); issues != "[issue=text_overflows_container id=v5 level=warning]" { + t.Fatalf("issues = %#v, want the backend report passed through verbatim", data["issues"]) + } + if data["revision_id"] != float64(9) { + t.Fatalf("revision_id = %#v, want 9 alongside the report", data["revision_id"]) + } +} + +// TestUpdateSlideOmitsIssuesWhenAbsent keeps the key out of the output rather +// than reporting an empty report, so a caller can test for presence. +func TestUpdateSlideOmitsIssuesWhenAbsent(t *testing.T) { + t.Parallel() + + f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/slides_ai/v1/xml_presentations/pres_abc/slide/replace", + Body: map[string]interface{}{"code": 0, "data": map[string]interface{}{"revision_id": 9}}, + }) + + if err := runSlidesShortcut(t, f, stdout, SlidesUpdateSlide, []string{ + "+update-slide", + "--presentation", "pres_abc", + "--slide-id", "pYw", + "--content", testPageXML, + "--as", "user", + }); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + data := decodeShortcutData(t, stdout) + if _, ok := data["issues"]; ok { + t.Fatalf("issues should be omitted when the backend sent none: %#v", data) + } +} + // TestUpdateSlideOmitsEmptyTID keeps an unset --tid out of the query rather than // sending tid="". func TestUpdateSlideOmitsEmptyTID(t *testing.T) { diff --git a/skills/lark-slides/references/cli/lark-slides-add-slide.md b/skills/lark-slides/references/cli/lark-slides-add-slide.md index 3d94542823..c81d1abdec 100644 --- a/skills/lark-slides/references/cli/lark-slides-add-slide.md +++ b/skills/lark-slides/references/cli/lark-slides-add-slide.md @@ -45,6 +45,7 @@ lark-cli slides +add-slide --presentation "$PRES_ID" --slide @page3.xml --dry-ru | `--slide` | 是 | 一个完整的 `...` 文档；支持字面量、`@file`、stdin `-` | | `--before-slide-id` | 否 | 插到该 `slide_id` 之前；**不传就是追加到末尾** | | `--revision-id` | 否 | 演示文稿版本号，默认 `-1`（最新）；传具体版本号做乐观锁 | +| `--no-lint` | 否 | 跳过服务端版式校验（默认开启）；仅在确认校验误判、该页必须原样发布时使用 | | `--dry-run` | 否 | 打印将要发起的请求（含图片上传步骤），不写入 | `@file` 路径**必须在 CWD 内**（如 `@./plan/page3.xml`）；绝对路径和 `../` 会被拒绝并报 `unsafe file path`。 @@ -73,14 +74,14 @@ lark-cli slides +add-slide --as user \ "revision_id": 42, "before_slide_id": "slide_example_target_id", "images_uploaded": 1, - "issues": "[issue=unsupported_attr tag= attr=style]" + "issues": "<服务端返回的发现>" } ``` | 字段 | 说明 | |------|------| | `slide_id` | 新创建页面的唯一标识 | -| `issues` | 字符串，**只在服务端丢弃过内容时才出现**：页面创建成功，但括号里列出的标签/属性没写进去。出现就必须 `+screenshot` 复核，别当纯警告忽略；干净提交时这个字段不返回 | +| `issues` | 字符串，仅在**页面已写入成功**且服务端有发现时返回，干净提交时不返回，不影响本次调用的成功状态。两种来源：提交的 XML 里有服务端不支持的标签/属性被丢弃（**页面内容与提交的不一致**），或未达阻断级的版式校验发现。格式不固定，不要解析；出现就用 `+screenshot` 复核该页，不要按普通告警忽略 | ## 常见错误 @@ -89,4 +90,5 @@ lark-cli slides +add-slide --as user \ | `--slide is not a single complete document` | 传了 `` 整份 XML，或多个 `` 拼在一起 | 一次只传一页，根元素必须是 `` | | `--slide cannot be empty` | `@file` 指向空文件，或 stdin 没内容 | 检查文件内容 | | 3350001 | XML 结构/转义有问题；**或 `--before-slide-id` 不是有效 `slide_id`** | 优先改用 `--slide @file` 绕开 shell 转义；插页失败先 `+xml-get` 回读确认 `slide_id`；再按 [workflow/error-handling.md](../workflow/error-handling.md) 排查 | +| 4000153 `xml lint blocked` | 服务端版式校验拒绝了这一页，页面未写入 | `error.message` 是完整的校验报告，按其中每条发现给出的修改建议修正后重试 | | 1061004 / 403 | 当前身份对这份 PPT 没有编辑权限 | 检查是否拥有 `slides:presentation:update` 或 `slides:presentation:write_only` scope；wiki 链接另需 `wiki:node:read`，`@` 占位符另需 `docs:document.media:upload`；`--as bot` 还要求该 bot 对目标 PPT 有编辑权限 | diff --git a/skills/lark-slides/references/cli/lark-slides-create.md b/skills/lark-slides/references/cli/lark-slides-create.md index a2c51a1fe4..50e8816206 100644 --- a/skills/lark-slides/references/cli/lark-slides-create.md +++ b/skills/lark-slides/references/cli/lark-slides-create.md @@ -54,11 +54,12 @@ lark-cli slides +create --title "项目汇报" --slide @./slide-01.xml --dry-run - **`slides_added`**（integer，可选）：带页面创建时返回，成功添加的页面数量 - **`images_uploaded`**（integer，可选）：页面 XML 中含 `@<本地路径>` 占位符时返回，已上传的去重后图片数量 - **`permission_grant`**（object，可选）：仅 `--as bot` 时返回，说明是否已自动为当前 CLI 用户授予可管理权限 +- **`slide_issues`**（数组，可选）：带页面创建时才可能返回，逐项对应一个**已写入成功的页面**（标明页序和 `slide_id`），内容是服务端对该页的发现，不影响本次调用的成功状态。两种来源：页面 XML 里有服务端不支持的标签/属性被丢弃（**页面内容与提交的不一致**），或未达阻断级的版式校验发现。格式不固定，不要解析；出现就用 `+screenshot` 复核对应页 > [!IMPORTANT] > 不带页面参数时，`slides +create` 只创建空白演示文稿。创建后用 [`+add-slide`](lark-slides-add-slide.md) 逐页添加 slide 内容。 > -> 带了页面时，CLI 先创建空白演示文稿，再逐页调用 slide 创建接口添加页面。如果某一页添加失败，CLI 会停止并报错，已创建的演示文稿和已添加的页面会保留。 +> 带了页面时，CLI 先创建空白演示文稿，再逐页调用 slide 创建接口添加页面，每页各过一次服务端版式校验。如果某一页失败，CLI 会停止并报错，已创建的演示文稿和已添加的页面会保留，报错会指明失败页序和此前已成功写入的页数。 > > 如果演示文稿是**以应用身份（bot）创建**的，如 `lark-cli slides +create --as bot`，CLI 会**尝试为当前 CLI 用户自动授予该演示文稿的 `full_access`（可管理权限）**。 > @@ -76,6 +77,7 @@ lark-cli slides +create --title "项目汇报" --slide @./slide-01.xml --dry-run | `--title` | 否 | 演示文稿标题（不传则默认 "Untitled"） | | `--slide` | 否 | 一页 `` XML，或 `@路径`；可重复，最多 10 次。格式见[页面输入形式](#页面输入形式) | | `--slides` | 否 | 页面 XML 的 JSON 字符串数组，最多 10 个；支持 `@文件` 和 `-`（stdin）。格式见[页面输入形式](#页面输入形式) | +| `--no-lint` | 否 | 跳过服务端版式校验（默认开启，每页各校验一次）；仅在确认校验误判、页面必须原样发布时使用 | 10 页是 CLI 的上限，服务端每次只接收一页。超过 10 页时先用 `+create` 创建空白 PPT，再用 [`+add-slide`](lark-slides-add-slide.md) 逐页添加。 @@ -169,6 +171,7 @@ lark-cli slides +add-slide --as user \ |--------|------|----------| | 400 | 参数错误 | 检查参数格式是否正确 | | 403 | 权限不足 | 检查是否拥有 `slides:presentation:create` 和 `slides:presentation:write_only` scope | +| 4000153 `xml lint blocked` | 服务端版式校验拒绝了该页；演示文稿及其之前的页面已写入成功 | `error.message` 是完整的校验报告，按其中每条发现给出的修改建议修正后，用 `+add-slide` 从该页续接，无需重建整份演示文稿 | ## 相关命令 diff --git a/skills/lark-slides/references/cli/lark-slides-replace-slide.md b/skills/lark-slides/references/cli/lark-slides-replace-slide.md index 08dae90306..10982689a8 100644 --- a/skills/lark-slides/references/cli/lark-slides-replace-slide.md +++ b/skills/lark-slides/references/cli/lark-slides-replace-slide.md @@ -51,6 +51,7 @@ lark-cli slides +replace-slide --as user \ | `--parts` | 是 | JSON 数组（`[{...}, ...]`），单次最多 200 条。支持 `@` 和 `-`（stdin）读取 | | `--revision-id` | 否 | 基础版本号；默认 `-1` 表示基于最新版执行；传具体版本号时，服务端以该版本为 base 执行；**传不存在的版本号（超过当前 revision）返回 3350002** | | `--tid` | 否 | 并发事务 ID；多人协作长事务才用，单次单人调用留空 | +| `--no-lint` | 否 | 跳过服务端版式校验（默认开启）。**校验主体是这些 parts 拼装后的整页**，不是片段本身 | ## parts 元素结构 @@ -172,6 +173,7 @@ lark-cli slides +replace-slide --as user \ | `revision_id` | 成功后的新版本号，下次做乐观锁时用 | | `failed_part_index` | 有部分失败时存在，指向第几条 part 失败 | | `failed_reason` | 失败原因文字描述 | +| `issues` | 与 `failed_reason` 相对：parts 已全部生效，服务端只是仍有发现，不影响本次调用的成功状态。内容是未达阻断级的版式校验发现；校验主体是**拼装后的整页**，因此可能报出页面上原有的元素。格式不固定，不要解析；出现就用 `+screenshot` 复核该页 | 整批作为原子事务：任一 part 失败则整批不生效，服务端通过 `failed_part_index` / `failed_reason` 告诉你是哪条；按此定位修正后重发。 @@ -249,6 +251,7 @@ lark-cli slides +replace-slide --as user \ | `--parts[i] (block_replace) requires non-empty block_id` / `replacement` | 字段名对，但值缺失或是空串 | 按 parts 元素结构补齐值 | | `` 不显示 / 显示破图 | `src` 写了外链 URL | 换成通过 [`+media-upload`](lark-slides-media-upload.md) 拿到的 `file_token` | | 3350001 | `replacement` 不是合法单根 XML 片段，或 `block_id` 不存在 | CLI 已自动注入 `id` 和 ``；如果仍报错，重新 `slide.get` 拿最新 XML 确认 `block_id` 存在；检查 XML 结构是否合法；坐标是否超出 960×540 | +| 4000153 `xml lint blocked` | 服务端版式校验拒绝了本次提交，页面维持原状；校验主体是拼装后的整页，因此片段自身合法也可能因与相邻元素重叠、或页面原有元素越界而被拒 | `error.message` 是完整的校验报告，按其中每条发现给出的修改建议修正；需要调整既有元素时，在同一批 `--parts` 内一并提交 | | 403 | 权限不足 | 需要 `slides:presentation:update` 或 `slides:presentation:write_only`；wiki URL 还需要 `wiki:node:read` | ## 相关命令 diff --git a/skills/lark-slides/references/cli/lark-slides-update-slide.md b/skills/lark-slides/references/cli/lark-slides-update-slide.md index 4e9dc6ee0f..7ded368b96 100644 --- a/skills/lark-slides/references/cli/lark-slides-update-slide.md +++ b/skills/lark-slides/references/cli/lark-slides-update-slide.md @@ -34,6 +34,7 @@ lark-cli slides +update-slide --as user \ | `--content` | 是 | 这一页的完整目标 XML，单一 `` 根；支持字面量、`@file`、stdin `-`。别名：`--xml` / `--slide-xml` / `--slide-content` / `--content-xml` | | `--revision-id` | 否 | 默认 `-1`（最新）。它只选择服务端执行所基于的快照，不是“页面有新编辑就拒绝”的乐观锁；传旧版本号会以旧快照重建页面并丢弃其后的编辑 | | `--tid` | 否 | 调用方提供的任务/事务标识，CLI 原样透传；用于关联同一编辑任务或重试，不等同于版本前置条件，不能单独保证并发冲突时拒绝写入。一般留空 | +| `--no-lint` | 否 | 跳过服务端版式校验（默认开启）；仅在确认校验误判、该页必须原样发布时使用 | `@file` 和 `+xml-get --output` 一样**只接受当前目录下的相对路径**，绝对路径会被拒。 命令别名：`slides +update`（隐藏）；服务别名：`lark-cli slide …` 等价于 `lark-cli slides …`。 @@ -147,6 +148,7 @@ lark-cli slides +xml-get --as user \ | `slide_id` | 与传入相同——整页覆盖不换页 id | | `revision_id` | 写入后的新版本号 | | `images_uploaded` | 仅当 `--content` 带 `@` 占位符时出现：本次去重后实际上传的图片张数 | +| `issues` | 仅在**页面已写入成功**且服务端有发现时返回，不影响本次调用的成功状态。内容是未达阻断级的版式校验发现。格式不固定，不要解析；出现就用 `+screenshot` 复核该页 | 服务端拒绝这次写入时（`failed_reason` 非空）**不会**返回成功输出，而是报错并带上原因——单个 part 承载整页，任何失败都意味着页面没被写入。 @@ -160,4 +162,5 @@ lark-cli slides +xml-get --as user \ | 3350001，原因包含 `not found` | `--presentation` 不匹配，或 `--slide-id` 对应的页面已被删除 | 检查 `--presentation` 和 `--slide-id`，再用 `slides +xml-get` 回读当前页面 ID | | 3350001，其他 invalid param | `--content` 的 XML 结构有问题（如 `` 缺 ``、包含服务端不支持的元素） | 按 [error-handling.md](../workflow/error-handling.md) 检查 `--content` 的 XML 结构 | | 3350002 not found | `--revision-id` 传了不存在的版本号 | 用 `-1` 或真实存在的 `revision_id` | +| 4000153 `xml lint blocked` | 服务端版式校验拒绝了本次写回，页面维持原状 | `error.message` 是完整的校验报告，按其中每条发现给出的修改建议修正后重试 | | 1061004 / 403 | 当前身份对这份 PPT 没有编辑权限 | 检查是否拥有 `slides:presentation:update` 或 `slides:presentation:write_only` scope；wiki 链接另需 `wiki:node:read`，`@` 占位符另需 `docs:document.media:upload`；`--as bot` 还要求该 bot 对目标 PPT 有编辑权限 | diff --git a/skills/lark-slides/references/workflow/error-handling.md b/skills/lark-slides/references/workflow/error-handling.md index f89385d88c..58bb90494b 100644 --- a/skills/lark-slides/references/workflow/error-handling.md +++ b/skills/lark-slides/references/workflow/error-handling.md @@ -53,6 +53,7 @@ | 1061004 forbidden | 当前用户对演示文稿无编辑权限 | 确认当前用户对目标 PPT 有编辑权限 | | 3350001 | XML 非 well-formed、XML 结构不符合服务端要求，或 replace 片段问题 | 优先检查未转义字符；replace 场景再看 `block_id` 和 `` | | 3350002 | `revision_id` 大于当前版本 | 用 `-1` 取当前版本，或重新用 `slides +xml-get` 取最新 `revision_id` | +| 4000153 `xml lint blocked` | 服务端版式校验拒绝了本次写入，被拒的页面未写入（`+create` 逐页提交，之前的页面保留） | 完整的校验报告在 `error.message`；`+replace-pages --continue-on-error` 不中断整批，被拒的那几页改在 `results[]` 里以 `error_code: 4000153` + `error`（同一份报告）+ `hint` 给出。按报告里每条发现的修改建议修正后重试。只有阻断级发现会拦截，未拦截的发现不会丢失——写入成功时通过返回值里的 `issues` 字段给出。确认误判时才用 `--no-lint` | | validation: unsafe file path | `--file` 给了绝对路径或上层路径 | `--file` 必须是 CWD 内相对路径；先 `cd` 到素材目录再执行 | ## Command-Specific References