init project #1
init project #1
Conversation
WalkthroughThis change introduces a new TypeScript-based project, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MCP_Server
participant Tool_Module
participant Yunxiao_API
User->>MCP_Server: ListToolsRequest / CallToolRequest
MCP_Server->>Tool_Module: Validate input, dispatch call
Tool_Module->>Yunxiao_API: Make HTTP request (with auth, params)
Yunxiao_API-->>Tool_Module: Return API response
Tool_Module-->>MCP_Server: Return parsed/validated result
MCP_Server-->>User: Return tool list or operation result
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🔭 Outside diff range comments (1)
LICENSE.txt (1)
1-46:⚠️ Potential issueIncorrect file content for LICENSE: The file named
LICENSE.txtcontains an HTML rate-limit page rather than a license. Please rename this file (for example,rate_limit.html) and add a proper open-source LICENSE file (e.g., MIT or Apache 2.0) at the project root.
🧹 Nitpick comments (29)
tsconfig.json (1)
17-19: Exclude build artifacts: Consider explicitly excluding thedistdirectory to prevent accidental inclusion of compiled files. For example:"exclude": [ + "dist", "node_modules" ]Dockerfile (1)
1-30: Optimize Docker build context: To reduce image size and speed up builds, add a.dockerignorefile that excludesnode_modules,dist, and other non-essential files from the build context:node_modules dist .gitREADME.md (1)
18-18: Add missing article: ChangeGet current user's organization informationtoGet the current user's organization informationfor grammatical correctness.🧰 Tools
🪛 LanguageTool
[uncategorized] ~18-~18: You might be missing the article “the” here.
Context: ... -get_current_organization_Info: Get current user's organization information - `get_...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
README.zh-ch.md (1)
57-73: Consider adding version specifier for npx commandWhen using npx to run the MCP server, it's generally a good practice to specify the version number to ensure consistency across environments.
"npx", "-y", - "alibabacloud-devops-mcp-server" + "[email protected]"package.json (2)
10-11: Consider updating author.url to point to author's profileCurrently, the author.url field points to the repository URL. Typically, this field would point to the author's personal page or organization profile rather than duplicating the repository URL.
"author": { "name": "Yunxiao", "email": "[email protected]", - "url": "https://github.com/aliyun/alibabacloud-devops-mcp-server.git" + "url": "https://www.alibabacloud.com/product/yunxiao" },
28-30: Remove duplicate "mcp" entry in keywords arrayThere's a duplicate entry for "mcp" in the keywords array.
"devops", "mcp", - "mcp", "ai",operations/organization/organization.ts (3)
5-15: Simplify HTTP request by removing redundant method specificationThe
yunxiaoRequestfunction defaults to GET if no method is specified, so the explicit specification of the method is redundant.export async function getCurrentOrganizationInfoFunc( ): Promise<z.infer<typeof CurrentOrganizationInfoSchema>> { const url = "/oapi/v1/platform/user"; - const response = await yunxiaoRequest(url, { - method: "GET", - }); + const response = await yunxiaoRequest(url); return CurrentOrganizationInfoSchema.parse(response); }
21-23: Simplify HTTP request by removing redundant method specificationThe
yunxiaoRequestfunction defaults to GET if no method is specified, so the explicit specification of the method is redundant.- const response = await yunxiaoRequest(url, { - method: "GET", - }); + const response = await yunxiaoRequest(url);
17-31:❓ Verification inconclusive
Consider adding pagination support for organization listing
The function to fetch user organizations should handle pagination if the API supports it, especially since the number of organizations can grow over time.
🏁 Script executed:
#!/bin/bash # Check if the API endpoint supports pagination parameters curl -s "https://api.github.com/search/repositories?q=pagination+api" | jq '.items[] | select(.name | contains("alibaba") or contains("aliyun")) | .html_url' | head -n 5Length of output: 171
Continuing verification by inspecting the
yunxiaoRequestimplementation to see if it already supports query-param options:
🏁 Script executed:
#!/bin/bash # Locate and inspect the definition of yunxiaoRequest rg -n 'yunxiaoRequest' -C 5 .Length of output: 24808
Consider adding pagination support to getUserOrganizationsFunc
The
/oapi/v1/platform/organizationsendpoint may return paged results once a user belongs to many organizations. Currently, the function always fetches the first “page” and ignores any query parameters:• File: operations/organization/organization.ts
• Function: getUserOrganizationsFuncBefore shipping, please:
- Confirm via your internal API docs whether this endpoint supports pagination parameters (e.g.
page,size,cursor, etc.).- If so, update the function signature to accept optional paging options and use
buildUrlto append them.Suggested diff:
-export async function getUserOrganizationsFunc( -): Promise<z.infer<typeof UserOrganizationsInfoSchema>> { - const url = "/oapi/v1/platform/organizations"; +export async function getUserOrganizationsFunc( + params?: { page?: number; size?: number } +): Promise<z.infer<typeof UserOrganizationsInfoSchema>> { + const url = buildUrl("/oapi/v1/platform/organizations", params);operations/codeup/utils.ts (3)
37-45: Translate Chinese comments to English for consistencyThere are Chinese comments in this function while the rest of the codebase appears to be in English. This inconsistency should be addressed to maintain code readability for all contributors.
- // 如果传入的是字符串，先尝试转为浮点数 + // If the input is a string, try to convert it to a float first if (typeof value === 'string') { const floatValue = parseFloat(value); if (!isNaN(floatValue)) { value = floatValue; } else { - return value; // 如果转换失败，返回原字符串 + return value; // If conversion fails, return the original string } }
47-51: Use Math.round() for more idiomatic roundingThe current implementation uses
Math.floor(value + 0.5)for rounding. While this works, usingMath.round(value)would be more idiomatic and clearer in intent.- // 处理浮点数 + // Handle floating point numbers if (typeof value === 'number') { - const intValue = Math.floor(value + 0.5); // 四舍五入转整数 + const intValue = Math.round(value); // Round to the nearest integer return intValue.toString(); }
53-55: Translate Chinese comments to English for consistencyThis comment is in Chinese while the rest of the codebase appears to be in English.
- // 处理其他情况，直接转字符串 + // For all other cases, convert directly to string return String(value);common/utils.ts (2)
21-27: Consider using a more structured logging approach.The debug function is using
console.errorfor all debug output. For a production-ready application, consider:
- Using a proper logging library with configurable log levels
- Adding a DEBUG environment variable flag to conditionally enable debugging
+// Enable debug logging via environment variable +const DEBUG_ENABLED = process.env.DEBUG === 'true'; + export function debug(message: string, data?: unknown): void { + if (!DEBUG_ENABLED) return; + if (data !== undefined) { console.error(`[DEBUG] ${message}`, typeof data === 'object' ? JSON.stringify(data, null, 2) : data); } else { console.error(`[DEBUG] ${message}`); } }
123-131: Translate Chinese comments to English for consistency.The code includes a Chinese comment which should be translated to English for codebase consistency.
export function pathEscape(filePath: string): string { - // 先使用encodeURIComponent进行编码 + // First encode the path using encodeURIComponent let encoded = encodeURIComponent(filePath); - // 将编码后的%2F（/的编码）替换回/ + // Replace encoded %2F (encoded slash) back to / character encoded = encoded.replace(/%2F/gi, "/"); return encoded; }operations/codeup/compare.ts (1)
52-68: Consider using a utility for repetitive query parameter construction.The pattern of conditionally adding parameters to a query object is repeated across multiple files. You could extract this to a utility function.
+// In a shared utility file, e.g., common/utils.ts +export function addQueryParam<T>( + params: Record<string, string | number | undefined>, + key: string, + value: T | undefined +): void { + if (value !== undefined) { + params[key] = typeof value === 'boolean' ? String(value) : value.toString(); + } +} // In this file: // Build query parameters const queryParams: Record<string, string | undefined> = { from, to }; -if (sourceType !== undefined) { - queryParams.sourceType = sourceType; -} - -if (targetType !== undefined) { - queryParams.targetType = targetType; -} - -if (straight !== undefined) { - queryParams.straight = straight; -} +addQueryParam(queryParams, 'sourceType', sourceType); +addQueryParam(queryParams, 'targetType', targetType); +addQueryParam(queryParams, 'straight', straight);operations/projex/sprint.ts (1)
39-45: Use schema-derived type for function parameters.The function parameters should align with the schema to ensure type safety. Consider using the derived type
ListSprintsOptionsfor the function parameters.-export async function listSprintsFunc( - organizationId: string, - id: string, - status?: string, - page?: number, - perPage?: number -): Promise<z.infer<typeof SprintInfoSchema>[]> { +export async function listSprintsFunc( + options: ListSprintsOptions +): Promise<z.infer<typeof SprintInfoSchema>[]> { + const { organizationId, projectId: id, status, page, perPage } = options;operations/codeup/repositories.ts (1)
94-96: Consistent handling of boolean parameters.Boolean parameters require conversion to string for URL parameters. This should be handled consistently across all parameter types.
if (archived !== undefined) { - queryParams.archived = String(archived); // Convert boolean to string + queryParams.archived = archived.toString(); // Match the string conversion approach used for other types }index.ts (1)
623-629: Uncomment or remove the strayconfig()callNow that
config()is executed at the top of the file (see previous comment), the leftover commented call becomes dead code.-// config();operations/projex/project.ts (1)
26-30: Redundant.nullish().optional()chains
z.string().nullish()already acceptsundefined, so the extra.optional()is unnecessary and slightly obscures intent. Same pattern repeats for several fields.operations/codeup/branches.ts (3)
52-62: Duplicate repositoryId-encoding logic – extract into a reusable helperThe same 14-line block that detects “/” and percent-encodes the repo name is copy-pasted in all three public functions. Apart from bloat, this increases the risk of subtle divergence in future edits.
Consider moving the logic into
common/utils.ts:+export function encodeRepoId(id: string): string { + if (!id.includes("/")) return id; + const [org, repo] = id.split("/", 2); + return `${org}%2F${encodeURIComponent(repo).replace(/\+/g, "%20")}`; +}and then:
- if (repositoryId.includes("/")) { … } + repositoryId = encodeRepoId(repositoryId);You can do the same for branch names (
encodeURIComponentunless already encoded).Also applies to: 86-96, 116-126, 154-163
58-60:encodeURIComponentalready yields “%20” – thereplace(/\+/g, "%20")is redundant
encodeURIComponent("my repo")returnsmy%20repo, never+. Unless you are mixing inencodeURI, this replacement can be safely removed.
72-74:console.errorused for debugging – remove or gate behind a log levelLeaving raw
console.errorcalls in library code will spam stderr for every consumer.
Switch to the existingdebug()helper (present incommon/utils.ts) or wrap behind a verbosity flag.Also applies to: 152-153
common/types.ts (2)
23-44:FieldItemandValueschemas look identical – can be unifiedBoth hold a
displayValueandidentifier. If there is no semantic difference, merge them to avoid duplicate types and keep generated JSON-schema smaller.
211-213:ConditionsSchemaloses type safety by usingz.any()
conditionGroups: z.array(z.array(z.any()))disables compile-time checking.
If full typing is hard, at least restrict the inner item toFilterConditionSchemato prevent accidental shape mismatches:-conditionGroups: z.array(z.array(z.any())) +conditionGroups: z.array(z.array(FilterConditionSchema))operations/projex/workitem.ts (2)
146-158: Creation-date filter only works whencreatedAfteris providedWhen users specify only
createdBefore, no condition is generated. Consider handling the other half:if (args.createdBefore && !args.createdAfter) { filterConditions.push({ className: "date", fieldIdentifier: "gmtCreate", format: "input", operator: "BETWEEN", toValue: `${args.createdBefore} 23:59:59`, value: [], }); }
101-136: Repetitive comma-split & trim logic – extract helperThe same pattern is used for
status,creator,assignedTo. A small util improves readability:const csv = (s?: string) => s?.split(",").map(v => v.trim()) ?? [];Then:
const values = csv(args.status); if (values.length) …Also applies to: 160-186
operations/codeup/changeRequestComments.ts (1)
1-4: Remove unused import to avoid lint / build failures
buildUrlis imported but never referenced. In most TS configs (noUnusedLocals/noUnusedParameters), this will surface as a compilation error and break CI.-import { yunxiaoRequest, buildUrl } from "../../common/utils.js"; +import { yunxiaoRequest } from "../../common/utils.js";operations/codeup/files.ts (2)
61-88: Dead code: helper never used
handlePathEncoding()is well-written but not invoked anywhere in the module. Either:
- Replace the duplicated encoding logic in each public function with this helper, or
- Remove the helper to keep the codebase lean.
Leaning on the helper will eliminate five separate blocks of near-identical code.
295-308: Missing URL-encoding forpathquery parameterWhen
listFilesFuncforwardspathtoqueryParams, any spaces or#etc. will break the URL. Encode before assignment:-if (path) { - queryParams.path = path; +if (path) { + queryParams.path = encodeURIComponent(path); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
.DS_Storeis excluded by!**/.DS_Storeoperations/.DS_Storeis excluded by!**/.DS_Storepackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (22)
Dockerfile(1 hunks)LICENSE.txt(1 hunks)README.md(1 hunks)README.zh-ch.md(1 hunks)common/errors.ts(1 hunks)common/types.ts(1 hunks)common/utils.ts(1 hunks)common/version.ts(1 hunks)index.ts(1 hunks)operations/codeup/branches.ts(1 hunks)operations/codeup/changeRequestComments.ts(1 hunks)operations/codeup/changeRequests.ts(1 hunks)operations/codeup/compare.ts(1 hunks)operations/codeup/files.ts(1 hunks)operations/codeup/repositories.ts(1 hunks)operations/codeup/utils.ts(1 hunks)operations/organization/organization.ts(1 hunks)operations/projex/project.ts(1 hunks)operations/projex/sprint.ts(1 hunks)operations/projex/workitem.ts(1 hunks)package.json(1 hunks)tsconfig.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (8)
operations/organization/organization.ts (2)
common/types.ts (2)
CurrentOrganizationInfoSchema(4-6)UserOrganizationsInfoSchema(14-14)common/utils.ts (1)
yunxiaoRequest(87-121)
index.ts (2)
common/version.ts (1)
VERSION(1-1)common/errors.ts (8)
YunxiaoError(1-10)YunxiaoValidationError(12-17)YunxiaoResourceNotFoundError(19-24)YunxiaoAuthenticationError(26-31)YunxiaoPermissionError(33-38)YunxiaoRateLimitError(40-48)YunxiaoConflictError(50-55)isYunxiaoError(57-59)
operations/codeup/compare.ts (3)
operations/codeup/utils.ts (1)
handleRepositoryIdEncoding(12-28)common/types.ts (1)
CompareSchema(454-462)common/utils.ts (2)
buildUrl(37-83)yunxiaoRequest(87-121)
operations/projex/project.ts (2)
common/types.ts (3)
ProjectInfoSchema(52-84)FilterConditionSchema(202-209)ConditionsSchema(211-213)common/utils.ts (1)
yunxiaoRequest(87-121)
operations/projex/sprint.ts (2)
common/types.ts (1)
SprintInfoSchema(109-126)common/utils.ts (2)
yunxiaoRequest(87-121)buildUrl(37-83)
operations/codeup/repositories.ts (3)
operations/codeup/utils.ts (1)
handleRepositoryIdEncoding(12-28)common/types.ts (1)
RepositorySchema(260-266)common/utils.ts (2)
yunxiaoRequest(87-121)buildUrl(37-83)
operations/codeup/branches.ts (2)
common/types.ts (1)
CodeupBranchSchema(215-238)common/utils.ts (2)
buildUrl(37-83)yunxiaoRequest(87-121)
operations/projex/workitem.ts (2)
common/types.ts (3)
WorkItemSchema(162-199)FilterConditionSchema(202-209)ConditionsSchema(211-213)common/utils.ts (1)
yunxiaoRequest(87-121)
🪛 LanguageTool
README.md
[uncategorized] ~18-~18: You might be missing the article “the” here.
Context: ... - get_current_organization_Info: Get current user's organization information - `get_...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~23-~23: Loose punctuation mark.
Context: ...Code Management Tools - create_branch: Create a branch - delete_branch: Dele...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~44-~44: Loose punctuation mark.
Context: ...roject Management Tools - get_project: Get project details - search_projects...
(UNLIKELY_OPENING_PUNCTUATION)
README.zh-ch.md
[uncategorized] ~2-~2: 动词的修饰一般为‘形容词（副词）+地+动词’。您的意思是否是：重要"地"创新
Context: ...业研发团队可以使用它协助代码审查、优化任务管理、减少重复性操作，从而专注于更重要的创新和产品交付。 ## 功能特性 alibabacloud-devops-m...
(wb4)
🔇 Additional comments (15)
common/version.ts (1)
1-1: Add version constant: TheVERSIONconstant is defined correctly. Please verify that this value matches theversionfield inpackage.jsonto avoid inconsistencies in publishing and reporting.README.zh-ch.md (1)
1-102: Well-structured Chinese README with comprehensive explanation of the MCP server capabilitiesThe README provides a clear and detailed explanation of the alibabacloud-devops-mcp-server tool, including its purpose, features, available tools, and setup instructions. It effectively communicates how AI assistants can interact with the Alibaba Cloud DevOps platform to automate various tasks.
The organization of tools into categories (organization, code management, project management) makes it easy for users to understand the available capabilities. The setup instructions for both NPX and Docker are well documented with clear examples.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~2-~2: 动词的修饰一般为‘形容词（副词）+地+动词’。您的意思是否是：重要"地"创新
Context: ...业研发团队可以使用它协助代码审查、优化任务管理、减少重复性操作，从而专注于更重要的创新和产品交付。 ## 功能特性 alibabacloud-devops-m...(wb4)
package.json (1)
1-51: Well-structured package.json with appropriate configurationThe package.json provides all the necessary configuration for the project, including metadata, dependencies, scripts, and CLI command setup. The build script correctly ensures executable permissions on the output JavaScript files.
operations/codeup/utils.ts (2)
12-28: Well-implemented repository ID encoding functionThe
handleRepositoryIdEncodingfunction correctly handles the case where a repository ID contains a slash, which is crucial for proper URL encoding. The function also properly handles spaces in repository names by replacing+with%20.
57-74: Robust implementation of number parsing with undefined handlingThe
safeParseNumberfunction correctly handles undefined inputs and invalid string conversions, making it a robust utility for handling optional numeric parameters in API calls.common/utils.ts (2)
5-13: Good implementation of configurable API base URL.The function provides a clean way to override the default API URL through environment variables, enhancing flexibility for different environments.
29-35: Good content-type based response parsing.The function correctly handles different response types based on content-type headers, with a sensible fallback to text.
operations/codeup/compare.ts (1)
6-14: Well-structured schema with detailed descriptions.The schema provides clear parameter descriptions, especially for complex parameters like the straight flag that controls Merge-Base behavior.
operations/projex/sprint.ts (1)
71-74: Good defensive programming for response handling.The function correctly checks if the response is an array before attempting to process it, preventing potential runtime errors.
index.ts (1)
335-347: Ensure a schema is exported forfiles.ListFilesSchemaThe handler assumes
ListFilesSchemaexists. Double-check thatoperations/codeup/files.tsreally exports it; otherwise runtime parsing will throw.operations/codeup/branches.ts (1)
167-178: Possible query-param mismatch –perPagevsper_pageThe restful endpoints in Codeup usually use
per_page. Here we sendperPage, which the backend might ignore and silently fall back to 20.Please confirm in the API docs and adjust accordingly.
operations/projex/workitem.ts (1)
68-82: Stringifying conditions twice may break the backend
buildWorkitemConditionsalready returns a JSON string, which is assigned verbatim topayload.conditions.
yunxiaoRequestlater doesJSON.stringify(payload), producing:{ "conditions": "{\"conditionGroups\":[…]}" }If the API expects an object, the server will read a string instead.
Either return an object frombuildWorkitemConditionsorJSON.parsebefore assignment.operations/codeup/changeRequestComments.ts (1)
67-78: Good validation for INLINE_COMMENT parametersNice guard that throws when the four mandatory inline-comment fields are missing. 👍
operations/codeup/changeRequests.ts (2)
228-244: Potential double-encoding when fetching numeric repo ID
createChangeRequestFuncpassesencodedRepoId(already containing “%2F”) togetRepositoryNumericId, which constructs
/repositories/${repositoryId}.If
repositoryIdoriginally contained a literal “%2F”, the backend may expect it un-encoded in this specific endpoint. Confirm with the API; if it requires the raw slash form, pass the originalrepositoryIdinstead:-const numericId = await getRepositoryNumericId(organizationId, encodedRepoId); +const numericId = await getRepositoryNumericId(organizationId, repositoryId);Please verify with an integration test.
91-168: Elegant array guard & parsing – looks goodThe pagination / filtering code is verbose but clear, and the defensive checks (
!Array.isArray) followed by schema parsing ensure robust typing of the return value. Nice work.
| function handleRepositoryIdEncoding(repositoryId: string): string { | ||
| let encodedRepoId = repositoryId; | ||
|
|
||
| // Automatically handle unencoded slashes in repositoryId | ||
| if (repositoryId.includes("/")) { | ||
| // Found unencoded slash, automatically URL encode it | ||
| const parts = repositoryId.split("/", 2); | ||
| if (parts.length === 2) { | ||
| const encodedRepoName = encodeURIComponent(parts[1]); | ||
| // Remove + signs from encoding (spaces are encoded as +, but we need %20) | ||
| const formattedEncodedName = encodedRepoName.replace(/\+/g, "%20"); | ||
| encodedRepoId = `${parts[0]}%2F${formattedEncodedName}`; | ||
| } | ||
| } | ||
|
|
||
| return encodedRepoId; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Duplicated repository ID encoding logic.
This function duplicates code that appears in operations/codeup/utils.ts. Consider importing and reusing the utility function rather than duplicating it.
-// Helper function to handle repositoryId encoding
-function handleRepositoryIdEncoding(repositoryId: string): string {
- let encodedRepoId = repositoryId;
-
- // Automatically handle unencoded slashes in repositoryId
- if (repositoryId.includes("/")) {
- // Found unencoded slash, automatically URL encode it
- const parts = repositoryId.split("/", 2);
- if (parts.length === 2) {
- const encodedRepoName = encodeURIComponent(parts[1]);
- // Remove + signs from encoding (spaces are encoded as +, but we need %20)
- const formattedEncodedName = encodedRepoName.replace(/\+/g, "%20");
- encodedRepoId = `${parts[0]}%2F${formattedEncodedName}`;
- }
- }
-
- return encodedRepoId;
-}
+import { handleRepositoryIdEncoding } from "./utils.js";Committable suggestion skipped: line range outside the PR's diff.
| organizationId: z.string().describe("Organization ID"), | ||
| id: z.string().describe("Project unique identifier"), | ||
| status: z.string().optional().describe("Filter by status: TODO, DOING, ARCHIVED, corresponding to not started, in progress, and completed; multiple statuses separated by commas"), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Parameter name inconsistency between schema and function.
The schema parameter id (line 14) is used as the project ID in the implementation. Consider renaming it to projectId in the schema for clarity and consistency with the function parameters.
export const ListSprintsSchema = z.object({
organizationId: z.string().describe("Organization ID"),
- id: z.string().describe("Project unique identifier"),
+ projectId: z.string().describe("Project unique identifier"),
status: z.string().optional().describe("Filter by status: TODO, DOING, ARCHIVED, corresponding to not started, in progress, and completed; multiple statuses separated by commas"),📝 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.
| organizationId: z.string().describe("Organization ID"), | |
| id: z.string().describe("Project unique identifier"), | |
| status: z.string().optional().describe("Filter by status: TODO, DOING, ARCHIVED, corresponding to not started, in progress, and completed; multiple statuses separated by commas"), | |
| export const ListSprintsSchema = z.object({ | |
| organizationId: z.string().describe("Organization ID"), | |
| projectId: z.string().describe("Project unique identifier"), | |
| status: z.string().optional().describe("Filter by status: TODO, DOING, ARCHIVED, corresponding to not started, in progress, and completed; multiple statuses separated by commas"), | |
| }); |
| export async function listRepositoriesFunc( | ||
| organizationId: string, | ||
| page?: number, | ||
| perPage?: number, | ||
| orderBy?: string, | ||
| sort?: string, | ||
| search?: string, | ||
| archived?: boolean | ||
| ): Promise<z.infer<typeof RepositorySchema>[]> { | ||
| const baseUrl = `/oapi/v1/codeup/organizations/${organizationId}/repositories`; | ||
|
|
||
| // Build query parameters | ||
| const queryParams: Record<string, string | number | undefined> = {}; | ||
|
|
||
| if (page !== undefined) { | ||
| queryParams.page = page; | ||
| } | ||
|
|
||
| if (perPage !== undefined) { | ||
| queryParams.perPage = perPage; | ||
| } | ||
|
|
||
| if (orderBy !== undefined) { | ||
| queryParams.orderBy = orderBy; | ||
| } | ||
|
|
||
| if (sort !== undefined) { | ||
| queryParams.sort = sort; | ||
| } | ||
|
|
||
| if (search !== undefined) { | ||
| queryParams.search = search; | ||
| } | ||
|
|
||
| if (archived !== undefined) { | ||
| queryParams.archived = String(archived); // Convert boolean to string | ||
| } | ||
|
|
||
| // Use buildUrl function to construct URL with query parameters | ||
| const url = buildUrl(baseUrl, queryParams); | ||
|
|
||
| const response = await yunxiaoRequest(url, { | ||
| method: "GET", | ||
| }); | ||
|
|
||
| // Ensure the response is an array | ||
| if (!Array.isArray(response)) { | ||
| return []; | ||
| } | ||
|
|
||
| // Parse each repository object | ||
| return response.map(repo => RepositorySchema.parse(repo)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve parameter handling with schema-derived type.
The function has 7 parameters, making it difficult to maintain and use. Consider refactoring to accept a single options object based on the ListRepositoriesOptions type, which would also enforce the schema constraints.
-export async function listRepositoriesFunc(
- organizationId: string,
- page?: number,
- perPage?: number,
- orderBy?: string,
- sort?: string,
- search?: string,
- archived?: boolean
-): Promise<z.infer<typeof RepositorySchema>[]> {
+export async function listRepositoriesFunc(
+ options: ListRepositoriesOptions
+): Promise<z.infer<typeof RepositorySchema>[]> {
+ const {
+ organizationId,
+ page,
+ perPage,
+ orderBy,
+ sort,
+ search,
+ archived
+ } = options;This would allow consumers to use the function more easily:
// Before
const repos = await listRepositoriesFunc("org123", 1, 20, "name", "asc", "test", false);
// After
const repos = await listRepositoriesFunc({
organizationId: "org123",
page: 1,
perPage: 20,
orderBy: "name",
sort: "asc",
search: "test",
archived: false
});| sort?: string, // Possible values: name_asc, name_desc, updated_asc, updated_desc | ||
| search?: string | ||
| ): Promise<z.infer<typeof CodeupBranchSchema>[]> { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Widened sort parameter loses type-safety
sort is declared as plain string, but the API accepts only four literals (name_asc … updated_desc).
Use a union to surface misuse at compile-time:
- sort?: string
+ sort?: "name_asc" | "name_desc" | "updated_asc" | "updated_desc"📝 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.
| sort?: string, // Possible values: name_asc, name_desc, updated_asc, updated_desc | |
| search?: string | |
| ): Promise<z.infer<typeof CodeupBranchSchema>[]> { | |
| sort?: "name_asc" | "name_desc" | "updated_asc" | "updated_desc", // Possible values: name_asc, name_desc, updated_asc, updated_desc | |
| search?: string | |
| ): Promise<z.infer<typeof CodeupBranchSchema>[]> { |
| organizationId: string, | ||
| category: string, | ||
| spaceId: string, | ||
| subject?: string, | ||
| status?: string, | ||
| createdAfter?: string, | ||
| createdBefore?: string, | ||
| creator?: string, | ||
| assignedTo?: string, | ||
| advancedConditions?: string, | ||
| orderBy: string = "gmtCreate" // Possible values: gmtCreate, subject, status, priority, assignedTo | ||
| ): Promise<z.infer<typeof WorkItemSchema>[]> { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
No pagination support in searchWorkitemsFunc
Large work-item sets will be truncated to the server default.
Expose page / perPage (or offset / limit) parameters to callers and forward them in the payload to avoid silent data loss.
| organizationId: string, | ||
| repositoryId: string, | ||
| localId: string, | ||
| comment_type: string, // Possible values: GLOBAL_COMMENT, INLINE_COMMENT | ||
| content: string, | ||
| draft: boolean, | ||
| resolved: boolean, | ||
| patchset_biz_id: string, | ||
| file_path?: string, | ||
| line_number?: number, | ||
| from_patchset_biz_id?: string, | ||
| to_patchset_biz_id?: string, | ||
| parent_comment_biz_id?: string | ||
| ): Promise<z.infer<typeof ChangeRequestCommentSchema>> { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Long positional parameter list – switch to typed options object
createChangeRequestCommentFunc takes 13 positional parameters, which is extremely error-prone (easy to mix order or forget a value) and hard to evolve.
Consider accepting a single CreateChangeRequestCommentOptions object that is already defined by Zod:
-export async function createChangeRequestCommentFunc(
- organizationId: string,
- repositoryId: string,
- localId: string,
- comment_type: string,
- ...
-) { … }
+export async function createChangeRequestCommentFunc(
+ opts: CreateChangeRequestCommentOptions
+): Promise<ChangeRequestComment> {
+ const {
+ organizationId,
+ repositoryId,
+ localId,
+ comment_type,
+ content,
+ draft,
+ resolved,
+ patchset_biz_id,
+ file_path,
+ line_number,
+ from_patchset_biz_id,
+ to_patchset_biz_id,
+ parent_comment_biz_id,
+ } = CreateChangeRequestCommentSchema.parse(opts);
+ …
+}Besides readability, you get runtime validation for free via CreateChangeRequestCommentSchema.parse.
| export async function updateFileFunc( | ||
| organizationId: string, | ||
| repositoryId: string, | ||
| filePath: string, | ||
| content: string, | ||
| commitMessage: string, | ||
| branch: string, | ||
| encoding?: string | ||
| ): Promise<z.infer<typeof CreateFileResponseSchema>> { | ||
| //const { encodedRepoId, encodedFilePath } = handlePathEncoding(repositoryId, filePath); | ||
| let encodedRepoId = repositoryId; | ||
| let encodedFilePath = filePath; | ||
|
|
||
| // 自动处理repositoryId中未编码的斜杠 | ||
| if (repositoryId.includes("/")) { | ||
| // 发现未编码的斜杠，自动进行URL编码 | ||
| const parts = repositoryId.split("/", 2); | ||
| if (parts.length === 2) { | ||
| const encodedRepoName = encodeURIComponent(parts[1]); | ||
| // 移除编码中的+号（空格被编码为+，但我们需要%20） | ||
| const formattedEncodedName = encodedRepoName.replace(/\+/g, "%20"); | ||
| encodedRepoId = `${parts[0]}%2F${formattedEncodedName}`; | ||
| } | ||
| } | ||
|
|
||
| // 确保filePath已被URL编码 | ||
| if (filePath.includes("/")) { | ||
| const pathToEncode = filePath.startsWith("/") ? filePath.substring(1) : filePath; | ||
| encodedFilePath = encodeURIComponent(pathToEncode); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Inconsistent file-path encoding method across CRUD functions
createFileFunc ➜ pathEscape
updateFileFunc ➜ encodeURIComponent (slashes encoded)
deleteFileFunc ➜ encodeURIComponent
getFileBlobsFunc ➜ encodeURIComponent
This inconsistency will lead to “NOT FOUND” / “400 Bad Request” errors depending on which function is called.
Unify all four functions on a single helper, e.g.:
const { encodedRepoId, encodedFilePath } = handlePathEncoding(repositoryId, filePath);and delete the duplicated inline encoding blocks.
| // const { encodedRepoId, encodedFilePath } = handlePathEncoding(repositoryId, filePath); | ||
| let encodedRepoId = repositoryId; | ||
| let encodedFilePath = filePath; | ||
| // 自动处理repositoryId中未编码的斜杠 | ||
| if (repositoryId.includes("/")) { | ||
| // 发现未编码的斜杠，自动进行URL编码 | ||
| const parts = repositoryId.split("/", 2); | ||
| if (parts.length === 2) { | ||
| const encodedRepoName = encodeURIComponent(parts[1]); | ||
| // 移除编码中的+号（空格被编码为+，但我们需要%20） | ||
| const formattedEncodedName = encodedRepoName.replace(/\+/g, "%20"); | ||
| encodedRepoId = `${parts[0]}%2F${formattedEncodedName}`; | ||
| } | ||
| } | ||
|
|
||
| // 确保filePath已被URL编码 | ||
| if (filePath.includes("/")) { | ||
| encodedFilePath = encodeURIComponent(filePath); | ||
| } | ||
|
|
There was a problem hiding this comment.
encodeURIComponent breaks path semantics – use pathEscape (or keep slashes)
getFileBlobsFunc fully encodes the slash characters:
encodedFilePath = encodeURIComponent(filePath);
// "/src/a.java" -> "%2Fsrc%2Fa.java"The API path then becomes
…/files/%2Fsrc%2Fa.java, which most servers interpret as literal “/src/a.java” (leading slash included) instead of the desired nested resource. In contrast, createFileFunc correctly uses pathEscape which preserves directory separators.
Patch suggestion:
- if (filePath.includes("/")) {
- encodedFilePath = encodeURIComponent(filePath);
- }
+ encodedFilePath = pathEscape(filePath);The same issue exists in deleteFileFunc (lines 249-252) and updateFileFunc (lines 205-209).
Summary by CodeRabbit
New Features
Documentation
Chores