Skip to content

feat(apps): add miaoda apps domain (6 shortcuts + dry-run e2e)#1002

Merged
liangshuo-1 merged 46 commits into
mainfrom
feat/apps-domain
May 21, 2026
Merged

feat(apps): add miaoda apps domain (6 shortcuts + dry-run e2e)#1002
liangshuo-1 merged 46 commits into
mainfrom
feat/apps-domain

Conversation

@raistlin042

@raistlin042 raistlin042 commented May 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds the apps domain to lark-cli for managing Miaoda (妙搭) applications: 6 shortcuts covering the full lifecycle (+create / +update / +list / +access-scope-set / +access-scope-get / +html-publish). Aligned with the OAPI v2 design — app_type enum (currently HTML), string scope enum (All / Tenant / Range), cursor pagination, in-memory tar.gz multipart publish flow. Namespace registered at /open-apis/spark/v1/ with spark:app.* scopes.

Changes

  • Shortcuts (shortcuts/apps/): 6 commands + 30+ unit tests (with -race), covering body builders / validators / walker / tarball / publish orchestration / failure-hint mapping.
  • Domain skill (skills/lark-apps/): SKILL.md + 6 reference files describing typical agent scenarios, push-strategy guidance (explicit deploy vs. presentation-only), end-to-end HTML / PPT / static-site flows. +list is Hidden: true and excluded from agent flows; agents that need an existing app_id ask the user for the Miaoda app URL and extract the path segment.
  • Dry-run E2E (tests/cli_e2e/apps/): 6 files, ~40 sub-tests against the real binary; pins request URL / method / body / params shape per AGENTS.md §E2E. Live E2E intentionally deferred (no +delete to clean up created apps; documented in coverage.md).
  • +html-publish packing: in-memory bytes.Buffer tar.gz (no temp files); all filesystem access goes through runtime.FileIO() for cwd-bounded path validation. --path must be a cwd-relative path (e.g. ./dist) and cannot equal cwd.

Security review fixes (round 4)

Address all 16 findings from the in-house security review:

  • Publish-then-public attack chain (P0): +html-publish rejects --path resolving to cwd at both Validate and runHTMLPublish entry; --scope public now requires explicit --require-login (no more fail-open require_login: false default); dry-run scans for .git/ / .env* / *.pem / *.key / SSH keys / AWS-Docker-gcloud-kube credentials and surfaces matches in a warnings envelope field with agent behavior contract in SKILL.md.
  • Stale +list reference (P0): 90002 error hint no longer points at the hidden +list command; instructs the user to confirm app_id from the Miaoda URL.
  • Empty --targets accepted (P1): validateTargetsJSON rejects [] so --scope specific cannot produce a {scope:'Range'} body with zero recipients.
  • Compression-bomb defense (P1): pre-pack maxHTMLPublishRawBytes = 200MB cap on sum(candidate.Size) before the in-memory tar+gzip so highly-compressible giant inputs cannot balloon process memory.
  • Dry-run / Execute symmetry: dry-run now runs ensureIndexHTML and surfaces failures as a validation_error envelope field (advisory, exit 0); --app-id and --path are trimmed in dry-run too so the previewed URL matches what Execute calls.
  • Defense-in-depth tar entry validation: writeHTMLPublishTarEntry rejects RelPath values that look like tar-slip (absolute, .., embedded traversal, null byte) before writing the header.
  • +access-scope-set URL builder: trims --app-id before EncodePathSegment (mirrors the earlier fix in +update / +access-scope-get); dry-run surfaces buildAccessScopeBody errors as a body_error field instead of swallowing.
  • Doc/code coherence: lark-apps-list.md stripped of agent-facing scenarios that contradicted the Hidden: true banner; named const errCodeBuildFailed / errCodeAppNotFound replace the 90001 / 90002 magic numbers; +list table-view column selection documented in source; coverage.md updated to reflect the new dry-run advisory semantics.

Test Plan

  • go test ./shortcuts/apps/... -race -count=3 — all unit tests pass with race detection
  • go test ./tests/cli_e2e/apps/... -count=3 — ~40 dry-run sub-tests pass against the real binary
  • go vet ./... + gofmt -l . clean
  • go mod tidy — no drift in go.mod / go.sum
  • golangci-lint --new-from-rev=origin/main — 0 issues
  • Manual dry-run smoke for all 6 commands

Related Issues

  • None

@CLAassistant

CLAassistant commented May 20, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@coderabbitai

coderabbitai Bot commented May 20, 2026

Copy link
Copy Markdown

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds the Miaoda "apps" domain with six CLI shortcuts, HTML publish tooling (walker, tarball, client), env override for Open API base, local mock/test infra and scripts, extensive unit and dry-run E2E tests, and documentation.

Changes

Miaoda Apps Domain

Layer / File(s) Summary
Open API Base URL Environment Override
internal/core/types.go, internal/core/types_test.go, internal/cmdutil/factory_default.go
Add LARK_CLI_OPEN_API_BASE override (trimmed) used by ResolveOpenBaseURL; factory uses it when constructing Lark client; tests validate override/empty/whitespace behavior.
Apps Domain Registration & Common
shortcuts/apps/common.go, shortcuts/apps/shortcuts.go, shortcuts/apps/shortcuts_test.go, shortcuts/register.go
Define appsService and apiBasePath, export Shortcuts() returning six app shortcuts, and register them in the global shortcuts list.
HTML Publish Infrastructure
shortcuts/apps/walk_html_publish_candidates.go, shortcuts/apps/walk_html_publish_candidates_test.go, shortcuts/apps/html_publish_tarball.go, shortcuts/apps/html_publish_tarball_test.go, shortcuts/apps/html_publish_client.go, shortcuts/apps/html_publish_client_test.go
Implement candidate discovery (file/directory walker), in-memory gzip+tar packing with SHA-256 digest, and multipart publish client with JSON envelope parsing and failure-hint mapping; include unit tests for packing, entry writing, and client behavior.
Apps HTML Publish Command
shortcuts/apps/apps_html_publish.go, shortcuts/apps/apps_html_publish_test.go, tests/cli_e2e/apps/apps_html_publish_dryrun_test.go
Add +html-publish: dry-run manifest, require index.html for publish, build tar.gz (20 MiB default limit), call publish client, and surface URL or error hints; includes unit and dry-run E2E tests.
Apps Create / Update / List
shortcuts/apps/apps_create.go, shortcuts/apps/apps_create_test.go, tests/cli_e2e/apps/apps_create_dryrun_test.go, shortcuts/apps/apps_update.go, shortcuts/apps/apps_update_test.go, tests/cli_e2e/apps/apps_update_dryrun_test.go, shortcuts/apps/apps_list.go, shortcuts/apps/apps_list_test.go, tests/cli_e2e/apps/apps_list_dryrun_test.go
Add +create (HTML-only app_type), +update (partial updates), and +list (cursor pagination) shortcuts with validation, request construction, unit tests, and dry-run E2E tests.
Apps Access Scope Get / Set
shortcuts/apps/apps_access_scope_get.go, shortcuts/apps/apps_access_scope_get_test.go, tests/cli_e2e/apps/apps_access_scope_get_dryrun_test.go, shortcuts/apps/apps_access_scope_set.go, shortcuts/apps/apps_access_scope_set_test.go, tests/cli_e2e/apps/apps_access_scope_set_dryrun_test.go
Add +access-scope-get to fetch scope payloads unchanged and +access-scope-set to configure `specific
Local Testing Infrastructure
scripts/apps-mock.py, scripts/apps-test-env.sh, cmd/_apps_dev_tools/inject_scopes/main.go, tests/cli_e2e/apps/helpers_test.go, tests/cli_e2e/apps/coverage.md
Add a Python mock OAPI server with failure modes, a Bash bootstrap to run/stop the mock and inject scopes, a dev Go tool to merge scopes into stored tokens, E2E helpers, and coverage documentation noting dry-run coverage.
Documentation (SKILL & References)
skill-template/domains/apps.md, skills/lark-apps/SKILL.md, skills/lark-apps/references/*
Add SKILL.md and per-command reference docs covering usage, parameters, validation rules, response shapes, examples, and guidance for all new apps commands.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Suggested labels

domain/ccm, feature

Suggested reviewers

  • liangshuo-1
  • fangshuyu-768
  • wittam-01

🐰 I hopped through files with cheer,
New apps and publish flows appear.
Tarballs packed and mocks in place,
Tests and docs to guide the race.
Cheers — now Miaoda apps can shine!

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/apps-domain

@github-actions github-actions Bot added the size/XL Architecture-level or global-impact change label May 20, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

license-eye has checked 1604 files.

Valid Invalid Ignored Fixed
1149 2 453 0
Click to see the invalid file list
  • cmd/_apps_dev_tools/inject_scopes/main.go
  • scripts/apps-mock.py
Use this command to fix any missing license headers
```bash

docker run -it --rm -v $(pwd):/github/workspace apache/skywalking-eyes header fix

</details>

Comment thread cmd/_apps_dev_tools/inject_scopes/main.go Outdated
Comment thread scripts/apps-mock.py Outdated
Comment thread skills/lark-apps/references/lark-apps-list.md Outdated
@codecov

codecov Bot commented May 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 68.75000% with 145 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.73%. Comparing base (898e0ee) to head (203e2e3).
⚠️ Report is 47 commits behind head on main.

Files with missing lines Patch % Lines
shortcuts/apps/apps_access_scope_set.go 62.83% 29 Missing and 13 partials ⚠️
shortcuts/apps/apps_html_publish.go 64.44% 24 Missing and 8 partials ⚠️
shortcuts/apps/apps_update.go 48.38% 12 Missing and 4 partials ⚠️
shortcuts/apps/apps_list.go 53.33% 12 Missing and 2 partials ⚠️
shortcuts/apps/apps_access_scope_get.go 35.00% 10 Missing and 3 partials ⚠️
shortcuts/apps/apps_create.go 71.87% 5 Missing and 4 partials ⚠️
shortcuts/apps/walk_html_publish_candidates.go 80.48% 4 Missing and 4 partials ⚠️
shortcuts/apps/html_publish_tarball.go 87.50% 3 Missing and 2 partials ⚠️
shortcuts/apps/html_publish_client.go 87.87% 2 Missing and 2 partials ⚠️
shortcuts/apps/sensitive_paths.go 91.66% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1002      +/-   ##
==========================================
+ Coverage   65.92%   67.73%   +1.81%     
==========================================
  Files         523      586      +63     
  Lines       49692    54823    +5131     
==========================================
+ Hits        32758    37135    +4377     
- Misses      14134    14598     +464     
- Partials     2800     3090     +290     

☔ 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

github-actions Bot commented May 20, 2026

Copy link
Copy Markdown

🚀 PR Preview Install Guide

🧰 CLI update

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

🧩 Skill update

npx skills add larksuite/cli#feat/apps-domain -y -g

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 14

🧹 Nitpick comments (1)
shortcuts/apps/apps_html_publish_test.go (1)

31-35: ⚡ Quick win

Don’t ignore fixture write errors in tests.

These fixture writes currently discard errors, which can hide setup failures and create misleading test outcomes. Please assert os.WriteFile errors consistently.

Also applies to: 127-128, 142-142, 161-161

🤖 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 `@shortcuts/apps/apps_html_publish_test.go` around lines 31 - 35, The test
helper writeAppsSampleSite currently ignores the os.WriteFile error; update it
to capture the returned error from os.WriteFile and fail the test on error
(e.g., if err != nil { t.Fatalf("write fixture: %v", err) }) so setup failures
surface; apply the same change to the other fixture writes flagged in this file
(the other os.WriteFile calls around the same helper/test helpers) to
consistently assert write errors.
🤖 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/_apps_dev_tools/inject_scopes/main.go`:
- Around line 56-59: The status messages currently printed with fmt.Printf ("OK:
injected %d..." and each extra scope) are sent to stdout and must be moved to
stderr to preserve stdout as the data channel; update the prints in main.go (the
block that uses variables added, out and iterates over extra) to write to
os.Stderr (e.g., use fmt.Fprintf or fmt.Fprintln with os.Stderr) so all
human-readable progress/warnings go to stderr while leaving stdout untouched.

In `@scripts/apps-mock.py`:
- Around line 72-77: The lines in the request handler use semicolon-chained
statements which trip Ruff E702; replace each chained sequence with one
statement per line: split occurrences like "self.send_response(404);
self.end_headers(); return", "self.send_header('Content-Type', 'text/plain');
self.end_headers(); self.wfile.write(raw); return", and
"self.send_header('Content-Type', 'application/json'); self.end_headers()" into
separate lines so each call (self.send_response, self.end_headers, return,
self.send_header, self.wfile.write) appears on its own line around the
build_response(action) call and subsequent response handling.

In `@shortcuts/apps/apps_access_scope_set.go`:
- Around line 92-101: In the "public" branch of the scope switch in
apps_access_scope_set.go, add validation to reject a provided approver: if scope
== "public" and the approver flag/variable (approver) is non-empty, return
output.ErrValidation("--approver is not allowed when --scope=public"); this
should sit alongside the existing checks for targets and applyEnabled so the CLI
fails early instead of silently dropping approver when building the request
body.

In `@shortcuts/apps/apps_update.go`:
- Around line 43-49: The API path is built from the raw flag value
rctx.Str("app-id") without trimming, so values like " app_x " become encoded
with spaces; update both the PATCH builder and the Execute path construction to
trim the app-id before encoding (use strings.TrimSpace(rctx.Str("app-id")) or
equivalent) where validate.EncodePathSegment(...) is called (affecting the
PATCH(...) call and the path variable in Execute), ensuring
buildAppsUpdateBody(rctx) use remains unchanged.

In `@shortcuts/apps/html_publish_tarball_test.go`:
- Around line 105-128: The test TestWriteHTMLPublishTarEntry_CopyFailure should
avoid chmod-based flakes and instead inject a deterministic failing reader:
modify the test to use a stub implementation of fileio.FileIO/fileio.File (the
same interfaces used by newTestFIO()) whose Open returns a File whose Read (or
ReadFrom/ReadAt used by io.Copy) returns a specific error; call
writeHTMLPublishTarEntry with that stubbed FIO and the same htmlPublishCandidate
(RelPath/AbsPath/Size) and assert the returned error contains the expected
message (e.g., contains "read" or your stub error), removing the
os.Chmod/permission manipulation and relying on the injected failing File to
trigger the io.Copy error path.

In `@shortcuts/apps/walk_html_publish_candidates.go`:
- Around line 43-58: The current loop appends any non-directory entry (including
symlinks) to out as an htmlPublishCandidate; update the check before appending
to only include regular files by using info.Mode().IsRegular() and explicitly
skip entries where info.Mode()&os.ModeSymlink != 0 (or use ModeSymlink) so
symlinks and other non-regular entries are not added; ensure this check is
applied right before creating htmlPublishCandidate (which sets RelPath, AbsPath,
Size) using the existing rel and info values.

In `@skills/lark-apps/references/lark-apps-access-scope-set.md`:
- Line 30: The table cell JSON example {"type":"user|department|chat",
"id":"<id>"} contains unescaped pipe characters which break Markdown table
rendering; update that cell to escape the pipes (e.g., replace
user|department|chat with user\|department\|chat or use the HTML entity &`#124`;
for the pipes) so the table parses correctly while keeping the rest of the JSON
unchanged.

In `@skills/lark-apps/references/lark-apps-html-publish.md`:
- Line 109: The example command "lark-cli apps +html-publish --path ./dist
--dry-run" is missing the required --app-id flag; update the example to include
--app-id (e.g. --app-id <your-app-id>) so the documented dry-run command
validates and runs correctly, and ensure the text around it notes that --app-id
is mandatory.

In `@tests/cli_e2e/apps/apps_access_scope_get_dryrun_test.go`:
- Line 55: The test currently asserts the validation message only against
result.Stderr (assert.Contains(t, result.Stderr, ...)) which is brittle; update
the assertion to check the combined output (result.Stdout + result.Stderr)
instead and ensure the test also asserts a non-zero exit code for the dry-run
validation failure, referencing the existing assert.Contains usage and the
result.Stdout/result.Stderr variables to locate where to change the check.

In `@tests/cli_e2e/apps/apps_access_scope_set_dryrun_test.go`:
- Line 133: The assertions currently inspect only result.Stderr using
gjson.Get(result.Stderr, "error.message"), which is fragile for dry-run
Validate-stage failures; update these to parse the combined output by using
gjson.Get(result.Stdout+result.Stderr, "error.message") (replace occurrences of
gjson.Get(result.Stderr, ...) in this file), and make the same change at the
other mentioned assertion sites (around the instances flagged at lines 152, 171,
191) so the tests assert against result.Stdout+result.Stderr and still check for
the "--targets is required" message and non-zero exit.

In `@tests/cli_e2e/apps/apps_create_dryrun_test.go`:
- Around line 91-167: The three subtests ("RejectsBlankName",
"RejectsInvalidAppType", "RejectsLowercaseAppType") are asserting only non-zero
exit and reading the validation envelope from result.Stderr; update each to
assert exit code == 2 and read the structured validation error from
result.Stdout (use gjson.Get(result.Stdout, "error.message").String()), falling
back to result.Stderr only if Stdout is empty, keeping the same assertion on the
message content; modify the assertions that currently use result.Stderr and
assert.NotEqual(t, 0, result.ExitCode) to use result.Stdout and assert.Equal(t,
2, result.ExitCode) when appropriate.

In `@tests/cli_e2e/apps/apps_html_publish_dryrun_test.go`:
- Around line 183-185: Update the dry-run validation assertions to match repo
convention: replace the non-zero exit assertion with assert.Equal(t, 2,
result.ExitCode) and check the combined output string by asserting the error
text is in result.Stdout+result.Stderr instead of only result.Stderr; apply the
same change for the other occurrence in the same test file that validates
required-flag failure (uses variables result, result.ExitCode, result.Stdout and
result.Stderr).

In `@tests/cli_e2e/apps/apps_update_dryrun_test.go`:
- Around line 83-99: The test in t.Run("RejectsNoFields") reads the validation
envelope from result.Stderr and only asserts a non-zero exit code; change it to
assert the dry-run validation convention by expecting result.ExitCode == 2 and
parse the validation JSON from result.Stdout (falling back to result.Stderr only
if stdout is empty) when extracting error.message with gjson.Get; update the
assertions in this test (function name RejectsNoFields, variables result and
msg) accordingly.

In `@tests/cli_e2e/apps/coverage.md`:
- Around line 18-21: Add one empty line before the "## Command Table" table
block and one empty line after the table block in the markdown (the table
beginning with the header row "| Status | Cmd | Type | Testcase | Key parameter
shapes | Notes / uncovered reason |") so the markdown has a blank line
separating the table from surrounding content to satisfy MD058.

---

Nitpick comments:
In `@shortcuts/apps/apps_html_publish_test.go`:
- Around line 31-35: The test helper writeAppsSampleSite currently ignores the
os.WriteFile error; update it to capture the returned error from os.WriteFile
and fail the test on error (e.g., if err != nil { t.Fatalf("write fixture: %v",
err) }) so setup failures surface; apply the same change to the other fixture
writes flagged in this file (the other os.WriteFile calls around the same
helper/test helpers) to consistently assert write errors.
🪄 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: a492d77c-8603-4bbd-98e4-49ebbf6af38d

📥 Commits

Reviewing files that changed from the base of the PR and between 3bab9a0 and 95b4dbc.

📒 Files selected for processing (44)
  • cmd/_apps_dev_tools/inject_scopes/main.go
  • internal/cmdutil/factory_default.go
  • internal/core/types.go
  • internal/core/types_test.go
  • scripts/apps-mock.py
  • scripts/apps-test-env.sh
  • shortcuts/apps/apps_access_scope_get.go
  • shortcuts/apps/apps_access_scope_get_test.go
  • shortcuts/apps/apps_access_scope_set.go
  • shortcuts/apps/apps_access_scope_set_test.go
  • shortcuts/apps/apps_create.go
  • shortcuts/apps/apps_create_test.go
  • shortcuts/apps/apps_html_publish.go
  • shortcuts/apps/apps_html_publish_test.go
  • shortcuts/apps/apps_list.go
  • shortcuts/apps/apps_list_test.go
  • shortcuts/apps/apps_update.go
  • shortcuts/apps/apps_update_test.go
  • shortcuts/apps/common.go
  • shortcuts/apps/html_publish_client.go
  • shortcuts/apps/html_publish_client_test.go
  • shortcuts/apps/html_publish_tarball.go
  • shortcuts/apps/html_publish_tarball_test.go
  • shortcuts/apps/shortcuts.go
  • shortcuts/apps/shortcuts_test.go
  • shortcuts/apps/walk_html_publish_candidates.go
  • shortcuts/apps/walk_html_publish_candidates_test.go
  • shortcuts/register.go
  • skill-template/domains/apps.md
  • skills/lark-apps/SKILL.md
  • skills/lark-apps/references/lark-apps-access-scope-get.md
  • skills/lark-apps/references/lark-apps-access-scope-set.md
  • skills/lark-apps/references/lark-apps-create.md
  • skills/lark-apps/references/lark-apps-html-publish.md
  • skills/lark-apps/references/lark-apps-list.md
  • skills/lark-apps/references/lark-apps-update.md
  • tests/cli_e2e/apps/apps_access_scope_get_dryrun_test.go
  • tests/cli_e2e/apps/apps_access_scope_set_dryrun_test.go
  • tests/cli_e2e/apps/apps_create_dryrun_test.go
  • tests/cli_e2e/apps/apps_html_publish_dryrun_test.go
  • tests/cli_e2e/apps/apps_list_dryrun_test.go
  • tests/cli_e2e/apps/apps_update_dryrun_test.go
  • tests/cli_e2e/apps/coverage.md
  • tests/cli_e2e/apps/helpers_test.go

Comment thread cmd/_apps_dev_tools/inject_scopes/main.go Outdated
Comment thread scripts/apps-mock.py Outdated
Comment thread shortcuts/apps/apps_access_scope_set.go
Comment thread shortcuts/apps/apps_update.go Outdated
Comment thread shortcuts/apps/html_publish_tarball_test.go
Comment thread tests/cli_e2e/apps/apps_access_scope_set_dryrun_test.go Outdated
Comment thread tests/cli_e2e/apps/apps_create_dryrun_test.go
Comment thread tests/cli_e2e/apps/apps_html_publish_dryrun_test.go Outdated
Comment thread tests/cli_e2e/apps/apps_update_dryrun_test.go
Comment thread tests/cli_e2e/apps/coverage.md
raistlin042 added a commit that referenced this pull request May 21, 2026
CI failures
-----------
- license-header: add SPDX/Apache header to the two new files that were
  missing it (scripts/apps-mock.py, cmd/_apps_dev_tools/inject_scopes/main.go).
- security/gitleaks: the cursor example in lark-apps-list.md tripped the
  generic-api-key rule because its base64-ish value had entropy > 4.3.
  Swap for a low-entropy placeholder `cursor_next_xxx`; same content, no
  fake-secret signal.

Real bugs surfaced in review
----------------------------
- apps +access-scope-set: --scope=public was silently dropping --approver
  if the user passed it. Add the missing mutex check so Validate rejects
  it consistently with the other public-mode flag conflicts.
- apps +update: --app-id was URL-encoded without TrimSpace, so a value
  like " app_x " would land in the request path with literal spaces. The
  rest of the apps shortcuts already trim; bring update in line.
- walker: only accept regular files. Symlinks, devices, sockets are
  skipped — symlink-not-followed was already the design intent (avoid
  loops + out-of-root escapes) but the walker was silently appending the
  symlink entry itself, which the downstream fio.Open would reject
  later. Filter at walk time and update the symlink test to assert the
  link is absent from the manifest.
- inject_scopes dev tool: status messages went to stdout, violating the
  AGENTS.md "stdout=data" contract. Redirect to stderr so a future
  caller can rely on stdout being silent.

Style + docs nits
-----------------
- apps-mock.py: split Ruff E702 semicolon chains in the response writer.
- lark-apps-access-scope-set.md: escape pipes in the targets JSON cell
  so the markdown table renders.
- lark-apps-html-publish.md: example command was missing required
  --app-id flag.
- tests/cli_e2e/apps/coverage.md: MD058 (blank line around table).

Test improvements
-----------------
- TestWriteHTMLPublishTarEntry_CopyFailure no longer uses chmod 0o000
  (flaky as root / on some filesystems). Inject a fileio.File stub
  whose Read returns a synthetic error so the io.Copy failure branch
  fires deterministically.
- e2e dryrun JSON-envelope assertions now read result.Stdout + result.Stderr
  instead of only Stderr. Different shortcut families in the repo land
  the dry-run validation envelope on different channels (markdown /
  drive_search write to stdout + exit 0; apps writes to stderr + exit
  non-zero). Concatenating lets the assertion succeed in either contract
  so the tests don't accidentally pin the runner-internal choice. Plain
  cobra `required flag(s)` text checks stay on Stderr — that one always
  goes to stderr.
raistlin042 and others added 21 commits May 21, 2026 11:18
Adds the apps package with AppsCreate shortcut (POST /open-apis/miaoda/v1/apps)
and reusable test helpers (newAppsExecuteFactory/runAppsShortcut) for Tasks 2.2–2.4/3.4.
Implements PUT /apps/{appId}/access-scope with three scope branches
(specific/public/tenant), mutual-exclusion validation, and JSON target
type enforcement. 7 new tests all pass.
Implements appsHTMLPublishClient interface and appsHTMLPublishAPI that
POSTs multipart/form-data to upload_and_release_html_code with field
name 'file', parses the JSON envelope, and maps business error codes
to hinted ExitErrors.
Wires walker + tarball + HTTP client into runHTMLPublish; adds 20MB size
precheck via mutable package var maxHTMLPublishTarballBytes; defer cleanup
is registered before size check so reject path never leaks temp files.
- skills/lark-apps/SKILL.md: handwritten for Agent discovery
  (gen-skills tool not in this repo yet)
- docs/dev/miaoda-local-dev.md: bulk-replace lark-miaoda → lark-apps,
  shortcuts/miaoda → shortcuts/apps, +publish-html → +html-publish.
  Preserved: scope strings (miaoda:app:*), URL prefix (/open-apis/miaoda/v1/*),
  BOE env names (boe_miaoda_*), pkg names (miaoda-qa-*), brand mention.
BOE 端目前以 spark 系列注册:
- OAPI base path: /open-apis/spark/v1 (replaces /miaoda/v1)
- Scope: spark:app.table:write (replaces miaoda:app:write / readonly / deploy)

Reverts pending: 待后端 /open-apis/miaoda/v1 + miaoda:app:* 注册稳定后切回。
Affected: 5 shortcuts (apps_create / apps_update / apps_list /
apps_access_scope_set / apps_html_publish) + apiBasePath constant.
之前用 spark:app.table:write 作为临时 placeholder(绕过 client 端 scope check)。
后端 BOE 现在正式注册了 5 个 scope,按 shortcut 职责更新:

  +create / +update          → spark:app:write
  +list                       → spark:app:read
  +access-scope-set           → spark:app.access_scope:write
  +html-publish               → spark:app:publish

意义:让客户端预检 + auth login hint 指向真实可授权的 scope,避免
"明明授权了还报 missing_scope" 的混淆体验。
- broaden description to cover "用 HTML 开发 PPT / 可演示 demo / 部署成
  可分享网站" phrasing so the skill fires when users describe building
  HTML presentations or asking lark-cli to deploy a folder
- add end-to-end workflow table (create -> publish -> optional
  access-scope) so the agent finishes the publish chain instead of
  stopping at "HTML 写好了"
- default to apps +create on a missing app_id; only fall back to +list
  when the user explicitly references an existing app
- realign overall structure with skills/lark-doc/SKILL.md (常用示例 /
  前置条件 / 快速决策 / Shortcuts) and drop the unused version field
…ot explicit publish

Split the publish decision into two intent buckets:

- Explicit publish/share ("部署 ./xxx", "发布到妙搭", "部署成可分享的
  网站"): keep the auto-publish behavior, no prompt.
- Presentation-only ("写一个可演示的 PPT", "做个 demo", no mention of
  deploy / share / URL): emit the local HTML first, then ASK the user
  whether to publish before invoking apps +html-publish.

Rewrite the end-to-end workflow as a two-step decision: (1) classify the
user's intent against the new lookup table; (2) only run the publish
chain after intent is confirmed. Mirror the same split in 快速决策 and
in the Shortcuts table's +html-publish row.
…plit arrays)

后端可用范围接口契约升级:
- scope: string "specific/public/tenant" → int 枚举(1=All / 2=Tenant / 3=Range)
- targets: 统一数组 [{type,id}] → 拆 users / departments / chats 三个并列字符串数组

CLI 用户接口保持稳定(仍写 --scope specific --targets '[{type:user,id:xxx}]'),
buildAccessScopeBody 内部做映射 + 分组,对 Agent / 用户透明。

同时补 commit 3355a3a 的遗留:测试文件里硬编码的 stub URL 从
/open-apis/miaoda/v1 → /open-apis/spark/v1 同步(之前没显式跑单测发现)。
妙搭服务端用 index.html 作为应用入口;目录形态如果缺 index.html
真发布会 404 / 没首页。在 runHTMLPublish 里加 ensureIndexHTMLForDir 预检:
- 目录形态 + 根目录缺 index.html → ErrWithHint(type=validation) 拦截
- 单文件形态(用户已明示入口)不做此校验

新增 3 个单测:
- DirRequiresIndexHTML: 目录缺 index → 拦
- DirWithIndexHTMLPasses: 目录含 index → 放行
- SingleFileSkipsIndexCheck: 单文件不强求

reference 同步更新 --path 行说明。
之前 ensureIndexHTMLForDir 只校验目录形态、单文件放行。改为统一约束:
不论目录还是单文件,walker 抓到的 candidates 中必须存在 RelPath == 'index.html'。

- 目录形态:根目录下必须有 index.html
- 单文件形态:文件名必须就是 index.html

helper 重命名 ensureIndexHTMLForDir → ensureIndexHTML(不再有目录判断)。
错误文案 / hint 同步更新;reference --path 行同步澄清。

测试:
- SingleFileSkipsIndexCheck → SingleFileRejectedIfNotNamedIndex(反转语义)
- 新增 SingleFileNamedIndexPasses 覆盖单文件 index.html happy path
- DirRequires/DirWithIndex 不变
`apps +access-scope-set --scope specific --targets '[]'` slipped
through Validate because the JSON-non-empty check only looks at the
string "[]" being non-empty. validateTargetsJSON's for-loop has no
body to enforce length, splitAccessScopeTargets returns three nils,
and the request body comes out as {scope:'Range'} — a 'specific'
scope with zero specific recipients. Semantically meaningless;
server behavior unspecified.

Add a len(items)==0 check at the top of validateTargetsJSON.
bytes.Buffer-based packing (introduced when html-publish moved off
temp files) means highly compressible giant content would balloon
process memory long before the existing post-gzip 20MB tarball
size check fires. 50GB of zeros gzips to a tiny tarball but spends
the buffer's full 50GB en route.

Add maxHTMLPublishRawBytes = 200MB and check sum(candidate.Size)
BEFORE buildHTMLPublishTarball is called. Walker has already
populated c.Size from fs.DirEntry, so the sum is free.
Previously index.html requirement was Execute-only, so dry-run could
pass clean and then the real publish would fail — defeats the
'dry-run = preview' contract. coverage.md misleadingly claimed
'empty dir / missing index.html rejection' was tested at dry-run.

Run ensureIndexHTML in DryRun closure too and surface failures as
envelope 'validation_error' field. Dry-run still exits 0 to match
the repo convention of advisory dry-run; the field lets the caller
(human or agent) see the impending Execute failure.

Rename MissingIndexHTML_PassesDryRun → _SurfacesValidationError to
match the new behavior. coverage.md now accurate.
Mirror of the same fix earlier in +update and +access-scope-get.
validateAccessScopeFlags already trims the input for the Validate
check, but the DryRun and Execute closures were still using the raw
`rctx.Str("app-id")` when building the path. "  app_x  " would
land in the URL as "%20%20app_x%20%20" via EncodePathSegment.

Also threads the buildAccessScopeBody error into a dry-run
'body_error' envelope field so dry-run no longer silently drops
construction failures (Finding 15).
DryRun was using rctx.Str("app-id") and rctx.Str("path") raw while
Execute trimmed both at the top. Whitespace-padded values displayed
literal space escapes in the dry-run URL and caused the path lookup
to fail (file_count=0) while Execute silently corrected them — the
contract 'dry-run previews what Execute will do' was violated. Trim
in DryRun too so the two closures stay symmetric.
writeHTMLPublishTarEntry now rejects RelPath values that look like
tar-slip attempts: absolute paths, parent-directory traversal,
embedded .. components, or null bytes. Walker currently never
produces such values, but a future walker change or an unusual
filesystem could; downstream consumers (server-side extraction)
shouldn't have to be perfectly hardened against malformed input
from this client. 5-line gatekeeper at the contract boundary.
90001 / 90002 magic numbers replaced with errCodeBuildFailed /
errCodeAppNotFound. Docstring on the const block records that the
backend owns these codes.
After 997bdfc set +list to Hidden:true and added a top-of-file
⚠️ banner, the existing 'typical scenarios' section still demoed
agent-facing flows the banner explicitly forbade. Strip the agent
scenarios; keep the file as a human-only command reference (params,
return shape, field semantics) plus a pointer to SKILL.md for the
URL-extraction alternative that agents should use.
Adds an in-source comment that table view (\`--format table\`)
intentionally hides description / icon_url / created_at to keep
rows narrow enough for a terminal. Full fields available via JSON
output. Reader of the source no longer has to wonder whether the
field drop was a bug or a choice.
filepath.WalkDir + filepath.Rel inside rootPath should never produce
a relative path with .. components, but defense in depth: a future
walker logic change or an unusual filesystem layout shouldn't be
able to inject unsafe RelPath values into htmlPublishCandidate.

Extract the existing tarball-side check into isUnsafeRelPath()
(path-component-aware so legitimate filenames containing ".." as a
substring like archive.tar..bak are not false-flagged) and call it
from both writeHTMLPublishTarEntry and the walker callback. The
guard now fires at the boundary where the unsafe value would FIRST
be visible, not just where it would be consumed.
@raistlin042
raistlin042 force-pushed the feat/apps-domain branch 2 times, most recently from 6ef103b to d82e367 Compare May 21, 2026 09:26
- service_descriptions.json: add Apps / 应用 entry with i18n title +
  description so `lark-cli auth login --domain apps` and other
  domain-aware UIs surface a proper label instead of a bare service
  name
- login_messages.go: add "apps" to getShortcutOnlyDomainNames() so the
  shortcut-only domain (no from_meta service spec) is picked up by
  the auth flow's domain discovery
- README.md / README.zh.md: add Apps row at the end of the Features
  table; bump the business-domain count 17→18 and the Skills count
  24→26 to match the actual repo state
- skills/lark-apps/SKILL.md: add the 「身份与一次性授权」section
  pointing agents at `lark-cli auth login --domain apps` so all 5
  apps scopes (spark:app:read/write/publish + access_scope read/write)
  are granted in one browser round-trip instead of prompting on each
  first-time shortcut invocation
@liangshuo-1
liangshuo-1 merged commit 6cea6c9 into main May 21, 2026
37 of 41 checks passed
@liangshuo-1
liangshuo-1 deleted the feat/apps-domain branch May 21, 2026 12:30
@liangshuo-1 liangshuo-1 mentioned this pull request May 21, 2026
3 tasks
tuxedomm pushed a commit to zhumiaoxin/cli that referenced this pull request Jun 6, 2026
…uite#1002)

Adds the apps domain to lark-cli for managing Miaoda (妙搭) applications: 6 shortcuts covering the full lifecycle (+create / +update / +list / +access-scope-set / +access-scope-get / +html-publish). Aligned with the OAPI v2 design — app_type enum (currently HTML), string scope enum (All / Tenant / Range), cursor pagination, in-memory tar.gz multipart publish flow. Namespace registered at /open-apis/spark/v1/ with spark:app.* scopes.

---------

Co-authored-by: wangjiangwen-gif <286006750+wangjiangwen-gif@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XL Architecture-level or global-impact change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants