feat(schema): output MCP tool spec envelope for all API commands#1048
Open
sang-neo03 wants to merge 23 commits into
Open
feat(schema): output MCP tool spec envelope for all API commands#1048sang-neo03 wants to merge 23 commits into
sang-neo03 wants to merge 23 commits into
Conversation
Rewrites cmd/schema/schema.go so the default --format json branch emits MCP-spec envelopes via schema.AssembleAll/AssembleService/AssembleEnvelope. The legacy --format pretty branch is preserved verbatim and still uses printServices / printResourceList / printMethodDetail. Args max raised from 1 to 8 so the path can be supplied either as a single dotted argument (im.reactions.list) or as space-separated segments (im reactions list); both forms route through schema.ParsePath and produce byte-identical output. The completeSchemaPath function is extended to drive tab-completion for both forms: legacy dotted prefix when len(args) == 0, and per-segment resource/method completion when args already contains earlier segments. BREAKING CHANGE: default JSON output shape changes from the raw meta_data structure to an MCP envelope array/object. Existing scripts parsing the old shape must either pin --format pretty or migrate to the new envelope fields (name, description, inputSchema, outputSchema, _meta).
Replaces TestSchemaCmd_NoArgs with two variants reflecting the new default shape: TestSchemaCmd_NoArgs_Pretty asserts the legacy "Available services" text appears only under --format pretty, and TestSchemaCmd_NoArgs_JSON_IsArray asserts the default JSON output parses as an envelope array with at least 180 entries. Adds six new tests: - TestSchemaCmd_JSONIsEnvelope: single-method output has name / description / inputSchema / outputSchema / _meta keys and envelope_version "1.0". - TestSchemaCmd_SpaceSeparatedPath_EqualsDotted: dotted and space forms produce identical output bytes for the same command path. - TestSchemaCmd_ServiceListIsArray: schema <service> returns a JSON array whose every entry's name starts with "<service> ". - TestSchemaCmd_HighRiskYesInjection: high-risk-write commands inject inputSchema.properties.yes. - TestSchemaCmd_NoYesForReadRisk: read-risk commands do not inject yes. - TestSchemaCmd_PrettyUnchanged_KeyTextPresent: --format pretty still surfaces the legacy section markers (Parameters:, Response:, Identity:, Scopes:, CLI:).
Nested resources whose meta_data key contains a dot (e.g. chat.members,
user_mailbox.templates) were previously split on '.' and rejoined with
spaces, producing envelope names like 'im chat members bots'. AI
consumers doing name.split(' ') and feeding the result back as argv
got 'lark-cli im chat members bots' which the CLI rejects — the actual
invocation form is 'lark-cli im chat.members bots'.
Pass the dotted resource key as a single argv segment so the envelope
name 'im chat.members bots' round-trips through name.split(' ') back
to the CLI. Mirror the same convention in the golden harness so its
single-method assembly matches the live AssembleService walk.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThis PR adds MCP envelope types and OrderedProps, deterministic assembly from embedded meta_data.json with recorded key orders, CLI parsing/completion supporting dotted and space-separated paths, JSON/pretty output modes (JSON produced from embedded specs), linting and coverage utilities, golden fixtures, and tests exercising assembly and linting. ChangesSchema Envelope Assembly and Validation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@54389d3484d85778c6f41798eefaf2c18cdd41fc🧩 Skill updatenpx skills add larksuite/cli#feat/ai-friendly-schema -y -g |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/schema/schema_test.go (1)
16-18:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSet isolated config dir in tests that use
cmdutil.TestFactory.Please set
LARKSUITE_CLI_CONFIG_DIRtot.TempDir()in these tests (or via a shared helper) to prevent cross-test config-state coupling.As per coding guidelines
**/*_test.go:Use t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) to isolate config state in tests.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/schema/schema_test.go` around lines 16 - 18, TestSchemaCmd_FlagParsing uses cmdutil.TestFactory and must isolate CLI config state; before calling cmdutil.TestFactory in TestSchemaCmd_FlagParsing, call t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) (or add a shared helper that sets this env var) so each test gets its own config directory and avoids cross-test coupling.
🧹 Nitpick comments (1)
internal/schema/testdata/golden/wiki.spaces.create.json (1)
4-35: 💤 Low valueConsider adding explicit
"required"field for consistency.The
inputSchemaomits the"required"field entirely, while all other golden fixtures include an explicit"required"array (even when empty or with just required params). For deterministic serialization and structural consistency across the 193 API method envelopes, consider always emitting"required": []when no fields are required.Proposed addition for consistency
"inputSchema": { "type": "object", + "required": [], "properties": {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/schema/testdata/golden/wiki.spaces.create.json` around lines 4 - 35, The inputSchema for this golden fixture is missing an explicit "required" array which other fixtures include; update the inputSchema object (the one containing "properties" with keys name, description, open_sharing, yes) to include a "required" field—use "required": [] if no properties are mandatory (or list the required property names if some must be required) so serialization is deterministic and consistent across fixtures.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/schema/schema.go`:
- Around line 596-607: The code currently accepts remaining[0] as the method
name and silently ignores remaining[1:], causing invalid paths to be accepted;
update the handler that reads remaining and methodName (and the analogous block
that resolves operations) to explicitly reject extra trailing segments: if
len(remaining) != 1 (or if len(remaining) > 1) return
output.ErrWithHint(output.ExitValidation, "validation", fmt.Sprintf("Invalid
path: extra trailing path segments on %s.%s", serviceName, resName),
fmt.Sprintf("Got: %v", remaining[1:])), so callers of methods,
resource["methods"], methodName and the other resolution block fail fast instead
of silently resolving to a different method.
In `@internal/registry/loader.go`:
- Around line 28-30: The EmbeddedMetaJSON function (and the other embedded
getter functions referenced around lines 75-77) currently returns the
package-level byte slice directly, allowing callers to mutate shared state;
change each getter (e.g., EmbeddedMetaJSON) to return a defensive copy of the
underlying slice by allocating a new []byte and copying the contents (for
example via append([]byte(nil), embeddedMetaJSON...) or make + copy) so callers
cannot modify the original package-level slice.
In `@internal/schema/assembler_test.go`:
- Around line 549-550: Before calling registry.LoadFromMeta in the tests that
call AssembleService (e.g., the test cases invoking registry.LoadFromMeta("im")
and AssembleService("im", spec, nil)), isolate registry-backed state by setting
the config dir to a temp directory using t.Setenv("LARKSUITE_CLI_CONFIG_DIR",
t.TempDir()) at the start of each test; apply this change to the occurrences
around the registry.LoadFromMeta calls (the blocks that start with spec :=
registry.LoadFromMeta("im") / envs := AssembleService(...)) including the other
noted occurrences in the file so host config/cache cannot leak into test
results.
In `@internal/schema/lint.go`:
- Around line 153-155: The current check only recurses into array item schemas
when p.Items.Properties exists, so primitive or invalid item schemas
(p.Items.type, etc.) are not linted; update the logic in validatePropertyTypes
to always validate the p.Items schema node when p.Items != nil — if
p.Items.Properties is present recurse with
validatePropertyTypes(p.Items.Properties, false, errs), otherwise run the
schema-level validation on p.Items (the same validations used for non-array
property schemas) and append errors to errs so invalid primitive item schemas
are caught.
In `@internal/schema/testdata/golden/calendar.calendars.list.json`:
- Around line 7-14: The "page_size" schema defines type "integer" but sets
"default" as the string "500"; update the default to an integer (500) to match
the declared type for the page_size property in the JSON schema (look for the
"page_size" object and its "default" field) so the default value's type aligns
with "type": "integer".
In
`@internal/schema/testdata/golden/mail.user_mailbox.template.attachments.download_url.json`:
- Around line 24-28: The "attachment_ids" schema entry uses an invalid JSON
Schema type "list"; update the "attachment_ids" object so its "type" is "array"
(replace "list" with "array") and, if missing, add an "items" definition (e.g.,
{"type":"string"} or the appropriate item schema) to match other golden files
and restore JSON Schema validation for the property.
In `@internal/schema/testdata/golden/mail.user_mailbox.templates.create.json`:
- Around line 291-292: The create operation currently has "danger": true but an
incorrect "risk": "read"; update the risk value for the create with scope
"mail:user_mailbox.message:modify" (and any other create entries marked
danger:true) to an appropriate write-level risk such as "write" or
"high-risk-write" so it matches the modify/create semantics and danger flag;
locate the JSON entry containing "danger": true and "risk": "read" associated
with the mail:user_mailbox.message:modify create operation and change "risk" to
"write" (or "high-risk-write" if applicable).
In `@internal/schema/types.go`:
- Around line 85-88: The MarshalJSON method on OrderedProps currently returns
"{}" when o.Order is empty, which drops any entries present in o.Map; change
MarshalJSON (for receiver OrderedProps) so that when o is nil return "{}", but
if o.Order is empty and o.Map is non-nil/non-empty it marshals the entries from
o.Map (instead of returning "{}"), otherwise preserve existing behavior that
emits fields in o.Order; reference OrderedProps.MarshalJSON, the Order slice,
and the Map field when implementing the fix.
---
Outside diff comments:
In `@cmd/schema/schema_test.go`:
- Around line 16-18: TestSchemaCmd_FlagParsing uses cmdutil.TestFactory and must
isolate CLI config state; before calling cmdutil.TestFactory in
TestSchemaCmd_FlagParsing, call t.Setenv("LARKSUITE_CLI_CONFIG_DIR",
t.TempDir()) (or add a shared helper that sets this env var) so each test gets
its own config directory and avoids cross-test coupling.
---
Nitpick comments:
In `@internal/schema/testdata/golden/wiki.spaces.create.json`:
- Around line 4-35: The inputSchema for this golden fixture is missing an
explicit "required" array which other fixtures include; update the inputSchema
object (the one containing "properties" with keys name, description,
open_sharing, yes) to include a "required" field—use "required": [] if no
properties are mandatory (or list the required property names if some must be
required) so serialization is deterministic and consistent across fixtures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 68123346-b43c-48f9-831e-92fc4c6cee2c
📒 Files selected for processing (33)
cmd/schema/schema.gocmd/schema/schema_test.gointernal/registry/loader.gointernal/schema/annotations/.gitkeepinternal/schema/assembler.gointernal/schema/assembler_test.gointernal/schema/golden_test.gointernal/schema/lint.gointernal/schema/lint_test.gointernal/schema/path.gointernal/schema/path_test.gointernal/schema/testdata/golden/approval.instances.cancel.jsoninternal/schema/testdata/golden/approval.instances.get.jsoninternal/schema/testdata/golden/calendar.calendars.list.jsoninternal/schema/testdata/golden/calendar.events.create.jsoninternal/schema/testdata/golden/drive.files.copy.jsoninternal/schema/testdata/golden/im.chats.create.jsoninternal/schema/testdata/golden/im.images.create.jsoninternal/schema/testdata/golden/im.messages.delete.jsoninternal/schema/testdata/golden/im.pins.delete.jsoninternal/schema/testdata/golden/im.reactions.create.jsoninternal/schema/testdata/golden/im.reactions.list.jsoninternal/schema/testdata/golden/mail.user_mailbox.folders.delete.jsoninternal/schema/testdata/golden/mail.user_mailbox.messages.get.jsoninternal/schema/testdata/golden/mail.user_mailbox.template.attachments.download_url.jsoninternal/schema/testdata/golden/mail.user_mailbox.templates.create.jsoninternal/schema/testdata/golden/okr.objectives.delete.jsoninternal/schema/testdata/golden/sheets.spreadsheet.sheet.filters.create.jsoninternal/schema/testdata/golden/slides.xml_presentation.slide.replace.jsoninternal/schema/testdata/golden/task.tasks.delete.jsoninternal/schema/testdata/golden/wiki.spaces.create.jsoninternal/schema/types.gointernal/schema/types_test.go
Comment on lines
+28
to
+30
| func EmbeddedMetaJSON() []byte { | ||
| return embeddedMetaJSON | ||
| } |
There was a problem hiding this comment.
Return defensive copies from embedded getters.
These functions currently expose mutable package-level slices. A caller can mutate shared state and affect subsequent assembly determinism.
🔧 Proposed fix
func EmbeddedMetaJSON() []byte {
- return embeddedMetaJSON
+ if len(embeddedMetaJSON) == 0 {
+ return nil
+ }
+ out := make([]byte, len(embeddedMetaJSON))
+ copy(out, embeddedMetaJSON)
+ return out
}
@@
func EmbeddedServiceNames() []string {
parseEmbeddedServices()
- return embeddedServiceNames
+ if len(embeddedServiceNames) == 0 {
+ return nil
+ }
+ out := make([]string, len(embeddedServiceNames))
+ copy(out, embeddedServiceNames)
+ return out
}Also applies to: 75-77
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/registry/loader.go` around lines 28 - 30, The EmbeddedMetaJSON
function (and the other embedded getter functions referenced around lines 75-77)
currently returns the package-level byte slice directly, allowing callers to
mutate shared state; change each getter (e.g., EmbeddedMetaJSON) to return a
defensive copy of the underlying slice by allocating a new []byte and copying
the contents (for example via append([]byte(nil), embeddedMetaJSON...) or make +
copy) so callers cannot modify the original package-level slice.
- coerce enum literals to typed JSON values (integer to int64,
number to float64, boolean to bool) so type:"integer" fields no
longer emit string enums; sort numeric/boolean enums while
preserving meta_data order for string enums that carry semantic
priority
- translate non-standard meta_data type:"list" to JSON Schema
type:"array" with items:{} fallback when element shape is absent
(covers the two mail attachment_ids fields)
- render inputSchema.required even when empty so consumers see a
stable envelope shape ("[]" means no required fields, not "field
is missing")
- reject trailing path segments in both JSON and pretty modes so
schema im.messages.delete.foo errors instead of silently
returning the delete method
- drop dead "list type" entry from lint_test isKnownDataInconsistency
whitelist now that list values are translated upstream
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
internal/schema/testdata/golden/mail.user_mailbox.templates.create.json (1)
291-292:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse write-level risk for this dangerous create operation.
danger: truewith modify scope on a create endpoint should not be classified as"read"; this under-classifies operation safety.🔧 Suggested fix
"danger": true, - "risk": "read", + "risk": "write",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/schema/testdata/golden/mail.user_mailbox.templates.create.json` around lines 291 - 292, The create template currently marks a dangerous create operation with "danger": true but incorrectly sets "risk": "read"; update the JSON in mail.user_mailbox.templates.create (keys "danger" and "risk") to use a write-level risk (change "risk" from "read" to "write") so the create endpoint is classified correctly as a modifying operation.internal/schema/testdata/golden/calendar.calendars.list.json (1)
8-12:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMatch
defaulttype with declared integer schema type.
page_sizeis declared as"type": "integer"but uses a string default"500".🔧 Suggested fix
"page_size": { "type": "integer", - "default": "500", + "default": 500,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/schema/testdata/golden/calendar.calendars.list.json` around lines 8 - 12, The JSON schema property "page_size" declares "type": "integer" but its "default" is the string "500"; change the default value for the page_size property to an integer (500) so the default type matches the declared schema type (look for the page_size property in the calendar.calendars.list JSON schema and replace the quoted "500" with the numeric 500).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/schema/assembler.go`:
- Around line 414-430: The code currently always calls sortEnum after building
p.Enum from optsRaw, which alphabetizes string enums; change the logic in the
branch that builds p.Enum (using optsRaw, om, raw, coerceEnumValue) to preserve
the source order for string enums by only invoking sortEnum(p.Type, p.Enum) when
p.Type is numeric or boolean (i.e., when p.Type != "string"), leaving p.Enum in
the appended order for string types; ensure you still dedupe using seen and keep
using coerceEnumValue and p.Enum append as before, and update
TestConvertProperty_OptionsToEnum expectations to no longer assume alphabetical
ordering for string enums.
---
Duplicate comments:
In `@internal/schema/testdata/golden/calendar.calendars.list.json`:
- Around line 8-12: The JSON schema property "page_size" declares "type":
"integer" but its "default" is the string "500"; change the default value for
the page_size property to an integer (500) so the default type matches the
declared schema type (look for the page_size property in the
calendar.calendars.list JSON schema and replace the quoted "500" with the
numeric 500).
In `@internal/schema/testdata/golden/mail.user_mailbox.templates.create.json`:
- Around line 291-292: The create template currently marks a dangerous create
operation with "danger": true but incorrectly sets "risk": "read"; update the
JSON in mail.user_mailbox.templates.create (keys "danger" and "risk") to use a
write-level risk (change "risk" from "read" to "write") so the create endpoint
is classified correctly as a modifying operation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ad7072a4-90d0-410b-8edb-16a74e2d39f1
📒 Files selected for processing (11)
cmd/schema/schema.gointernal/schema/assembler.gointernal/schema/assembler_test.gointernal/schema/lint_test.gointernal/schema/testdata/golden/calendar.calendars.list.jsoninternal/schema/testdata/golden/im.chats.create.jsoninternal/schema/testdata/golden/mail.user_mailbox.messages.get.jsoninternal/schema/testdata/golden/mail.user_mailbox.template.attachments.download_url.jsoninternal/schema/testdata/golden/mail.user_mailbox.templates.create.jsoninternal/schema/testdata/golden/wiki.spaces.create.jsoninternal/schema/types.go
💤 Files with no reviewable changes (1)
- internal/schema/lint_test.go
CI fix
- Replace hard-coded absolute key-order assertions in TestKeyOrderIndex_*
and TestBuildInputSchema_* with set-membership and propagation invariants;
the upstream meta_data API does not guarantee stable JSON key order across
fetches, so the old tests were flaky on CI by design.
- Skip byte-level TestGoldenEnvelopes when CI=true; golden snapshots are a
manual refresh artefact tied to a specific meta_data fetch, not a CI gate.
- Add TestMain to isolate registry-backed tests from any host ~/.lark-cli
cache (LARKSUITE_CLI_CONFIG_DIR + LARKSUITE_CLI_REMOTE_META=off) so the
suite gives the same answer on every machine.
CodeRabbit review actionables
- EmbeddedServiceNames returns a defensive copy so callers cannot mutate
the package-level slice and affect subsequent assembly determinism.
- coerceEnumValue is now also applied to default literals: integer fields
no longer ship default: "500" — they ship default: 500 (same idea as the
earlier enum coercion fix).
- options-branch string enums preserve meta_data source order, matching the
enum-branch policy; only numeric/boolean enums get sorted.
- validatePropertyTypes now validates the array element schema itself
(type, nested items), not only items.properties — previously a primitive
element with an invalid type (e.g. items.type="list") slipped past lint.
- OrderedProps.MarshalJSON falls back to alphabetical key order when Map
has entries but Order is empty, instead of silently emitting {}.
Tests pass locally and with CI=true env (simulating GitHub Actions).
Re-generated with UPDATE_GOLDEN=1 against the current meta_data.json
snapshot. The bulk of the diff is upstream noise (description wording,
enum entries, field order) which the CI snapshot diff can no longer
reasonably gate (see previous commit). Side-effects of the code fixes
in the parent commit are also captured:
- integer-typed defaults now emit numeric literals (e.g. page_size
default 500, not "500") thanks to coerceEnumValue
- mail.user_mailbox.templates.create _meta.risk corrects to "write"
(assembler already emitted "write"; the old golden was stale)
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1048 +/- ##
==========================================
+ Coverage 67.76% 67.85% +0.09%
==========================================
Files 590 594 +4
Lines 55188 55937 +749
==========================================
+ Hits 37398 37956 +558
- Misses 14679 14819 +140
- Partials 3111 3162 +51 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/schema/testdata/golden/im.reactions.list.json (1)
30-34:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
page_size.defaulttype mismatches declared schema type.
page_sizeis declared asinteger, butdefaultis""(string). This makes the contract internally inconsistent.Suggested fix
"page_size": { "type": "integer", "description": "分页大小，用于限制一次请求返回的数据条目数。;;**默认值**：20", - "default": "", + "default": 20, "example": "10", "maximum": 50, "x-in": "query" },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/schema/testdata/golden/im.reactions.list.json` around lines 30 - 34, The schema for the "page_size" property has a default value of "" (string) but its type is integer; update the "page_size" property's "default" to an integer (e.g., 20 to match the description) so the "page_size" entry's "type", "default", and description remain consistent; locate the "page_size" object in the JSON and replace the empty-string default with a numeric value.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/schema/assembler_test.go`:
- Around line 20-27: TestMain currently defers cleanup which never runs because
os.Exit skips defers and also silently continues on MkdirTemp error; change it
to fail-fast on MkdirTemp error (return non-zero exit immediately) and ensure
cleanup always runs by storing the result of m.Run in a variable, calling
os.RemoveAll(dir) after the run, then calling os.Exit(code). Only call
os.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) and the other env setup after
MkdirTemp succeeds; reference the TestMain function, MkdirTemp, os.Setenv,
os.RemoveAll, m.Run, and os.Exit when making the change.
---
Outside diff comments:
In `@internal/schema/testdata/golden/im.reactions.list.json`:
- Around line 30-34: The schema for the "page_size" property has a default value
of "" (string) but its type is integer; update the "page_size" property's
"default" to an integer (e.g., 20 to match the description) so the "page_size"
entry's "type", "default", and description remain consistent; locate the
"page_size" object in the JSON and replace the empty-string default with a
numeric value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 284a855d-b800-42a3-b879-acf198593e58
📒 Files selected for processing (19)
internal/registry/loader.gointernal/schema/assembler.gointernal/schema/assembler_test.gointernal/schema/golden_test.gointernal/schema/lint.gointernal/schema/testdata/golden/approval.instances.get.jsoninternal/schema/testdata/golden/calendar.calendars.list.jsoninternal/schema/testdata/golden/calendar.events.create.jsoninternal/schema/testdata/golden/drive.files.copy.jsoninternal/schema/testdata/golden/im.chats.create.jsoninternal/schema/testdata/golden/im.reactions.create.jsoninternal/schema/testdata/golden/im.reactions.list.jsoninternal/schema/testdata/golden/mail.user_mailbox.folders.delete.jsoninternal/schema/testdata/golden/mail.user_mailbox.messages.get.jsoninternal/schema/testdata/golden/mail.user_mailbox.templates.create.jsoninternal/schema/testdata/golden/sheets.spreadsheet.sheet.filters.create.jsoninternal/schema/testdata/golden/slides.xml_presentation.slide.replace.jsoninternal/schema/testdata/golden/wiki.spaces.create.jsoninternal/schema/types.go
✅ Files skipped from review due to trivial changes (4)
- internal/schema/testdata/golden/im.reactions.create.json
- internal/schema/testdata/golden/approval.instances.get.json
- internal/schema/testdata/golden/sheets.spreadsheet.sheet.filters.create.json
- internal/schema/testdata/golden/im.chats.create.json
- TestMain: cleanup now runs reliably. os.Exit skips deferred functions, so the previous defer os.RemoveAll(dir) never executed. Replace defer with explicit cleanup, and fail fast if MkdirTemp errors instead of silently running against the host cache (which defeats isolation). - convertProperty default coercion: when the literal cannot be coerced to the declared type (e.g. default:"" on integer field, used by meta_data to mean "no default"), omit the field entirely rather than emit a type-mismatched default. Removes a contract violation flagged on im.reactions.list.json#page_size.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replace
lark-cli schema <path>default JSON output with MCP Tool spec compliant envelopes covering all 193 embedded API methods, so AI / MCP consumers can read the full call contract (parameters, types, conditional flags, scopes, risk, access tokens, doc URL) directly from one envelope without writing per-command Skill markdown. This is a BREAKING CHANGE to the JSON shape only;--format prettykeeps its legacy ANSI rendering and remains the developer-facing browsing view.Changes
internal/schema/package — stateless transformation frommeta_data.jsonto MCP envelope:Envelope/InputSchema/OutputSchema/Property(with order-preservingOrderedProps) /Meta/Affordancetypes intypes.go;AssembleEnvelope/AssembleService/AssembleAll/ParsePathentry points inassembler.goandpath.go; L1–L3 hard-fail lint + L4 coverage inlint.go; embed slotannotations/for future affordance overlays (PR-1 stub, no YAML).meta_data.jsononly, bypassingremote_meta.jsonoverlay — guarantees deterministic output across machines / brands. Newregistry.EmbeddedSpec/EmbeddedServiceNames/EmbeddedMetaJSONhelpers ininternal/registry/loader.go.cmd/schema/schema.goJSON branch rewired to envelope output; pretty branch (printServices/printResourceList/printMethodDetail) preserved verbatim and still walks merged registry data for runtime inspection. Cobra arg cap raised from 1 to 8;completeSchemaPathextended to handle both dotted (im.messages.reply) and space-separated (im messages reply) forms.namefield reflects the actual CLI argv form: nested resources keep their dotted segment (e.g."im chat.members bots"), soname.split(" ")is directly executable asargv.inputSchema.properties.yesinjected for high-risk-write methods (52 envelopes); file fields encoded astype: "string"+format: "binary"+x-in: "body"; path/query parameters taggedx-in: "path"|"query";_meta.access_tokenstranslatedtenant→botand sorted;_meta.required_scopesalways emitted ([]when empty);_meta.doc_urland_meta.affordanceomitempty.meta_data.jsondata-quality patterns (danger↔riskinconsistency, typeless arrays, non-standardtype: "list",min==max); 20 hand-picked golden fixtures ininternal/schema/testdata/golden/covering edge cases (file upload, high-risk-write, missingrisk,requiredScopes, nested response, bot-only / user-only access tokens, deep nesting, multi-segment resource paths).UPDATE_GOLDEN=1 go test ./internal/schema/...refreshes goldens after meta_data updates.cmd/schema/schema_test.go(envelope JSON shape, dotted-vs-spaced byte equality, bare/service list arrays,yesinjection presence/absence, pretty mode preservation);TestSchemaCmd_NoArgssplit into_Prettyand_JSON_IsArrayfor the new default output.Test Plan
make unit-testpassedgo test $(go list ./... | grep -Ev '/(tests_e2e|tests/cli_e2e)(/|$)') -short -count=1passedtests_e2e/schema/...19/19 subtests passed (8 test functions); other historical E2E packages report env-level failures unrelated to this PR (test appcli_aa9a9dde09781cb0lacks several scopes in the Lark developer console)namesegmentation, spec §5.3 lint known-list disclosure)golangci-lint --new-from-rev=origin/main0 issues;go mod tidyno difflark-cli schema im.images.create --format json | jq '{name, has_yes: (.inputSchema.properties.yes != null), risk: ._meta.risk}'—{"name":"im images create","has_yes":false,"risk":"write"}lark-cli schema im messages delete --format json | jq '._meta.risk, .inputSchema.properties.yes.type'—"high-risk-write"/"boolean"lark-cli schema im.chat.members.bots --format json | jq -r '.name'—im chat.members bots(matcheslark-cli im chat.members botsargv)lark-cli schema --format pretty | head— legacy ANSI service catalog preservedRelated Issues
N/A
Summary by CodeRabbit
New Features
Improvements
yesconfirmation flag.Tests / Quality