fix: resolve schema against runtime metadata in plugin builds; gate cache overlay by version#1764
Conversation
📝 WalkthroughWalkthroughThis PR adds ChangesSchemaCatalog and version-gated registry overlay
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1764 +/- ##
=======================================
Coverage 74.43% 74.44%
=======================================
Files 854 854
Lines 88490 88494 +4
=======================================
+ Hits 65867 65877 +10
+ Misses 17554 17549 -5
+ Partials 5069 5068 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Binaries built from the bare Go module (plugin builds) embed only the empty meta_data_default.json stub because meta_data.json is gitignored and fetched at build time. The schema command, its completion, and the affordance command-form resolver read the embedded-only catalog, so every schema lookup failed with "Unknown service" even though the runtime registry had already sync-fetched full metadata. Add registry.SchemaCatalog(): embedded when compiled in (official builds unchanged, still deterministic), otherwise the merged runtime catalog seeded from cache or remote fetch. When neither source has data (offline plugin build with a cold cache), schema now returns a failed_precondition error with an actionable hint instead of "Unknown service" with an empty candidate list.
The cached remote meta was overlaid onto the embedded meta_data.json unconditionally, so after a CLI upgrade an equal- or older-version cache kept shadowing the freshly shipped embedded definitions until a later refresh happened to rewrite it. Only overlay when the cache version is strictly newer than the embedded baseline. The bare-module stub baseline is "0.0.0", so plugin builds without compiled metadata still take any real cached version (TestOverlayGate_StubEmbedded_OverlaysRealCache) and the schema runtime fallback keeps working offline from a warm cache. Ports #1376 onto the typed meta model.
c6bd6b3 to
b7ac661
Compare
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@b7ac661e73f24fc8c0b1824fa093fe4c13c310dc🧩 Skill updatenpx skills add larksuite/cli#fix/schema-meta-fallback -y -g |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 89-102: Add a test in schema_test.go that exercises runSchema’s
empty-catalog precondition by forcing registry.SchemaCatalog() to return a
catalog with no Services, then assert the returned error is a
failed_precondition validation error. Use the runSchema and SchemaCatalog
symbols to locate the new branch, and verify the hint/error path for the “No API
metadata available” case.
🪄 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: 8ca0aaf0-b609-436a-bad1-d5708e9d5022
📒 Files selected for processing (7)
cmd/schema/schema.gointernal/affordance/affordance.gointernal/registry/catalog.gointernal/registry/catalog_test.gointernal/registry/loader.gointernal/registry/loader_test.gointernal/registry/remote_test.go
…ma catalog Two plugin_e2e tests resolved differently on a clean CI runner than on a developer box because they used run(), which inherits the host's ~/.lark-cli metadata cache and network. On CI (cold cache, offline) they failed; locally they passed. - Replace TestSubsystemTransportAbort with TestRuntimeCatalogResolvesSchema. The transport round-trip it targeted is unreachable in a bare-module fork offline (credential resolution fails first); the transport path is covered end-to-end by the sidecar suite and the credential-block test. The new test seeds an obviously-synthetic service into a bare-module fork's on-disk cache and asserts `schema` resolves it from the runtime catalog -- the primary fix for #1764 (module builds now consult the runtime catalog instead of returning "Unknown service"). It complements the existing cold-cache degrade test, pinning both branches, and runs fully offline. - Drop the readonly parent-group sub-case: mixed_children_policy needs a metadata-enumerated child tree the bare-module fork does not have, so offline it collapses to domain_not_allowed; that reason_code is covered by cmdpolicy unit tests. - Fix errorlint in run() (errors.As), and stream `git archive` into `tar` through an explicit pipe instead of a shell in gitArchive. - Set persist-credentials: false on the two new job checkouts and reuse assertReasonCodeEnvelope in TestReadonlyDenial.
Summary
Two related metadata-loading fixes:
Plugin builds:
schemafailed withUnknown service. Binaries built from the bare Go module (a wrapper main that blank-imports a plugin package and builds againstgithub.com/larksuite/cli) embed only the emptymeta_data_default.jsonstub —meta_data.jsonis gitignored and fetched at build time, and the Go module zip only packs tracked files. The runtime registry already sync-fetches full metadata on first run (service commands work), butschema, its shell completion, and the affordance command-form resolver read the embedded-only catalog, so every lookup failed withUnknown serviceand an empty candidates hint.Stale cache shadowing embedded meta (ports fix: gate cached meta overlay on version newer than embedded #1376). Cached remote meta was overlaid onto the embedded baseline unconditionally, so after a CLI upgrade an equal- or older-version cache kept shadowing the freshly shipped embedded definitions. fix: gate cached meta overlay on version newer than embedded #1376 (approved) predates the typed meta model and no longer applies to main; re-implemented on top of it.
Changes
registry.SchemaCatalog(): returns the embedded catalog when metadata is compiled in (official builds unchanged, still deterministic), otherwise the merged runtime catalog seeded from cache or remote fetchcmd/schema: resolve + completion go throughSchemaCatalog(); when no metadata exists at all (offline plugin build with a cold cache) schema returnsfailed_preconditionwith an actionable hint instead ofUnknown servicewith an empty listinternal/affordance: command-form resolver uses the same catalog source, restoring affordance mapping for irregularly-pluralized resources in plugin buildsInitWithBrand: overlay the cache only whenupdate.IsNewer(cached.Version, embeddedVersion); the bare-module stub baseline is0.0.0, so plugin builds keep taking any real cached versionTest Plan
go test -race -count=1 ./cmd/... ./internal/... ./shortcuts/... ./extension/...— 106 packages ok, 0 failures, 0 data races--new-from-revmain) 0 issues; lintcheck clean;make quality-gatepass;make script-test95/95schemadump (1.74 MB), service/method envelopes, completion, root/service help, error surfaces — all identicalmeta_data.json): first run sync-fetches and schema resolves; warm cache within TTL does not refetch; offline cold cache returns the actionablefailed_preconditionenvelope; full schema dump byte-identical to the official builddata_versionprotocol verified live: current version returns not-modified (data:{}), older versions return the full payloadRelated Issues
Ports #1376 onto the typed meta model.
Summary by CodeRabbit
New Features
Bug Fixes
Tests