[migrate] upgrade to MobX-Lark 2.8.1, Open-Source-Bazaar/feishu-action@v3 & Vercel CLI 50.x#13
Conversation
…n@v3 & Vercel CLI 50.x [migrate] upgrade to Node.js 24, Undici 8, Globals 17, ESLint 10, File-Type 22 & other latest Upstream packages/actions
There was a problem hiding this comment.
Pull request overview
This PR modernizes the project’s dependency stack and CI/CD tooling (Node 24, newer upstream packages/actions) while adjusting several Lark API routes to align with the MobX-Lark upgrade and updated proxying/deployment behavior.
Changes:
- Upgrade key runtime/dev dependencies (notably
mobx-lark@2.8.1,undici@8,eslint@10,next@16.2.x, etc.). - Update Lark API endpoints: extend document copy behavior (including optional owner transfer), simplify bitable proxy route, and adjust proxied headers.
- Revise GitHub Actions workflows: switch Vercel deployment to Vercel CLI, upgrade actions versions, and migrate Feishu/Lark notification format/action.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pages/api/Lark/document/copy/[...slug].ts | Extends copy endpoint to support more document path types and adds optional ownership transfer. |
| pages/api/Lark/core.ts | Adjusts Lark proxy header handling (drops content-length). |
| pages/api/Lark/bitable/v1/[...slug].ts | Simplifies bitable API routing to a single safe proxy handler. |
| package.json | Updates dependencies/devDependencies to newer versions. |
| .vscode/extensions.json | Updates recommended VS Code extensions list. |
| .github/workflows/main.yml | Migrates Vercel deploy mechanism and updates Lark notification action/payload; bumps action versions. |
| .github/workflows/init-template.yml | Bumps action versions used by template initialization workflow. |
| .github/workflows/deploy-production.yml | Bumps action versions and SCP action major tag. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const copiedFile = | ||
| type === 'wiki' | ||
| ? await lark.copyFile(`${type as 'wiki'}/${id}`, name, parentToken) | ||
| : await lark.copyFile( | ||
| `${type as LarkDocumentPathType}/${id}`, | ||
| name, | ||
| parentToken, | ||
| ); |
There was a problem hiding this comment.
type comes from the URL params but is only narrowed via TypeScript casts before being used to build the Lark path. This allows arbitrary/unsupported values to be forwarded to lark.copyFile(...), which can yield confusing upstream errors. Consider validating type at runtime against an explicit allow-list of supported document types and returning 400 for anything else.
| if (ownerType && ownerId) | ||
| try { | ||
| await lark.driveFileStore.transferOwner(type, newId, { | ||
| member_type: ownerType, | ||
| member_id: ownerId, | ||
| }); |
There was a problem hiding this comment.
This endpoint now accepts ownerType/ownerId from the request body and performs transferOwner(...) using the app's credentials. With only verifyJWT in place, any authenticated user can potentially transfer ownership of newly copied files to arbitrary members. Please add an authorization check (e.g., restrict transfers to the caller, or require an admin role / server-side allow-list) and/or validate ownerType/ownerId against the JWT identity.
| if (ownerType && ownerId) | ||
| try { | ||
| await lark.driveFileStore.transferOwner(type, newId, { | ||
| member_type: ownerType, | ||
| member_id: ownerId, | ||
| }); | ||
| } catch (error) { | ||
| console.error(JSON.stringify(error, null, 2)); | ||
| } | ||
| context.body = copiedFile; |
There was a problem hiding this comment.
transferOwner(...) failures are caught and only logged, but the API still returns the copied file as if the overall operation succeeded. This can lead to silent partial success and make client-side error handling impossible. Consider returning a non-2xx status (or including an explicit warning/result field) when ownership transfer fails.
| run: | | ||
| set -euo pipefail | ||
|
|
||
| npm install vercel -g | ||
|
|
||
| if [[ "$GITHUB_REF" == 'refs/heads/main' ]]; then | ||
| DeployOutput=$(vercel -t "$VERCEL_TOKEN" --prod) | ||
| else | ||
| DeployOutput=$(vercel -t "$VERCEL_TOKEN") | ||
| fi |
There was a problem hiding this comment.
The Vercel deploy step checks for VERCEL_ORG_ID/VERCEL_PROJECT_ID, but the script never uses them (and there is no .vercel/ project linkage in the repo). In CI, vercel commonly prompts to link/select a project unless --yes/--confirm and project/scope options are provided, which can hang the job. Please pass the org/project explicitly (or commit a .vercel/project.json), and add non-interactive flags so the deployment is deterministic.
| method, | ||
| url, | ||
| headers: { host, authorization, ...headers }, | ||
| headers: { host, authorization, 'content-length': _, ...headers }, |
There was a problem hiding this comment.
The destructuring introduces a _ binding for content-length that is never used. With @typescript-eslint/no-unused-vars enabled, this will raise lint warnings. Consider renaming it to something matching an ignore pattern (e.g. _contentLength) or adjusting the rule options / destructuring approach to omit the header without creating an unused variable.
| headers: { host, authorization, 'content-length': _, ...headers }, | |
| headers: { host, authorization, 'content-length': _contentLength, ...headers }, |
Uh oh!
There was an error while loading. Please reload this page.