Skip to content

fix(apps): read app object from data.app for +create and +update#1087

Merged
liangshuo-1 merged 3 commits into
mainfrom
fix/apps-response-app-wrapper
May 25, 2026
Merged

fix(apps): read app object from data.app for +create and +update#1087
liangshuo-1 merged 3 commits into
mainfrom
fix/apps-response-app-wrapper

Conversation

@raistlin042
Copy link
Copy Markdown
Collaborator

@raistlin042 raistlin042 commented May 25, 2026

Summary

The Miaoda OpenAPI returns the application object nested under data.app for both POST /apps (create) and PATCH /apps/{appId} (update), per the design doc. The CLI text helper was reading common.GetString(data, "app_id"), which yields an empty string against the wire format — so lark-cli apps +create --format pretty printed created: with no ID. Same defect on update.

Changes

  • shortcuts/apps/apps_create.go, shortcuts/apps/apps_update.go: navigate the new nested path via common.GetString(data, "app", "app_id").
  • shortcuts/apps/apps_create_test.go, shortcuts/apps/apps_update_test.go: wrap the mock response under app to match the wire shape.
  • skills/lark-apps/references/lark-apps-create.md, .../lark-apps-update.md: update the example response with the app wrapper, plus a note about the new field path so agents read data.app.app_id.
  • skills/lark-apps/references/lark-apps-html-publish.md: update the jq path in scenario 2 from .data.app_id to .data.app.app_id.

Wire format is passed through to the user's JSON envelope untouched — no unwrapping in CLI. Consumers reading the response should use .data.app.app_id.

The GET /apps list endpoint (apps_list.go) is unchanged: per the design doc its items[] are flat objects, no wrapper.

Test Plan

  • go test ./shortcuts/apps/ ./tests/cli_e2e/apps/ passes
  • go vet ./... clean
  • gofmt -l shortcuts/apps/ clean
  • Manual: rebuilt binary and confirmed apps +create --help / apps +update --help register without flag changes
  • Existing happy-path unit tests (TestAppsCreate_Success, TestAppsCreate_WithIconURL, TestAppsUpdate_Success, TestAppsUpdate_TrimsAppIDInPath) updated to the new mock shape and pass

Related Issues

  • None

Summary by CodeRabbit

  • Bug Fixes

    • Fixed create/update commands so the created/updated app ID is read and displayed from the corrected response structure.
  • Documentation

    • Updated examples and docs to reflect the nested response format and correct path to the application object and ID.
    • Clarified recovery guidance for authorization failures to prefer a one-time domain login before retrying.
  • Tests

    • Added tests verifying CLI/pretty output reads the app ID from the updated response shape.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 25, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: cd709e44-ee7b-4cfc-8dae-d6eb1c978c97

📥 Commits

Reviewing files that changed from the base of the PR and between d743870 and ec98eee.

📒 Files selected for processing (2)
  • skills/lark-apps/SKILL.md
  • skills/lark-apps/references/lark-apps-html-publish.md

📝 Walkthrough

Walkthrough

Create/update commands, tests, and documentation were updated so application objects are returned under data.app; the CLI now reads app_id from .data.app.app_id and tests/docs were adjusted accordingly.

Changes

Apps API Response Structure Update

Layer / File(s) Summary
Apps create command and tests
shortcuts/apps/apps_create.go, shortcuts/apps/apps_create_test.go
AppsCreate now reads the created app ID from nested data["app"]["app_id"]. Test stubs for success and icon flows were updated to nest the app object under data.app, and a pretty-output test was added.
Apps update command and tests
shortcuts/apps/apps_update.go, shortcuts/apps/apps_update_test.go
AppsUpdate now reads the updated app ID from data["app"]["app_id"]. PATCH request/response stubs were changed so data contains a nested app object; tests updated and a pretty-output test was added.
Reference documentation & SKILL
skills/lark-apps/references/lark-apps-create.md, skills/lark-apps/references/lark-apps-update.md, skills/lark-apps/references/lark-apps-html-publish.md, skills/lark-apps/SKILL.md
Create/update success response examples and html-publish snippet now show the app object under data.app and use .data.app.app_id. SKILL guidance refined for authorization failure recovery (one-time lark-cli auth login --domain apps).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

domain/ccm

Suggested reviewers

  • liangshuo-1
  • fangshuyu-768

Poem

🐰 I hopped through docs and test arrays,
Found IDs now nested in new ways.
Create and Update sing,
"data.app.app_id" is the thing—
The rabbit twirls and cheers for days 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: fixing the CLI to read the app object from the nested data.app field for both create and update operations.
Description check ✅ Passed The description comprehensively covers the Summary, Changes, Test Plan, and Related Issues sections following the repository template with clear explanations of the problem, solution, and verification.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/apps-response-app-wrapper

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added the size/M Single-domain feat or fix with limited business impact label May 25, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 25, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.87%. Comparing base (aea9f37) to head (ec98eee).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1087      +/-   ##
==========================================
+ Coverage   67.85%   67.87%   +0.02%     
==========================================
  Files         592      592              
  Lines       55373    55368       -5     
==========================================
+ Hits        37574    37582       +8     
+ Misses      14685    14674      -11     
+ Partials     3114     3112       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 25, 2026

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@ec98eee38158d3f723bdff449083431a436d4bdd

🧩 Skill update

npx skills add larksuite/cli#fix/apps-response-app-wrapper -y -g

@raistlin042 raistlin042 force-pushed the fix/apps-response-app-wrapper branch from a3858b3 to 55f5858 Compare May 25, 2026 13:31
The Miaoda OpenAPI returns the application object nested under
data.app for both POST /apps and PATCH /apps/{appId}. The CLI text
helper was reading common.GetString(data, "app_id"), which yields an
empty string against the wire format -- so `lark-cli apps +create
--format pretty` printed `created: ` with no ID.

Navigate the new nested path via GetString(data, "app", "app_id") for
both create and update. Update unit-test mocks to wrap the response
under `app`. Refresh the lark-apps skill references (example response
shape + jq paths) so agents reading them follow the right path.

Wire format is passed through to the user's JSON envelope untouched
-- no unwrapping in CLI. Consumers reading the response should use
.data.app.app_id.

The GET /apps list endpoint is unchanged: per the design doc its
items[] are flat objects, no wrapper.
@raistlin042 raistlin042 force-pushed the fix/apps-response-app-wrapper branch from 55f5858 to d743870 Compare May 25, 2026 13:50
The "用户没有 app_id" snippet in lark-apps-html-publish.md was missing
the required --app-type flag, so copy-pasting it triggered Validate
("--app-type is required") and left $APP empty -- the following
+html-publish then failed with --app-id "". Bring the snippet in line
with every other apps +create example in the skill.
Every apps shortcut declares Scopes, so the precheck path in
shortcuts/common/runner.go:825 is always the one that fires on scope
violations and the envelope's error.type is the stable discriminator.
Drop the keyword-sniffing of error.hint, the chain explanation, and the
bot caveat — they all reduce to one boolean: error.type == "missing_scope"
→ run `lark-cli auth login --domain apps`.

Also collapse the corresponding bullet in 快速决策 to point at this rule.
@liangshuo-1 liangshuo-1 merged commit 0dda569 into main May 25, 2026
21 checks passed
@liangshuo-1 liangshuo-1 deleted the fix/apps-response-app-wrapper branch May 25, 2026 15:16
@liangshuo-1 liangshuo-1 mentioned this pull request May 26, 2026
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Single-domain feat or fix with limited business impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants