feat(apps): design_html support, creative-design skill, unified TOS publish#1901
feat(apps): design_html support, creative-design skill, unified TOS publish#1901anngo-nk wants to merge 20 commits into
Conversation
github-actions
Bot
added
domain/ccm
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis change updates app identifier handling, HTML publishing, app initialization, Git credential metadata, and shortcut request headers. It also adds the creative-design skill system, harness references, built-in design guidance, reusable UI frames, animation, deck, canvas, and tweaks components. ChangesMiaoda app workflows
Creative-design guidance
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shortcuts/apps/apps_init_test.go (1)
1765-1804: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winMissing test for the "provided identity" path of
ensureGitIdentity.All three updated tests pass
("", "")for the newname/name/parseCredentialInitEnvelope'sCommitAuthorName/CommitAuthorEmail(see lines 113-125) — is actually written togit configwhen supplied. Since this is the actual behavior change the new parameters exist for, please add a test likeensureGitIdentity(ctx, "/repo", "Real Name", "real@example.com")assertinguser.name/user.emailare set to those values.Want me to draft this test case?
As per path instructions: "Every behavior change must have an adjacent test, and contract tests must assert the new field or behavior directly rather than relying only on happy-path substrings."
🤖 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_init_test.go` around lines 1765 - 1804, Add a test for the provided-identity path of ensureGitIdentity, passing a real name and email while git identity is unset, and assert that git config writes user.name and user.email with those exact supplied values rather than the defaults. Keep the existing default, existing-identity, and failure tests unchanged.Source: Path instructions
🧹 Nitpick comments (1)
skills/lark-apps/creative-design/starter-components/tweaks-panel.jsx (1)
178-248: 🔒 Security & Privacy | 🔵 Trivial | ⚖️ Poor tradeoffShared "miaoda" host-bridge postMessage protocol never specifies/validates an origin. Both starter components send with target origin
'*'and listen for'message'events without checkinge.origin, matching the ast-greppostmessage-permissive-origin(CWE-345) hint. Best practice is to always specify an exact target origin, not * and to only accept postMessages from known and trusted origins. The root cause is the same host-bridge pattern reused verbatim across both files, so fixing it once (e.g. a sharedgetHostOrigin()helper derived fromdocument.referrer/an injected host-origin constant, used for both thepostMessagetarget and anevent.originguard in each listener) addresses every site.
skills/lark-apps/creative-design/starter-components/tweaks-panel.jsx#L178-L248: pass the resolved host origin (not'*') to thepostMessagecalls at lines 187, 241, 247, 580, and add ane.origincheck in theonMsglistener (lines 235-239) before acting onactivate/deactivate.skills/lark-apps/creative-design/starter-components/design-canvas.jsx#L179-L186: pass the resolved host origin to thepostMessagecalls at lines 181, 447, 680, 693, and add ane.origincheck inonHostMsg(lines 669-684) before acting onset-zoom/probe.🤖 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 `@skills/lark-apps/creative-design/starter-components/tweaks-panel.jsx` around lines 178 - 248, The shared host-bridge messaging uses permissive origins and lacks sender validation. In skills/lark-apps/creative-design/starter-components/tweaks-panel.jsx:178-248, add or reuse a resolved host-origin helper for the postMessage calls at lines 187, 241, 247, and 580, and require the onMsg listener to accept only events whose e.origin matches it before handling activate/deactivate. Apply the same origin resolution and validation in skills/lark-apps/creative-design/starter-components/design-canvas.jsx:179-186 for postMessage calls at lines 181, 447, 680, and 693, guarding onHostMsg before set-zoom/probe; replace '*' with the exact trusted origin throughout.Source: Linters/SAST tools
🤖 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 `@shortcuts/apps/apps_html_publish_test.go`:
- Around line 267-268: Update the dry-run contract test around the existing
pre_release assertion to decode the dry-run output and explicitly verify the
pre-release request, TOS upload metadata including the {tos_path} body, and the
release_create endpoint and body. Replace the single substring-only check with
direct assertions for each required request field and preserve clear failure
messages.
In `@shortcuts/apps/apps_html_publish.go`:
- Around line 80-83: Update the dry-run registration in the apps publish flow so
every operation in the described sequence appears as an explicit call: retain
the GET pre_release entry and add placeholder PUT and POST entries for the TOS
upload and release-create steps. Keep the existing endpoint and payload metadata
aligned with the publish sequence so machine consumers can inspect the complete
plan.
In `@shortcuts/apps/apps_init.go`:
- Around line 344-356: Update ensureGitIdentity to return the established typed
validation error when authorName or authorEmail is missing after trimming,
without falling back to defaultGitUserName or defaultGitUserEmail. Ensure the
user.name and user.email writes performed by ensureGitConfigValue use
repository-local Git configuration via --local, rather than accepting unrelated
global values.
- Around line 81-88: Add a "design_html" entry to appTypePolicies using the same
skipInstall, skipEnvPull, skipSkillsSync, and skipAppSync settings as the
existing static HTML policies. Add a focused policy or scaffold test covering
design_html and verifying those initialization steps are skipped.
In `@shortcuts/apps/common.go`:
- Around line 47-50: The validation error message in the appID prefix check must
mention both accepted prefixes. Update the message returned by the appID
validation logic to state that --app-id may start with either "app_" or "cli_",
while leaving the existing validation condition and error handling unchanged.
In `@shortcuts/apps/git_credential.go`:
- Line 471: Update the AppID assignment in the surrounding credential
construction to remove the appID variable from the firstString lookup arguments,
leaving only valid source key names; preserve the existing fallback logic below
for missing app IDs.
In `@shortcuts/common/runner.go`:
- Around line 434-437: Remove the unconditional BOE header setup from
buildRequest, including the boeHeaders construction and larkcore.WithHeaders
append, so shortcut requests no longer target boe_aily_lark_cli. If testing
still requires it, gate the header behind an explicit non-production
configuration switch rather than applying it to every request.
In `@shortcuts/drive/drive_list_comments.go`:
- Around line 204-220: Update parseDriveListCommentsAppsURL to accept only
recognized Miaoda/Lark hosts and require exactly two path components matching
/page/<token>; reject foreign hosts and paths such as /page/token/extra before
returning the token. Add tests covering both rejection cases.
In `@skills/lark-apps/creative-design/starter-components/android-frame.jsx`:
- Line 132: Thread the device’s dark-mode state through the Android keyboard:
update AndroidDevice to pass dark when rendering AndroidKeyboard, and update
AndroidKeyboard to accept that prop and apply dark-aware Material colors
consistently. Preserve the existing light-mode behavior and keyboard rendering
when dark is not enabled.
In `@skills/lark-apps/creative-design/starter-components/deck-stage.js`:
- Line 588: Restrict cross-window messaging in deck-stage by replacing wildcard
targets in every window.parent.postMessage call with the configured trusted host
origin, and update _onMessage to ignore events whose e.origin does not match
that origin before processing commands or exposing state. Reuse the existing
host-origin configuration or symbol rather than introducing a separate trust
value.
---
Outside diff comments:
In `@shortcuts/apps/apps_init_test.go`:
- Around line 1765-1804: Add a test for the provided-identity path of
ensureGitIdentity, passing a real name and email while git identity is unset,
and assert that git config writes user.name and user.email with those exact
supplied values rather than the defaults. Keep the existing default,
existing-identity, and failure tests unchanged.
---
Nitpick comments:
In `@skills/lark-apps/creative-design/starter-components/tweaks-panel.jsx`:
- Around line 178-248: The shared host-bridge messaging uses permissive origins
and lacks sender validation. In
skills/lark-apps/creative-design/starter-components/tweaks-panel.jsx:178-248,
add or reuse a resolved host-origin helper for the postMessage calls at lines
187, 241, 247, and 580, and require the onMsg listener to accept only events
whose e.origin matches it before handling activate/deactivate. Apply the same
origin resolution and validation in
skills/lark-apps/creative-design/starter-components/design-canvas.jsx:179-186
for postMessage calls at lines 181, 447, 680, and 693, guarding onHostMsg before
set-zoom/probe; replace '*' with the exact trusted origin throughout.
🪄 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: d0360041-236a-4194-90dd-792c9434fc60
⛔ Files ignored due to path filters (1)
skills/lark-apps/creative-design/agents/assets/vision-probe.pngis excluded by!**/*.png
📒 Files selected for processing (53)
shortcuts/apps/apps_get.goshortcuts/apps/apps_html_publish.goshortcuts/apps/apps_html_publish_test.goshortcuts/apps/apps_init.goshortcuts/apps/apps_init_test.goshortcuts/apps/apps_meta.goshortcuts/apps/apps_release_create.goshortcuts/apps/apps_release_get.goshortcuts/apps/common.goshortcuts/apps/git_credential.goshortcuts/apps/git_credential_test.goshortcuts/apps/gitcred/helper.goshortcuts/apps/gitcred/types.goshortcuts/common/runner.goshortcuts/drive/drive_list_comments.goshortcuts/drive/drive_list_comments_test.goskills/lark-apps/SKILL.mdskills/lark-apps/creative-design/SKILL.mdskills/lark-apps/creative-design/agents/fork-verifier-agent.mdskills/lark-apps/creative-design/agents/vision-probe-agent.mdskills/lark-apps/creative-design/built-in-skills/animated-video/SKILL.mdskills/lark-apps/creative-design/built-in-skills/charts/SKILL.mdskills/lark-apps/creative-design/built-in-skills/data-report/SKILL.mdskills/lark-apps/creative-design/built-in-skills/frontend-design/SKILL.mdskills/lark-apps/creative-design/built-in-skills/hi-fi-design/SKILL.mdskills/lark-apps/creative-design/built-in-skills/interactive-prototype/SKILL.mdskills/lark-apps/creative-design/built-in-skills/make-a-deck/SKILL.mdskills/lark-apps/creative-design/built-in-skills/visual-exposure/SKILL.mdskills/lark-apps/creative-design/built-in-skills/wireframe/SKILL.mdskills/lark-apps/creative-design/references/aily.mdskills/lark-apps/creative-design/references/claude.mdskills/lark-apps/creative-design/references/codex.mdskills/lark-apps/creative-design/starter-components/android-frame.jsxskills/lark-apps/creative-design/starter-components/animations.jsxskills/lark-apps/creative-design/starter-components/browser-window.jsxskills/lark-apps/creative-design/starter-components/deck-stage.jsskills/lark-apps/creative-design/starter-components/design-canvas.jsxskills/lark-apps/creative-design/starter-components/ios-frame.jsxskills/lark-apps/creative-design/starter-components/macos-window.jsxskills/lark-apps/creative-design/starter-components/tweaks-panel.jsxskills/lark-apps/creative-design/system-prompt.mdskills/lark-apps/references/lark-apps-create.mdskills/lark-apps/references/lark-apps-env-pull.mdskills/lark-apps/references/lark-apps-git-credential.mdskills/lark-apps/references/lark-apps-html-publish.mdskills/lark-apps/references/lark-apps-init.mdskills/lark-apps/references/lark-apps-list.mdskills/lark-apps/references/lark-apps-local-dev.mdskills/lark-apps/references/lark-apps-release-create.mdskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-list-comments.mdtests/cli_e2e/drive/coverage.mdtests/cli_e2e/drive/drive_list_comments_dryrun_test.go
| if !strings.Contains(got, "/open-apis/spark/v1/apps/app_x/pre_release") { | ||
| t.Fatalf("dry-run missing pre_release endpoint: %s", got) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the complete TOS dry-run contract.
This passes even if the release_create step or {tos_path} body disappears. Decode the dry-run output and directly assert the pre-release call, TOS upload metadata, and release-create endpoint/body.
As per coding guidelines, contract tests must assert the new field or behavior directly rather than relying only on happy-path substrings.
🤖 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 267 - 268, Update the
dry-run contract test around the existing pre_release assertion to decode the
dry-run output and explicitly verify the pre-release request, TOS upload
metadata including the {tos_path} body, and the release_create endpoint and
body. Replace the single substring-only check with direct assertions for each
required request field and preserve clear failure messages.
Source: Coding guidelines
| func ensureGitIdentity(ctx context.Context, dir, authorName, authorEmail string) error { | ||
| name := strings.TrimSpace(authorName) | ||
| if name == "" { | ||
| name = defaultGitUserName | ||
| } | ||
| email := strings.TrimSpace(authorEmail) | ||
| if email == "" { | ||
| email = defaultGitUserEmail | ||
| } | ||
| if err := ensureGitConfigValue(ctx, dir, "user.name", name); err != nil { | ||
| return err | ||
| } | ||
| return ensureGitConfigValue(ctx, dir, "user.email", defaultGitUserEmail) | ||
| return ensureGitConfigValue(ctx, dir, "user.email", email) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Apply the credential-provided author identity faithfully and repository-locally.
Missing author fields are silently replaced with bot identity, while git config --get may preserve an unrelated global identity instead of writing the supplied values locally. Return a typed validation error for missing identity and set user.name/user.email with --local.
As per coding guidelines, do not default missing identities or ignore unavailable requested values.
Also applies to: 645-648
🤖 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_init.go` around lines 344 - 356, Update ensureGitIdentity
to return the established typed validation error when authorName or authorEmail
is missing after trimming, without falling back to defaultGitUserName or
defaultGitUserEmail. Ensure the user.name and user.email writes performed by
ensureGitConfigValue use repository-local Git configuration via --local, rather
than accepting unrelated global values.
Source: Coding guidelines
| if !strings.HasPrefix(appID, "app_") && !strings.HasPrefix(appID, "cli_") { | ||
| return errs.NewValidationError(errs.SubtypeInvalidArgument, | ||
| `--app-id must be an app_id starting with "app_".`, | ||
| ).WithParam("--app-id").WithHint( |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Include the cli_ prefix in the error message.
The validation logic accepts both app_ and cli_ prefixes, but the validation error message only mentions app_. Update the message to accurately reflect the allowed prefixes to avoid confusing users who provide a valid cli_ identifier.
💬 Proposed fix
if !strings.HasPrefix(appID, "app_") && !strings.HasPrefix(appID, "cli_") {
return errs.NewValidationError(errs.SubtypeInvalidArgument,
- `--app-id must be an app_id starting with "app_".`,
+ `--app-id must start with "app_" or "cli_".`,
).WithParam("--app-id").WithHint(📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if !strings.HasPrefix(appID, "app_") && !strings.HasPrefix(appID, "cli_") { | |
| return errs.NewValidationError(errs.SubtypeInvalidArgument, | |
| `--app-id must be an app_id starting with "app_".`, | |
| ).WithParam("--app-id").WithHint( | |
| if !strings.HasPrefix(appID, "app_") && !strings.HasPrefix(appID, "cli_") { | |
| return errs.NewValidationError(errs.SubtypeInvalidArgument, | |
| `--app-id must start with "app_" or "cli_".`, | |
| ).WithParam("--app-id").WithHint( |
🤖 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/common.go` around lines 47 - 50, The validation error message
in the appID prefix check must mention both accepted prefixes. Update the
message returned by the appID validation logic to state that --app-id may start
with either "app_" or "cli_", while leaving the existing validation condition
and error handling unchanged.
| Username: firstString(source, "username"), | ||
| PAT: firstString(source, "token", "Token", "pat", "password"), | ||
| ExpiresAt: firstInt64(source, "expiredTime", "ExpiredTime", "expired_time", "expires_at"), | ||
| AppID: firstString(source, "app_id", appID), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Remove the appID value from the list of lookup keys.
The appID variable holds the actual app ID value (e.g., "cli_a123"), but firstString expects a list of key names to search for in the map. Passing appID here causes the code to mistakenly look for a key named after the app's value (e.g., source["cli_a123"]).
Since a correct fallback for a missing app ID is already implemented right below at lines 479-481, you can safely remove appID from the firstString arguments.
🐛 Proposed fix
- AppID: firstString(source, "app_id", appID),
+ AppID: firstString(source, "app_id"),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| AppID: firstString(source, "app_id", appID), | |
| AppID: firstString(source, "app_id"), |
🤖 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/git_credential.go` at line 471, Update the AppID assignment in
the surrounding credential construction to remove the appID variable from the
firstString lookup arguments, leaving only valid source key names; preserve the
existing fallback logic below for missing app IDs.
| // TODO: remove BOE header once testing is complete and promoted to production. | ||
| boeHeaders := http.Header{} | ||
| boeHeaders.Set("x-tt-env", "boe_aily_lark_cli") | ||
| req.ExtraOpts = append(req.ExtraOpts, larkcore.WithHeaders(boeHeaders)) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C3 'x-tt-env|boe_aily_lark_cli|buildRequest\s*\(' --type=go .Repository: larksuite/cli
Length of output: 2712
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Matches for BOE/header usage ==\n'
rg -n -C2 'x-tt-env|boe_aily_lark_cli|WithHeaders\(|buildRequest\s*\(' --type=go .
printf '\n== runner.go slice around buildRequest ==\n'
sed -n '390,455p' shortcuts/common/runner.go
printf '\n== File references to environment/config gating ==\n'
rg -n -C2 'prod|production|boe|env|environment|test-only|temporary|config' shortcuts/common --type=goRepository: larksuite/cli
Length of output: 39146
Remove the BOE header from shortcut requests. buildRequest is shared by all shortcut API paths, so this sends every call to boe_aily_lark_cli. Gate it behind an explicit non-production switch or delete it before merge.
🤖 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/common/runner.go` around lines 434 - 437, Remove the unconditional
BOE header setup from buildRequest, including the boeHeaders construction and
larkcore.WithHeaders append, so shortcut requests no longer target
boe_aily_lark_cli. If testing still requires it, gate the header behind an
explicit non-production configuration switch rather than applying it to every
request.
| func parseDriveListCommentsAppsURL(rawURL string) (string, bool) { | ||
| u, err := url.Parse(strings.TrimSpace(rawURL)) | ||
| if err != nil || u.Scheme == "" || u.Host == "" { | ||
| return "", false | ||
| } | ||
|
|
||
| path := strings.Trim(u.Path, "/") | ||
| parts := strings.Split(path, "/") | ||
| if len(parts) < 2 || parts[0] != "page" { | ||
| return "", false | ||
| } | ||
|
|
||
| token := strings.TrimSpace(parts[1]) | ||
| if token == "" { | ||
| return "", false | ||
| } | ||
| return token, true |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Restrict apps parsing to recognized hosts and an exact /page/<token> path.
The current parser accepts any absolute URL and silently ignores trailing segments. Validate the Miaoda/Lark host allowlist and require exactly two path components before returning an apps token. Add rejection tests for foreign hosts and /page/token/extra.
As per coding guidelines, unsupported input must not be silently coerced into a supported value.
🤖 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/drive/drive_list_comments.go` around lines 204 - 220, Update
parseDriveListCommentsAppsURL to accept only recognized Miaoda/Lark hosts and
require exactly two path components matching /page/<token>; reject foreign hosts
and paths such as /page/token/extra before returning the token. Add tests
covering both rejection cases.
Source: Coding guidelines
| <div style={{ flex: 1, overflow: 'auto' }}> | ||
| {children} | ||
| </div> | ||
| {keyboard && <AndroidKeyboard />} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
AndroidKeyboard doesn't support dark mode, unlike its iOS counterpart.
AndroidDevice renders <AndroidKeyboard /> without forwarding dark, and AndroidKeyboard() accepts no props — it always uses light Material colors regardless of the device's dark mode. ios-frame.jsx's IOSKeyboard correctly threads dark through, so this looks like an oversight rather than a deliberate omission, and the usage header advertises dark+keyboard as a supported combination.
🎨 Proposed fix to thread `dark` through the keyboard
-function AndroidKeyboard() {
+function AndroidKeyboard({ dark = false }) {
let _k = 0;
const key = (l, { flex = 1, bg = MD_C.surface, r = 6, minW, fs = 21 } = {}) => (
<div key={_k++} style={{
height: 46, borderRadius: r, flex, minWidth: minW,
- background: bg, display: 'flex', alignItems: 'center', justifyContent: 'center',
+ background: dark ? '`#3c3c3e`' : bg, display: 'flex', alignItems: 'center', justifyContent: 'center',
fontFamily: 'Roboto, system-ui', fontSize: fs,
- color: MD_C.onPrimaryContainer,
+ color: dark ? '`#fff`' : MD_C.onPrimaryContainer,
}}>{l}</div>
);
...
return (
<div style={{
- background: MD_C.inverseOnSurface, padding: '0 8px 8px',
+ background: dark ? '`#1d1b20`' : MD_C.inverseOnSurface, padding: '0 8px 8px',
display: 'flex', flexDirection: 'column', gap: 4,
}}>And in AndroidDevice:
- {keyboard && <AndroidKeyboard />}
+ {keyboard && <AndroidKeyboard dark={dark} />}Also applies to: 141-184
🤖 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 `@skills/lark-apps/creative-design/starter-components/android-frame.jsx` at
line 132, Thread the device’s dark-mode state through the Android keyboard:
update AndroidDevice to pass dark when rendering AndroidKeyboard, and update
AndroidKeyboard to accept that prop and apply dark-aware Material colors
consistently. Preserve the existing light-mode behavior and keyboard rendering
when dark is not enabled.
| new Promise((r) => setTimeout(r, 2000)), | ||
| ]).then(reveal, reveal); | ||
| }); | ||
| // Announce to the host that this product IS a deck scene. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant sections of the file and look for any trusted-origin configuration.
FILE="skills/lark-apps/creative-design/starter-components/deck-stage.js"
echo "== File size =="
wc -l "$FILE"
echo
echo "== Around line 588 =="
sed -n '560,620p' "$FILE"
echo
echo "== Around message handlers =="
sed -n '1330,1415p' "$FILE"
echo
echo "== Around emit deck change =="
sed -n '1915,1990p' "$FILE"
echo
echo "== Search for origin-related constants/usages =="
rg -n "postMessage|origin|trusted|ancestor|frame-ancestors|message" "$FILE"Repository: larksuite/cli
Length of output: 12719
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for any documented embedding contract or trusted origin used elsewhere.
rg -n "miaoda:deck|deck-stage|postMessage\\(|e\\.origin|targetOrigin|frame-ancestors|parent origin|trusted origin|origin" \
skills/lark-apps/creative-design -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**'
echo
echo "== Nearby constructor/setup section =="
sed -n '500,560p' skills/lark-apps/creative-design/starter-components/deck-stage.jsRepository: larksuite/cli
Length of output: 16990
Restrict deck-stage cross-window messaging to a trusted origin.
Every window.parent.postMessage(...) here uses '*', and _onMessage accepts commands without checking e.origin. That exposes deck HTML/notes to an unexpected parent and lets an untrusted frame drive presenting/rail/goto state. Pass the known host origin explicitly and reject other origins.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 588-588: Specify origin in postMessage
Context: window.parent.postMessage({ type: 'miaoda:deck:available' }, '*')
Note: [CWE-345] Insufficient Verification of Data Authenticity. Security best practice.
(postmessage-permissive-origin)
🤖 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 `@skills/lark-apps/creative-design/starter-components/deck-stage.js` at line
588, Restrict cross-window messaging in deck-stage by replacing wildcard targets
in every window.parent.postMessage call with the configured trusted host origin,
and update _onMessage to ignore events whose e.origin does not match that origin
before processing commands or exposing state. Reuse the existing host-origin
configuration or symbol rather than introducing a separate trust value.
…entity - Add design_html to appTypePolicies (same as modern_html: skip install/env-pull/skills-sync) - Route +html-publish via policy (useTOSPublish) instead of hardcoded type check - Parse commit_author_name/commit_author_email from +git-credential-init response - Use server-provided author identity for repo-local git config, fallback to defaults - Support meta_token as identifier in +get command - Use envvars.AgentName() for source_agent in +create (reads LARKSUITE_CLI_AGENT_NAME) - Add creative HTML guide reference skeleton and SKILL.md routing entry - Update git-credential skill docs with new output fields
- Remove useTOSPublish policy field, html-publish always uses TOS upload - Add html type to appTypePolicies (skip install/env-pull/skills-sync) - Remove design_html from policies (not yet in use) - Fix git credential dry-run test for new local_effects entry
- Add creative-design skill under lark-apps/ (same level as references/) - Update SKILL.md description with creative design trigger keywords - Add creative design routing in development path selection table - Add --path relative path guidance in html-publish reference - Remove old creative-html-guide skeleton (replaced by creative-design)
Add skipAppSync policy field; html and modern_html skip npx app sync on non-empty repo path since static HTML sites don't need it.
- Merge static HTML and creative-design into one path selection row - Add creative-design intent routing entry before html-publish
- Add html端到端 flow in local-dev.md (create → init → dev → release-create) - Unify publish link source: html and full_stack both use +release-get - Update SKILL.md routing and publish护栏 accordingly
- DryRun shows actual 3-step TOS flow (pre_release → TOS PUT → release-create) - Skill docs: output is release_id, use +release-get to poll for online_url - Remove references to legacy multipart upload and data.url
- Pin miaoda-cli to 0.1.24-alpha.fb2cf0a (revert to @latest before merge) - Add x-tt-env=boe_aily_lark_cli header globally (remove before merge) - html app-type uses --template design-html instead of --app-type (remove before merge)
- Add creative mode (html) link format `https://{tenant}/page/{meta_token}` in publish护栏
- Note dev and publish URLs are the same for creative mode, unlike full_stack
- Add meta_token to app_id resolution with full link format in app_id获取
- Select dev path: html apps now default to local-dev pipeline instead of skipping local/cloud axis - Intent routing: creative-design publishes via local-dev flow instead of +html-publish - Remove +html-publish fallback from local-dev "when not to use" section
…tack Remove full_stack-only wording from init, create, list, env-pull, and release-create references since html apps now share the same local dev and release flow.
- Remove "HTML" as separate dev path; html and full_stack both go through local-dev - Intent routing: read local-dev before creative-design to establish git pipeline first - Mark +html-publish as legacy, redirect to local-dev for creative mode - Split html local-dev into 3 scenarios: first-time, iteration, pre-generated files - git add . instead of selective add to capture all creative-design output files
…ease-get online_url
- Rename "创意设计产物" to "HTML 应用 / 创意模式" to catch both keywords - Same pipeline: read local-dev first, then load creative-design
anngo-nk
force-pushed
the
feat/apps-design
branch
from
6108c74 to
a07222a
Compare
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@5550b67a138d37b5cf5016de3b03cda95b87c1a2🧩 Skill updatenpx skills add larksuite/cli#feat/apps-design -y -g |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1901 +/- ##
==========================================
- Coverage 74.73% 74.72% -0.01%
==========================================
Files 887 887
Lines 92644 92691 +47
==========================================
+ Hits 69235 69265 +30
- Misses 18050 18061 +11
- Partials 5359 5365 +6 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shortcuts/apps/git_credential.go (1)
89-102: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winReplace loose map with a typed struct.
As per coding guidelines, avoid introducing new loose-map code. Modifying the existing
map[string]interface{}to conditionally add more fields continues a loose-map pattern. Instead, define a typed struct withomitemptyJSON tags to represent the payload.♻️ Proposed refactor
- payload := map[string]interface{}{ - "app_id": result.AppID, - "repository_url": result.GitHTTPURL, - "status": initStatus(result), - } - if result.CommitAuthorName != "" { - payload["commit_author_name"] = result.CommitAuthorName - } - if result.CommitAuthorEmail != "" { - payload["commit_author_email"] = result.CommitAuthorEmail - } - if result.ConfigWarning != "" { - payload["git_config_warning"] = result.ConfigWarning - } + type initPayload struct { + AppID string `json:"app_id"` + RepositoryURL string `json:"repository_url"` + Status string `json:"status"` + CommitAuthorName string `json:"commit_author_name,omitempty"` + CommitAuthorEmail string `json:"commit_author_email,omitempty"` + GitConfigWarning string `json:"git_config_warning,omitempty"` + } + payload := initPayload{ + AppID: result.AppID, + RepositoryURL: result.GitHTTPURL, + Status: initStatus(result), + CommitAuthorName: result.CommitAuthorName, + CommitAuthorEmail: result.CommitAuthorEmail, + GitConfigWarning: result.ConfigWarning, + }🤖 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/git_credential.go` around lines 89 - 102, Replace the payload map in the git credential response flow with a dedicated typed struct representing app_id, repository_url, status, commit author name/email, and git config warning. Add JSON omitempty tags for optional fields, populate the struct from result, and preserve the existing conditional omission behavior during serialization.Source: Coding guidelines
♻️ Duplicate comments (1)
shortcuts/apps/git_credential.go (1)
471-471: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRemove the
appIDvalue from the list of lookup keys.The
appIDvariable holds the actual app ID value (e.g.,"cli_a123"), butfirstStringexpects a list of key names to search for in the map. PassingappIDhere causes the code to mistakenly look for a key named after the app's value.Since a correct fallback for a missing app ID is already implemented right below at lines 479-481, you can safely remove
appIDfrom thefirstStringarguments.🐛 Proposed fix
- AppID: firstString(source, "app_id", appID), + AppID: firstString(source, "app_id"),🤖 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/git_credential.go` at line 471, Update the AppID assignment in the surrounding credential construction to remove appID from the firstString lookup keys, leaving only valid source key names. Preserve the existing fallback logic below that handles a missing app ID.
🧹 Nitpick comments (2)
skills/lark-apps/creative-design/starter-components/deck-stage.js (1)
535-546: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo reconnect guard — a second
connectedCallbackwould duplicate the shadow DOM.
_render()unconditionally appends a newstyle/rail/resize/stage/notes/menuset into the existing shadow root without clearing prior content. If the element is ever disconnected and reconnected (moved in the DOM, re-inserted by a host app), this produces two stacked<slot>s, ails, and menus — only the first default slot receives the assigned light-DOM children, so the second stage/rail render empty while listeners/observers double up.♻️ Guard `_render()` against re-invocation
connectedCallback() { ... - this._render(); + if (!this._built) { + this._built = true; + this._render(); + } this._loadNotes();Also applies to: 1016-1016
🤖 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 `@skills/lark-apps/creative-design/starter-components/deck-stage.js` around lines 535 - 546, Guard the rendering lifecycle around _render() so reconnecting the custom element cannot append duplicate shadow-DOM structures. Add an idempotence check using an existing rendered-state marker or the shadow root’s expected children, and skip the append/setup work when already rendered while preserving the initial connectedCallback flow and existing listeners/observers.skills/lark-apps/creative-design/starter-components/design-canvas.jsx (1)
669-685: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winHost-bridge
messagelisteners don't verify the sender. Both starter components registerwindow.addEventListener('message', ...)handlers that act onmiaoda:*protocol messages without checkinge.origin/e.source, so any frame able to reach this window (not just the trusted host) can drive canvas zoom, toggle the Tweaks panel, etc. Root cause is the same missing sender check in both listeners.
skills/lark-apps/creative-design/starter-components/design-canvas.jsx#L669-L685: inonHostMsg, add a guard such asif (e.source !== window.parent) return;before acting onmiaoda:canvas:set-zoom/miaoda:canvas:probe.skills/lark-apps/creative-design/starter-components/tweaks-panel.jsx#L234-L243: inonMsg, add the samee.source !== window.parentguard before acting onmiaoda:tweaks:activate/miaoda:tweaks:deactivate.🤖 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 `@skills/lark-apps/creative-design/starter-components/design-canvas.jsx` around lines 669 - 685, Validate the message sender before processing host-bridge commands. In skills/lark-apps/creative-design/starter-components/design-canvas.jsx lines 669-685, add an e.source !== window.parent guard at the start of onHostMsg; apply the same guard in skills/lark-apps/creative-design/starter-components/tweaks-panel.jsx lines 234-243 within onMsg, before handling the miaoda:tweaks:activate/deactivate messages.Source: Linters/SAST tools
🤖 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 `@skills/lark-apps/creative-design/built-in-skills/frontend-design/SKILL.md`:
- Line 49: Update the CSS specificity guidance in the relevant example so it
contrasts genuinely different selector shapes, replacing the equal-specificity
`.section` versus `.cta` example with a combination such as `.section` and
`section.cta`; preserve the warning about unintended rule overrides.
In `@skills/lark-apps/creative-design/references/claude.md`:
- Around line 22-29: Update the use-design-system.md reference in the
AskUserQuestion section to point to an existing documentation file, or add the
missing document at the referenced location; preserve the intended design-system
guidance and ensure the link resolves correctly.
In `@skills/lark-apps/creative-design/starter-components/animations.jsx`:
- Around line 219-230: Update the ImageSprite component to accept an alt prop
with an empty-string default, then pass that value to the rendered image element
instead of always emitting alt="". Preserve the empty default so callers can
continue marking decorative images appropriately.
- Around line 335-338: Validate the duration option wherever it is initialized
or updated, including the defaults near the animation component and the
corresponding path around the later duration definition, and reject values that
are non-positive or non-finite. Ensure loop playback cannot reach the next %
duration calculation with an invalid duration, while preserving valid positive
finite durations.
In `@skills/lark-apps/creative-design/starter-components/browser-window.jsx`:
- Around line 1-3: Update the usage header comment in browser-window.jsx to
identify the actual starter-component filename instead of Chrome.jsx, ensuring
generated setup instructions reference the existing component file.
In `@skills/lark-apps/creative-design/starter-components/deck-stage.js`:
- Around line 113-115: Re-enable the speaker-notes dock by changing
NOTES_DISABLED in deck-stage.js so _notesVisible() can expose the editable notes
panel and preserve the documented postMessage contract in SKILL.md.
In `@skills/lark-apps/creative-design/starter-components/design-canvas.jsx`:
- Around line 176-221: Add a userEdited ref to the DesignCanvas initialization
and set it at the start of patchSection in the api memo. In the sidecar fetch
resolution, only hydrate state.sections from saved.sections when no local edit
has occurred; otherwise preserve the in-memory edits. Keep the existing
didRead/ready lifecycle and write suppression behavior intact.
---
Outside diff comments:
In `@shortcuts/apps/git_credential.go`:
- Around line 89-102: Replace the payload map in the git credential response
flow with a dedicated typed struct representing app_id, repository_url, status,
commit author name/email, and git config warning. Add JSON omitempty tags for
optional fields, populate the struct from result, and preserve the existing
conditional omission behavior during serialization.
---
Duplicate comments:
In `@shortcuts/apps/git_credential.go`:
- Line 471: Update the AppID assignment in the surrounding credential
construction to remove appID from the firstString lookup keys, leaving only
valid source key names. Preserve the existing fallback logic below that handles
a missing app ID.
---
Nitpick comments:
In `@skills/lark-apps/creative-design/starter-components/deck-stage.js`:
- Around line 535-546: Guard the rendering lifecycle around _render() so
reconnecting the custom element cannot append duplicate shadow-DOM structures.
Add an idempotence check using an existing rendered-state marker or the shadow
root’s expected children, and skip the append/setup work when already rendered
while preserving the initial connectedCallback flow and existing
listeners/observers.
In `@skills/lark-apps/creative-design/starter-components/design-canvas.jsx`:
- Around line 669-685: Validate the message sender before processing host-bridge
commands. In
skills/lark-apps/creative-design/starter-components/design-canvas.jsx lines
669-685, add an e.source !== window.parent guard at the start of onHostMsg;
apply the same guard in
skills/lark-apps/creative-design/starter-components/tweaks-panel.jsx lines
234-243 within onMsg, before handling the miaoda:tweaks:activate/deactivate
messages.
🪄 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: d561b877-b456-482d-9a4c-8aadd44eadcd
⛔ Files ignored due to path filters (1)
skills/lark-apps/creative-design/agents/assets/vision-probe.pngis excluded by!**/*.png
📒 Files selected for processing (47)
shortcuts/apps/apps_get.goshortcuts/apps/apps_html_publish.goshortcuts/apps/apps_html_publish_test.goshortcuts/apps/apps_init.goshortcuts/apps/apps_init_test.goshortcuts/apps/apps_meta.goshortcuts/apps/apps_release_create.goshortcuts/apps/apps_release_get.goshortcuts/apps/common.goshortcuts/apps/git_credential.goshortcuts/apps/git_credential_test.goshortcuts/apps/gitcred/helper.goshortcuts/apps/gitcred/types.goshortcuts/common/runner.goskills/lark-apps/SKILL.mdskills/lark-apps/creative-design/SKILL.mdskills/lark-apps/creative-design/agents/fork-verifier-agent.mdskills/lark-apps/creative-design/agents/vision-probe-agent.mdskills/lark-apps/creative-design/built-in-skills/animated-video/SKILL.mdskills/lark-apps/creative-design/built-in-skills/charts/SKILL.mdskills/lark-apps/creative-design/built-in-skills/data-report/SKILL.mdskills/lark-apps/creative-design/built-in-skills/frontend-design/SKILL.mdskills/lark-apps/creative-design/built-in-skills/hi-fi-design/SKILL.mdskills/lark-apps/creative-design/built-in-skills/interactive-prototype/SKILL.mdskills/lark-apps/creative-design/built-in-skills/make-a-deck/SKILL.mdskills/lark-apps/creative-design/built-in-skills/visual-exposure/SKILL.mdskills/lark-apps/creative-design/built-in-skills/wireframe/SKILL.mdskills/lark-apps/creative-design/references/aily.mdskills/lark-apps/creative-design/references/claude.mdskills/lark-apps/creative-design/references/codex.mdskills/lark-apps/creative-design/starter-components/android-frame.jsxskills/lark-apps/creative-design/starter-components/animations.jsxskills/lark-apps/creative-design/starter-components/browser-window.jsxskills/lark-apps/creative-design/starter-components/deck-stage.jsskills/lark-apps/creative-design/starter-components/design-canvas.jsxskills/lark-apps/creative-design/starter-components/ios-frame.jsxskills/lark-apps/creative-design/starter-components/macos-window.jsxskills/lark-apps/creative-design/starter-components/tweaks-panel.jsxskills/lark-apps/creative-design/system-prompt.mdskills/lark-apps/references/lark-apps-create.mdskills/lark-apps/references/lark-apps-env-pull.mdskills/lark-apps/references/lark-apps-git-credential.mdskills/lark-apps/references/lark-apps-html-publish.mdskills/lark-apps/references/lark-apps-init.mdskills/lark-apps/references/lark-apps-list.mdskills/lark-apps/references/lark-apps-local-dev.mdskills/lark-apps/references/lark-apps-release-create.md
🚧 Files skipped from review as they are similar to previous changes (28)
- shortcuts/apps/apps_release_get.go
- skills/lark-apps/creative-design/built-in-skills/interactive-prototype/SKILL.md
- shortcuts/apps/git_credential_test.go
- shortcuts/apps/apps_release_create.go
- skills/lark-apps/creative-design/built-in-skills/wireframe/SKILL.md
- skills/lark-apps/references/lark-apps-list.md
- skills/lark-apps/references/lark-apps-init.md
- skills/lark-apps/references/lark-apps-release-create.md
- shortcuts/apps/apps_meta.go
- skills/lark-apps/creative-design/agents/fork-verifier-agent.md
- skills/lark-apps/creative-design/references/codex.md
- skills/lark-apps/references/lark-apps-git-credential.md
- shortcuts/apps/apps_html_publish_test.go
- skills/lark-apps/references/lark-apps-html-publish.md
- skills/lark-apps/creative-design/built-in-skills/charts/SKILL.md
- shortcuts/apps/apps_html_publish.go
- shortcuts/common/runner.go
- shortcuts/apps/apps_get.go
- skills/lark-apps/creative-design/starter-components/android-frame.jsx
- skills/lark-apps/creative-design/built-in-skills/hi-fi-design/SKILL.md
- shortcuts/apps/gitcred/types.go
- shortcuts/apps/apps_init_test.go
- shortcuts/apps/gitcred/helper.go
- skills/lark-apps/SKILL.md
- skills/lark-apps/creative-design/starter-components/macos-window.jsx
- shortcuts/apps/common.go
- skills/lark-apps/references/lark-apps-local-dev.md
- shortcuts/apps/apps_init.go
|
|
||
| Then review that plan against the brief before building: if any part of it reads like the generic default you would produce for any similar page (work through a similar prompt to see if you arrive somewhere similar) rather than a choice made for this specific brief — revise that part, say what you changed and why. Only after you've confirmed the relative uniqueness of your design plan should you start to write the code, following the revised plan exactly and deriving every color and type decision from it. | ||
|
|
||
| When writing the code, be careful of structuring your CSS selector specificities. It's easy to generate CSS classes that cancel each other out (especially with a type-based selector like .section and a element-based selector like .cta). This can happen often with paddings/margins between sections. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the CSS specificity example.
.section and .cta are both class selectors with equal specificity, so this guidance misdiagnoses how the rules can override each other. Use a genuinely different selector shape, such as .section versus section.cta.
Proposed correction
-When writing the code, be careful of structuring your CSS selector specificities. It's easy to generate CSS classes that cancel each other out (especially with a type-based selector like .section and a element-based selector like .cta).
+When writing the code, structure CSS selector specificity carefully. Broad class selectors can be unexpectedly overridden by compound selectors—for example, `.section` versus `section.cta`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| When writing the code, be careful of structuring your CSS selector specificities. It's easy to generate CSS classes that cancel each other out (especially with a type-based selector like .section and a element-based selector like .cta). This can happen often with paddings/margins between sections. | |
| When writing the code, structure CSS selector specificity carefully. Broad class selectors can be unexpectedly overridden by compound selectors—for example, `.section` versus `section.cta`. |
🧰 Tools
🪛 LanguageTool
[style] ~49-~49: The adverb ‘often’ is usually put before the verb ‘happen’.
Context: ...ent-based selector like .cta). This can happen often with paddings/margins between sections....
(ADVERB_WORD_ORDER)
🪛 SkillSpector (2.3.11)
[error] 65: [AR2] Anti-Refusal Statement: Skill instructs the agent to omit warnings, disclaimers, or ethical commentary. Stripping safety caveats hides risk from the user and is a common jailbreak preamble.
Remediation: Remove instructions that suppress warnings, disclaimers, or ethical commentary. Let the agent surface safety-relevant caveats to the user.
(Anti-Refusal (AR2))
🤖 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 `@skills/lark-apps/creative-design/built-in-skills/frontend-design/SKILL.md` at
line 49, Update the CSS specificity guidance in the relevant example so it
contrasts genuinely different selector shapes, replacing the equal-specificity
`.section` versus `.cta` example with a combination such as `.section` and
`section.cta`; preserve the warning about unintended rule overrides.
| ## AskUserQuestion（澄清性提问） | ||
|
|
||
| 替代 `ask_user_question`。`AskUserQuestion` **把用户的答案内联返回**——先问，等用户答复后再继续。每次调用最多展示 4 个问题；大型新项目先问一轮聚焦的问题，不够就再补一次调用。 | ||
|
|
||
| - 记忆中的偏好可以作为问题里的*建议*默认值给出，但仍须由用户确认。 | ||
| - 优先用它，而不是在回复里用文字列点罗列选项。 | ||
| - 项目设置类提问——项目**保存到哪里**、使用**哪个（哪些）设计系统**（一次 multiSelect；见 [`use-design-system.md`](../built-in-skills/use-design-system.md)）——都是普通的 `AskUserQuestion` 调用。 | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -i 'use-design-system.md'
rg -n 'use-design-system' -g '*.md'Repository: larksuite/cli
Length of output: 151
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== relevant files =='
fd -a 'claude.md|use-design-system.md|built-in-skills' skills || true
echo
echo '== outline claude.md =='
ast-grep outline skills/lark-apps/creative-design/references/claude.md --view expanded || true
echo
echo '== surrounding lines =='
sed -n '1,80p' skills/lark-apps/creative-design/references/claude.md || true
echo
echo '== search for referenced path text =='
rg -n '\.\./built-in-skills/use-design-system\.md|use-design-system\.md|built-in-skills' skills -g '*.md' || trueRepository: larksuite/cli
Length of output: 8832
🏁 Script executed:
#!/bin/bash
set -euo pipefail
pwd
echo '---'
find skills -maxdepth 5 \( -name 'claude.md' -o -name 'use-design-system.md' \) -print | sort
echo '---'
python3 - <<'PY'
from pathlib import Path
p = Path('skills/lark-apps/creative-design/references/claude.md')
print('claude exists:', p.exists())
print('target from relative link:', (p.parent / '../built-in-skills/use-design-system.md').resolve())
print('target exists:', (p.parent / '../built-in-skills/use-design-system.md').exists())
PYRepository: larksuite/cli
Length of output: 389
Fix the use-design-system.md link skills/lark-apps/creative-design/references/claude.md:28 points to ../built-in-skills/use-design-system.md, but that file does not exist under skills/lark-apps/creative-design/built-in-skills/. Update the reference to a real doc or add the missing file.
🤖 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 `@skills/lark-apps/creative-design/references/claude.md` around lines 22 - 29,
Update the use-design-system.md reference in the AskUserQuestion section to
point to an existing documentation file, or add the missing document at the
referenced location; preserve the intended design-system guidance and ensure the
link resolves correctly.
| function ImageSprite({ | ||
| src, | ||
| x = 0, y = 0, | ||
| width = 400, height = 300, | ||
| entryDur = 0.6, | ||
| exitDur = 0.4, | ||
| kenBurns = false, | ||
| kenBurnsScale = 1.08, | ||
| radius = 12, | ||
| fit = 'cover', | ||
| placeholder = null, // {label: string} for striped placeholder | ||
| }) { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Allow callers to provide image alternative text.
ImageSprite always emits alt="", so meaningful images are unconditionally hidden from screen readers. Expose an alt prop while retaining an empty default for decorative images.
Proposed fix
function ImageSprite({
src,
+ alt = '',
x = 0, y = 0,
@@
- <img src={src} alt="" style={{ width: '100%', height: '100%', objectFit: fit, display: 'block' }} />
+ <img src={src} alt={alt} style={{ width: '100%', height: '100%', objectFit: fit, display: 'block' }} />Also applies to: 265-265
🤖 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 `@skills/lark-apps/creative-design/starter-components/animations.jsx` around
lines 219 - 230, Update the ImageSprite component to accept an alt prop with an
empty-string default, then pass that value to the rendered image element instead
of always emitting alt="". Preserve the empty default so callers can continue
marking decorative images appropriately.
| duration = 10, | ||
| background = '#f6f4ef', | ||
| fps = 60, | ||
| loop = true, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject non-positive or non-finite durations.
With loop enabled, duration={0} executes next % duration, making the playhead NaN and producing invalid progress styles and persisted state.
Proposed validation
function Stage({
width = 1280,
height = 720,
duration = 10,
@@
children,
}) {
+ if (!Number.isFinite(duration) || duration <= 0) {
+ throw new TypeError('Stage duration must be a positive finite number');
+ }Also applies to: 402-406
🤖 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 `@skills/lark-apps/creative-design/starter-components/animations.jsx` around
lines 335 - 338, Validate the duration option wherever it is initialized or
updated, including the defaults near the animation component and the
corresponding path around the later duration definition, and reject values that
are non-positive or non-finite. Ensure loop playback cannot reach the next %
duration calculation with an invalid duration, while preserving valid positive
finite durations.
| /* BEGIN USAGE */ | ||
| // Chrome.jsx — Simplified Chrome browser window (dark theme, macOS) | ||
| // No dependencies, no image assets. All inline styles + inline SVG. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the actual starter-component filename in the usage header.
Naming this file Chrome.jsx can cause generated setup instructions to reference a component that does not exist.
-// Chrome.jsx — Simplified Chrome browser window (dark theme, macOS)
+// browser-window.jsx — Simplified Chrome browser window (dark theme, macOS)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /* BEGIN USAGE */ | |
| // Chrome.jsx — Simplified Chrome browser window (dark theme, macOS) | |
| // No dependencies, no image assets. All inline styles + inline SVG. | |
| /* BEGIN USAGE */ | |
| // browser-window.jsx — Simplified Chrome browser window (dark theme, macOS) | |
| // No dependencies, no image assets. All inline styles + inline SVG. |
🤖 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 `@skills/lark-apps/creative-design/starter-components/browser-window.jsx`
around lines 1 - 3, Update the usage header comment in browser-window.jsx to
identify the actual starter-component filename instead of Chrome.jsx, ensuring
generated setup instructions reference the existing component file.
| // TEMP: presenter-notes dock is hidden for now. Flip back to false to restore | ||
| // the editable speaker-notes panel (all notes logic below stays intact). | ||
| const NOTES_DISABLED = true; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Repo files of interest:\n'
git ls-files 'skills/lark-apps/creative-design/starter-components/deck-stage.js' 'skills/**/SKILL.md' | sed 's#^`#-` #'
printf '\nOutline of deck-stage.js:\n'
ast-grep outline skills/lark-apps/creative-design/starter-components/deck-stage.js --view expanded || true
printf '\nSearch for speaker-notes / postMessage references:\n'
rg -n "speaker-notes|postMessage|NOTES_DISABLED|_notesVisible|presenter-notes|deck-stage" skills/lark-apps/creative-design -S || trueRepository: larksuite/cli
Length of output: 13969
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the relevant section of deck-stage.js with line numbers.
sed -n '90,150p' skills/lark-apps/creative-design/starter-components/deck-stage.js | cat -n
printf '\nRelevant SKILL.md files:\n'
fd -a 'SKILL.md' skills/lark-apps/creative-design | sed 's#^`#-` #'
printf '\nSearch for "speaker-notes" in SKILL.md files:\n'
rg -n "speaker-notes|notes" skills/lark-apps/creative-design -g 'SKILL.md' -S || trueRepository: larksuite/cli
Length of output: 5500
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the notes visibility / panel logic around the referenced lines.
sed -n '1064,1102p' skills/lark-apps/creative-design/starter-components/deck-stage.js | cat -n
printf '\nShow the speaker-notes contract in make-a-deck SKILL.md:\n'
sed -n '18,34p' skills/lark-apps/creative-design/built-in-skills/make-a-deck/SKILL.md | cat -nRepository: larksuite/cli
Length of output: 3026
Speaker-notes should not be hard-disabled. NOTES_DISABLED makes _notesVisible() always return false, so the editable dock never appears and the documented speaker-notes postMessage contract in skills/lark-apps/creative-design/built-in-skills/make-a-deck/SKILL.md is unavailable. Re-enable it for this release or update the skill doc to match the disabled state.
🤖 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 `@skills/lark-apps/creative-design/starter-components/deck-stage.js` around
lines 113 - 115, Re-enable the speaker-notes dock by changing NOTES_DISABLED in
deck-stage.js so _notesVisible() can expose the editable notes panel and
preserve the documented postMessage contract in SKILL.md.
| const DC_STATE_FILE = '.design-canvas.state.json'; | ||
|
|
||
| // Persist a sidecar file back to the host. | ||
| function miaodaWriteFile(path, content) { | ||
| try { | ||
| window.parent.postMessage({ type: 'miaoda:bridge:write-file', path: path, content: content }, '*'); | ||
| } catch (e) { | ||
| /* no parent — ignore */ | ||
| } | ||
| return Promise.resolve(); | ||
| } | ||
|
|
||
| function DesignCanvas({ children, minScale, maxScale, style }) { | ||
| const [state, setState] = React.useState({ sections: {}, focus: null }); | ||
| // Hold rendering until the sidecar read settles so the saved order/titles | ||
| // appear on first paint (no source-order flash). didRead gates writes until | ||
| // the read settles so the empty initial state can't clobber a slow read; | ||
| // skipNextWrite suppresses the one echo-write that would otherwise follow | ||
| // hydration. | ||
| const [ready, setReady] = React.useState(false); | ||
| const didRead = React.useRef(false); | ||
| const skipNextWrite = React.useRef(false); | ||
|
|
||
| React.useEffect(() => { | ||
| let off = false; | ||
| fetch('./' + DC_STATE_FILE) | ||
| .then((r) => (r.ok ? r.json() : null)) | ||
| .then((saved) => { | ||
| if (off || !saved || !saved.sections) return; | ||
| skipNextWrite.current = true; | ||
| setState((s) => ({ ...s, sections: saved.sections })); | ||
| }) | ||
| .catch(() => {}) | ||
| .finally(() => { didRead.current = true; if (!off) setReady(true); }); | ||
| const t = setTimeout(() => { if (!off) setReady(true); }, 150); | ||
| return () => { off = true; clearTimeout(t); }; | ||
| }, []); | ||
|
|
||
| React.useEffect(() => { | ||
| if (!didRead.current) return; | ||
| if (skipNextWrite.current) { skipNextWrite.current = false; return; } | ||
| const t = setTimeout(() => { | ||
| miaodaWriteFile(DC_STATE_FILE, JSON.stringify({ sections: state.sections })).catch(() => {}); | ||
| }, 250); | ||
| return () => clearTimeout(t); | ||
| }, [state.sections]); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Sidecar read can be clobbered by edits made during the 150ms fallback window.
didRead.current is only set once the fetch settles, but ready can also flip true via the independent 150ms timeout (line 210) without didRead becoming true. If the fetch takes longer than 150ms, the canvas renders and becomes interactive before the saved state loads; any patchSection call (rename/reorder/delete) made in that window updates state.sections, but when the fetch later resolves, its handler (lines 203-207) unconditionally replaces state.sections with the on-disk saved.sections, silently discarding that edit. The write-side guard (if (!didRead.current) return;, line 215) only prevents a premature write; it does nothing to protect the in-memory edit from being overwritten by the later read. This also means the "no source-order flash" guarantee in the comment doesn't hold for slow fetches.
🛡️ Proposed fix — don't let a late read clobber local edits
const [ready, setReady] = React.useState(false);
const didRead = React.useRef(false);
const skipNextWrite = React.useRef(false);
+ const userEdited = React.useRef(false);
React.useEffect(() => {
let off = false;
fetch('./' + DC_STATE_FILE)
.then((r) => (r.ok ? r.json() : null))
.then((saved) => {
- if (off || !saved || !saved.sections) return;
+ if (off || !saved || !saved.sections || userEdited.current) return;
skipNextWrite.current = true;
setState((s) => ({ ...s, sections: saved.sections }));
})
.catch(() => {})
.finally(() => { didRead.current = true; if (!off) setReady(true); });
const t = setTimeout(() => { if (!off) setReady(true); }, 150);
return () => { off = true; clearTimeout(t); };
}, []);And set userEdited.current = true at the top of patchSection in the api memo.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 180-180: Specify origin in postMessage
Context: window.parent.postMessage({ type: 'miaoda:bridge:write-file', path: path, content: content }, '*')
Note: [CWE-345] Insufficient Verification of Data Authenticity. Security best practice.
(postmessage-permissive-origin)
🤖 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 `@skills/lark-apps/creative-design/starter-components/design-canvas.jsx` around
lines 176 - 221, Add a userEdited ref to the DesignCanvas initialization and set
it at the start of patchSection in the api memo. In the sidecar fetch
resolution, only hydrate state.sections from saved.sections when no local edit
has occurred; otherwise preserve the in-memory edits. Keep the existing
didRead/ready lifecycle and write suppression behavior intact.
…d guardrails All HTML apps now go through local-dev pipeline. +html-publish is deprecated.
…/cloud-dev pages html-publish is no longer the recommended path for HTML apps; all html and full_stack apps now follow the same local-dev + release-create flow.
2 participants
Summary
design_htmlapp type support with init skip policy (same asmodern_html)creative-designskill underlark-apps/for UI mockup / prototype / deck / visual design workflows+html-publishto always use TOS upload path (remove legacy multipart)htmltype toappTypePolicies(skip install/env-pull/skills-sync/app-sync)commit_author_name/commit_author_emailfrom+git-credential-initfor repo-local git identitymeta_tokenas identifier in+get, addmeta_tokento pretty output+list-commentsapps support (temporary, for testing)TEMP changes (remove before merge)
miaoda-clipinned to0.1.24-alpha.fb2cf0a→ revert to@latestx-tt-env: boe_aily_lark_cliheader → removeTest plan
go test ./shortcuts/apps/ -count=1— all passgo test ./shortcuts/drive/ -run TestResolveDriveListCommentsInput— passmake build— clean buildSummary by CodeRabbit
New Features
Bug Fixes
Documentation