deno lint -> oxlint#1561
Conversation
- Replaced `deno lint` with `oxlint` for linting. - Adjusted script commands for consistency and clarity. - Updated `bun.lockb` to reflect changes in dependencies. - Refactored header merging logic in various components for improved readability. - Enhanced regex patterns in multiple files for consistency in path matching. These changes streamline the development process and improve code maintainability.
|
Warning Rate limit exceeded@viktormarinho has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 16 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (5)
WalkthroughMigrates linting from Deno to Oxlint across the repo, removes an explicit Deno setup step from CI, replaces many Changes
Sequence Diagram(s)sequenceDiagram
participant CI as GitHub Actions
participant Setup as denoland/setup-deno@v2
participant Install as InstallDeps
participant Test as RunTests
Note over CI: Previous workflow
CI->>Setup: setup deno (deno-version input)
Setup-->>CI: deno available
CI->>Install: install dependencies
Install-->>CI: deps installed
CI->>Test: run tests
Note over CI: New workflow (this PR)
CI->>Install: install dependencies
Install-->>CI: deps installed
CI->>Test: run tests
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
- Introduced a new `.oxlintrc.json` file to configure linting rules, specifically turning off the `no-unused-expressions` rule and ignoring certain patterns. - Refactored error handling in various components to remove unused error parameters, improving code clarity and consistency. - Updated `bun.lockb` to reflect dependency changes. These updates enhance the linting process and improve the overall readability of the codebase.
Deploying chat with
|
| Latest commit: |
99a94cc
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://20872d56.chat-46r.pages.dev |
| Branch Preview URL: | https://oxlint.chat-46r.pages.dev |
- Removed unused compiler options and lint configuration from package.json, streamlining the file for better clarity. - Updated error handling in the getAppUUID function to log the error object instead of a generic variable, enhancing debugging capabilities. These changes improve the maintainability and readability of the codebase.
- Updated the metadata assignment in the AIAgent class to ensure proper handling of timings. This change enhances the clarity and maintainability of the code by explicitly spreading the existing metadata before adding new properties. No functionality is broken; this update improves the robustness of the agent's context management.
- Added `no-explicit-any` and `ban-types` rules to `.oxlintrc.json` with warnings to improve type safety. - Updated ignore patterns to include `docs/*` for better linting coverage. - Replaced `deno-lint-ignore-file` comments with `oxlint-disable` for consistency across the codebase. These changes enhance the linting process and promote better coding practices throughout the project.
- Replaced `deno-lint-ignore` comments with `oxlint-disable-next-line` across multiple files to standardize linting practices. - This change improves code consistency and aligns with the updated linting rules established in previous commits.
- Added a new linting rule to enforce kebab-case naming for filenames in the project, improving consistency in file naming conventions. - Updated `.oxlintrc.json` to include the new rule and associated plugin, enhancing the linting configuration. - Removed the old TypeScript implementation of the kebab-case rule in favor of the new JavaScript version for better compatibility with the current linting setup. These changes promote better coding practices and maintainability across the codebase.
- Moved the linting rules section within `.oxlintrc.json` to improve clarity and organization. - Retained existing rules such as `no-unused-expressions`, `no-explicit-any`, `enforce-kebab-case-file-names/kebab-case`, and `ban-types` to maintain code quality and consistency. These adjustments enhance the readability of the linting configuration while preserving the established linting practices.
- Eliminated the Deno setup step from the GitHub Actions workflow as linting has been fully migrated to Biome. - Updated package.json to remove the deprecated Deno lint command, streamlining the linting process. These changes enhance the CI configuration by removing unnecessary steps and aligning with the current linting practices.
- Refactored the `handleLiteral` function in `ensure-tailwind-design-system-tokens.js` to use a more concise parameter destructuring syntax, improving code readability. - Updated `.oxlintrc.json` to streamline the configuration by consolidating the `jsPlugins` array into a single line. These changes enhance the clarity of the code and maintain consistency in the linting configuration.
- Eliminated the `no-explicit-any` linting directive from `do-storage.ts` to align with updated linting practices. - This change enhances code clarity by removing unnecessary linting comments and maintaining consistency across the codebase.
- Updated the version of `deco-cli` from `0.22.4` to `0.22.5` to reflect recent changes. - Incremented the version of `@deco/workers-runtime` from `0.23.1` to `0.23.2` for consistency with the latest updates. These version bumps ensure that the packages are aligned with the latest developments in the codebase.
- Updated the package name in package.json to reflect the new branding for the admin interface. - This change aligns the package with the overall project structure and naming conventions.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (18)
packages/cli/src/lib/prompt-ide-setup.ts (2)
62-72: Restore the nullish coalescing fallback to prevent silent data loss.If the existing
mcp.jsonfile parses successfully but lacks themcpServersproperty (e.g., the file contains{}or has a different structure),existingConfig.mcpServerswill beundefinedat runtime despite theMCPConfigtype annotation. Spreadingundefinedin an object literal is safe but silent—any existing config that doesn't match the expected interface will be dropped without warning.Apply this diff to restore defensive handling:
const config = { mcpServers: { - ...existingConfig.mcpServers, + ...(existingConfig.mcpServers ?? {}), ...mcpConfig.mcpServers, }, };Alternatively, consider adding runtime validation (e.g., Zod schema) to ensure the parsed JSON conforms to
MCPConfigbefore using it.
96-106: Restore the nullish coalescing fallback to prevent silent data loss.If the existing
mcp.jsonfile parses successfully but lacks themcpServersproperty (e.g., the file contains{}or has a different structure),existingConfig.mcpServerswill beundefinedat runtime despite theMCPConfigtype annotation. Spreadingundefinedin an object literal is safe but silent—any existing config that doesn't match the expected interface will be dropped without warning.Apply this diff to restore defensive handling:
const config = { mcpServers: { - ...existingConfig.mcpServers, + ...(existingConfig.mcpServers ?? {}), ...mcpConfig.mcpServers, }, };Alternatively, consider adding runtime validation (e.g., Zod schema) to ensure the parsed JSON conforms to
MCPConfigbefore using it.apps/api/src/email.ts (1)
82-83: Potential bug: inconsistent Cc handling.Line 82 retrieves the Cc header into the
ccvariable usingmessage.headers.get("Cc"), but line 83 checksccfor truthiness and then accessesmessage.cc(a different property) instead. This appears to be a logic error where the retrieved header value isn't actually used.Consider fixing the logic to use the retrieved header value:
const cc = message.headers.get("Cc"); -// @ts-ignore: cc is not a valid property of the EmailMessage class -cc && msg.setCc(message.cc); +if (cc) { + msg.setCc(cc); +}packages/runtime/src/proxy.ts (1)
165-168: execute() drops inputs by calling with input.contextThis breaks tools that expect the full input. Pass the input object directly.
Apply:
- execute: (input: any) => { - return callToolFn(input.context); - }, + execute: (input: any) => { + return callToolFn(input); + },packages/sdk/src/mcp/hosting/assertions.ts (1)
81-104: Overly permissive type compatibility (string ↔ number) can miss breakagesTreating string→number or number→string as compatible can be breaking in practice.
Tighten compatibility:
- if (fromT === "string" && toT === "number") return true; // strings can often be parsed as numbers - if (fromT === "number" && toT === "string") return true; // numbers can be stringified + // Only allow safe widen/narrow within numeric domain + // string<->number is NOT safe: parsing/stringifying is not guaranteed + if (fromT === "integer" && toT === "number") return true; + if (fromT === "number" && toT === "integer") return false;Optionally, allow explicit whitelisting via config if you truly coerce at runtime.
packages/sdk/src/mcp/api-keys/api.ts (1)
519-552: checkAccess ignores the provided key; authorization check always uses the current contextYou parse “key” into user but never pass it to authorization. Result: access is checked for c.user, not the supplied key.
Apply:
- c.resourceAccess.grant(); // this is public because it uses the current key from context - - // TODO(@mcandeia): remove this ignore - // eslint-disable-next-line eslint/no-unused-vars - let user = c.user; + c.resourceAccess.grant(); // public: uses current key unless overridden + let user = c.user; @@ - const hasAccess = await Promise.all( + const ctxForCheck = key ? ({ ...c, user } as typeof c) : c; + const hasAccess = await Promise.all( tools.map(async (tool) => { return [ tool, - await assertWorkspaceResourceAccess(c, tool) + await assertWorkspaceResourceAccess(ctxForCheck, tool) .then(() => true) .catch(() => false), ]; }), );This removes the need for the unused-var suppression and aligns behavior with the tool description.
packages/runtime/src/client.ts (4)
49-54: Set a default Content-Type header for JSON requestsWithout Content-Type: application/json, some servers won’t parse the body. Add it to DEFAULT_INIT so both paths inherit it.
Apply this diff:
export const DEFAULT_INIT: CustomInit = { credentials: "include", headers: { + "Content-Type": "application/json", [DECO_MCP_CLIENT_HEADER]: "true", }, };
43-45: Support Authorization bearer tokens in the clientMCP calls should carry Authorization: Bearer when available. Add an authToken helper field and propagate it into headers. Based on learnings.
Apply this diff:
export type CustomInit = RequestInit & { handleResponse?: (response: Response) => Promise<unknown>; + authToken?: string; // optional bearer token helper }; // in callMCPTool(...) const mergedInit: CustomInit = { ...init, headers: { + "Content-Type": "application/json", ...DEFAULT_INIT.headers, + ...(init?.authToken + ? { Authorization: `Bearer ${init.authToken}` } + : {}), ...init?.headers, }, }; // in createClient regular method path const mergedInit: CustomInit = { ...init, ...innerInit, headers: { + "Content-Type": "application/json", ...DEFAULT_INIT.headers, + ...((innerInit?.authToken ?? init?.authToken) + ? { + Authorization: `Bearer ${ + innerInit?.authToken ?? init?.authToken + }`, + } + : {}), ...init?.headers, ...innerInit?.headers, }, };If you already inject Authorization via init.headers externally, this remains backward‑compatible.
Also applies to: 64-71, 173-190
185-197: Unify error handling: throw on non-OK responses in both pathscallMCPTool throws on !response.ok but the proxy path does not, silently returning JSON for error responses. Align behavior.
Apply this diff:
- if (typeof mergedInit.handleResponse === "function") { + if (typeof mergedInit.handleResponse === "function") { return mergedInit.handleResponse(response); } - - return response.json(); + if (!response.ok) { + throw new Error( + `Failed to call ${String(prop)}: ${response.status} ${response.statusText}`, + ); + } + return response.json();Also applies to: 79-84
133-156: Close EventSource on iterator completion/error to avoid leaksThe async generator never closes the EventSource. Ensure cleanup in finally; optionally hook AbortSignal.
Apply this diff:
- const eventSource = new EventSource(watchUrl.href); + const eventSource = new EventSource(watchUrl.href); + const abortHandler = + init?.signal ? () => eventSource.close() : undefined; + if (init?.signal && abortHandler) { + init.signal.addEventListener("abort", abortHandler, { once: true }); + } const eventStream = toAsyncIterator<{ uri: string }>( eventSource, "message", ); - for await (const event of eventStream) { - const uri = event.uri; - - if (uri) { - // Call READ to get full resource data - const readData = await callMCPTool<{ data: unknown }>( - readMethodName, - { uri }, - init, - ); - - yield { uri, data: readData.data }; - } - } + try { + for await (const event of eventStream) { + const uri = event.uri; + if (uri) { + const readData = await callMCPTool<{ data: unknown }>( + readMethodName, + { uri }, + init, + ); + yield { uri, data: readData.data }; + } + } + } finally { + eventSource.close(); + if (init?.signal && abortHandler) { + init.signal.removeEventListener("abort", abortHandler); + } + }apps/api/tail.ts (3)
224-231: Use each log’s timestamp (falls back to event timestamp)Currently uses event.eventTimestamp for all logs, losing per‑log timing.
Apply this diff:
- return { - timeUnixNano: event.eventTimestamp * 1000000, + return { + timeUnixNano: + ((log.timestamp ?? event.eventTimestamp) as number) * + 1000000, severityText: log.level, body: { stringValue: message },
305-315: Use each exception’s timestamp (falls back to event timestamp)Exceptions also have their own timestamps; preserve them.
Apply this diff:
- scope: {}, - logRecords: event.exceptions.map((exception) => ({ - timeUnixNano: event.eventTimestamp * 1000000, + scope: {}, + logRecords: event.exceptions.map((exception) => ({ + timeUnixNano: + ((exception.timestamp ?? event.eventTimestamp) as number) * + 1000000, severityText: "ERROR",
147-159: Harden header filtering to avoid PII leakageDenylist misses common sensitive headers (x-forwarded-for, cf-connecting-ip, x-api-key, proxy-authorization, etc.). Switch to an allowlist or expand denylist.
Apply this diff and helper:
- ? Object.entries(event.event!.request!.headers) - .filter( - ([key]) => - !key.toLowerCase().includes("authorization") && - !key.toLowerCase().includes("cookie") && - !key.toLowerCase().includes("x-real-ip"), - ) + ? Object.entries(event.event!.request!.headers) + .filter(([key]) => isSafeHeader(key)) .slice(0, 10)Add this helper near the top (outside the selected range):
const SENSITIVE_HEADER_SUBSTRINGS = [ "authorization", "proxy-authorization", "cookie", "x-real-ip", "x-forwarded-for", "cf-connecting-ip", "x-client-ip", "x-api-key", "x-auth-token", ]; function isSafeHeader(name: string): boolean { const lower = name.toLowerCase(); return !SENSITIVE_HEADER_SUBSTRINGS.some((s) => lower.includes(s)); }As per coding guidelines (privacy/compliance).
apps/web/src/components/settings/profile.tsx (1)
50-51: Brazil placeholder has an extra digit.E.164 for BR is +55 + 2-digit area + 9-digit local = 13 digits total. Placeholder shows 14. Fix the example.
- placeholder: "+55119100000000", + placeholder: "+5511910000000",apps/web/src/components/views/internal-resource-list.tsx (3)
86-117: Guard against missing integration connection in runSearchrunSearch may call callTool with an undefined connection, causing runtime errors during initial load. Guard before toggling loading.
Apply this diff:
async function runSearch(term: string) { - setLoading(true); + if (!integration?.connection) return; + setLoading(true); try { - const result = (await callTool(integration?.connection, { + const result = (await callTool(integration.connection, { name: "DECO_CHAT_RESOURCES_SEARCH", arguments: { name, term, limit: 50 }, })) as { structuredContent?: { items?: Array<{ name?: string; uri: string; title?: string; description?: string; mimeType?: string; thumbnail?: string; }>; }; };
119-121: Effect deps should include connection; avoid firing before integration is readyPrevent premature searches; also avoids unhandled rejections.
Apply this diff:
-useEffect(() => { - runSearch(q); -}, [q]); +useEffect(() => { + if (integration?.connection) runSearch(q); +}, [integration?.connection, q]);
279-283: Regex allows many punctuation due to a hyphen range; sanitize bugIn a character class,
0-9-_creates the range9-_. This unintentionally whitelists many chars, so they won’t be replaced. Use0-9_-or escape-.Apply this diff:
- .replace(/[^a-zA-Z0-9-_]/g, "-") + .replace(/[^a-zA-Z0-9_-]/g, "-")packages/sdk/src/mcp/deconfig/branch.ts (1)
1121-1124: Avoid spreading large Uint8Array into fromCharCode; can crash
String.fromCharCode(...uint8Array)overflows for big files. Use chunked encoding.Apply this diff:
- if (content) { - const uint8Array = new Uint8Array(content); - enrichedMetadata.content = btoa(String.fromCharCode(...uint8Array)); - } + if (content) { + const u8 = new Uint8Array(content); + let binary = ""; + const CHUNK = 0x8000; + for (let i = 0; i < u8.length; i += CHUNK) { + binary += String.fromCharCode(...u8.subarray(i, i + CHUNK)); + } + enrichedMetadata.content = btoa(binary); + }If preferred, we can extract a toBase64(u8: Uint8Array) helper and reuse.
🧹 Nitpick comments (47)
packages/sdk/src/mcp/bindings/binder.ts (1)
12-12: Consider replacinganywithunknownwhere feasible.While not introduced by this PR, the coding guidelines recommend avoiding
anyin favor of more specific types. Consider whetherunknownor more constrained types could replaceanyin the generic defaults and constraints (e.g., line 12:TDefinition extends readonly ToolBinder[] = any, line 18:TContext = any).This would improve type safety without significantly impacting the API surface, though it may require downstream adjustments.
As per coding guidelines.
Also applies to: 17-18, 98-98
apps/web/src/components/layout/decopilot-layout.tsx (1)
38-38: LGTM! Regex pattern simplified.The removal of redundant escapes inside the character class improves readability. The pattern remains functionally equivalent.
Note: This same regex pattern appears in
apps/web/src/components/layout/project.tsxat line 248. Consider extracting it to a shared constant to avoid duplication and ensure consistency across both files.apps/web/src/components/layout/project.tsx (1)
248-248: LGTM! Consider extracting the duplicated pattern.The regex simplification is correct and improves readability. However, this exact pattern also appears in
apps/web/src/components/layout/decopilot-layout.tsxat line 38.Consider extracting this pattern to a shared constant or utility function to maintain consistency and reduce duplication:
// utils/path-matchers.ts or similar export const AGENT_DETAIL_PAGE_PATTERN = /\/agent\/[^/]+\/[^/]+$/;Then import and use it in both files:
+import { AGENT_DETAIL_PAGE_PATTERN } from "../../utils/path-matchers"; + export const TopbarControls = () => { const location = useLocation(); const isProjectContext = useIsProjectContext(); - const isAgentDetailPage = location.pathname.match(/\/agent\/[^/]+\/[^/]+$/); + const isAgentDetailPage = location.pathname.match(AGENT_DETAIL_PAGE_PATTERN);apps/web/src/components/prompts/rich-text/index.tsx (2)
145-153: Remove suppression; type props precisely for MentionNode wrapperUse React.ComponentProps to avoid
anyand drop the oxlint disable. As per coding guidelines.Apply within this hunk:
- // oxlint-disable-next-line no-explicit-any - (props: any) => ( + (props: ComponentProps<typeof MentionNode>) => (And add a type-only import (outside this hunk):
-import { useEffect, useMemo, useRef, useCallback } from "react"; +import { useEffect, useMemo, useRef, useCallback, type ComponentProps } from "react";
157-163: Same here: precise props type for MentionDropdownMirror the fix to remove
anyand the oxlint suppression. As per coding guidelines.Apply within this hunk:
- // oxlint-disable-next-line no-explicit-any - (props: any) => ( + (props: ComponentProps<typeof MentionDropdown>) => (apps/web/src/components/rich-text-editor/components/mention-dropdown.tsx (1)
24-26: Type IntegrationAvatar to remove the any + directiveYou can drop the oxlint disable by typing props from the callsites.
Apply this inline change:
- // oxlint-disable-next-line no-explicit-any - IntegrationAvatar?: React.ComponentType<any>; + IntegrationAvatar?: React.ComponentType<{ + url?: string; + fallback?: string; + size?: "xs" | "sm" | "md" | "lg"; + className?: string; + }>;packages/runtime/src/proxy.ts (1)
86-88: Timeout seems excessively high (~50 minutes)3000000 ms may exceed platform limits (e.g., Workers). Consider a conservative default (e.g., 120_000) or make it configurable.
- { - timeout: 3000000, - }, + { + timeout: options?.timeoutMs ?? 120_000, + },packages/sdk/src/mcp/file-processor.ts (2)
375-381: Fix fractional slice/step when splitting long sentencesUsing this.config.chunkSize / 10 yields a fractional step; slice indexes are coerced and can produce uneven chunks.
Use an integer step tied to your 6-char word heuristic:
- const words = trimmedSentence.split(/\s+/); - for (let i = 0; i < words.length; i += this.config.chunkSize / 10) { - const chunk = words - .slice(i, i + this.config.chunkSize / 10) - .join(" "); - chunks.push({ content: chunk }); - } + const words = trimmedSentence.split(/\s+/); + const step = Math.max(1, Math.floor(this.config.chunkSize / 6)); + for (let i = 0; i < words.length; i += step) { + const chunk = words.slice(i, i + step).join(" "); + chunks.push({ content: chunk }); + }
199-201: Minor: prefer unknown over any hereIf you keep the lint rule on, consider narrowing to unknown and refining locally for better type safety.
- // oxlint-disable-next-line no-explicit-any - private chunkLongStringsInObject(json: any): string { + private chunkLongStringsInObject(json: unknown): string { + const obj = json as Record<string, unknown>; + // ...use obj instead of json...apps/api/src/durable-objects/workspace-database.ts (1)
19-21: Avoid ban-types suppression by using unknownSwitch DurableObjectState<{}> to DurableObjectState and remove the directive.
- // oxlint-disable-next-line ban-types - protected override ctx: DurableObjectState<{}>, + protected override ctx: DurableObjectState<unknown>,apps/api/main.ts (1)
90-99: Reuse the Postgres client across requestsCreating a new client per request can exhaust connections. Cache at module scope.
- fetch(request: Request, env: any, ctx: ExecutionContext): Promise<Response> { - const sql = contextStorage.getStore()?.sql ?? createPostgres(env); + // module scope cache + let cachedSql: ReturnType<typeof postgres> | undefined; + fetch(request: Request, env: any, ctx: ExecutionContext): Promise<Response> { + const sql = contextStorage.getStore()?.sql ?? (cachedSql ??= createPostgres(env));packages/sdk/src/mcp/api-keys/api.ts (2)
385-391: Narrow updateData type to avoid anyUse a precise shape to keep type-safety and drop the oxlint suppression.
- // oxlint-disable-next-line no-explicit-any - const updateData: Record<string, any> = {}; + const updateData: Partial<{ + name: string; + enabled: boolean; + policies: Statement[]; + updated_at: string; + }> = {};
165-167: Optional: avoid any in ensureStateIsWellFormedYou can cast the binding object locally without polluting the broader type.
- (prop as any)["__type"] = integration.appName; // ensure it's a binding object + (prop as Record<string, unknown>)["__type"] = integration.appName; // ensure it's a binding objectpackages/ai/src/utils/json-schema-to-model.ts (1)
4-5: Reduce surface any usage in JSON schema helpersTyping inputs as records improves safety without changing behavior.
Examples:
-export function jsonSchemaPropertiesToTSTypes(value: any): z.ZodTypeAny { +export function jsonSchemaPropertiesToTSTypes(value: Record<string, unknown>): z.ZodTypeAny { @@ -export function jsonSchemaToModel( - jsonSchema: Record<string, any>, -): ZodObject<any> { +export function jsonSchemaToModel( + jsonSchema: Record<string, unknown>, +): ZodObject<z.ZodRawShape> { @@ - const zodSchema: Record<string, any> = {}; - for (const [key, _] of Object.entries(properties)) { - const value = _ as any; + const zodSchema: Record<string, z.ZodTypeAny> = {}; + for (const [key, _] of Object.entries(properties)) { + const value = _ as Record<string, unknown>; @@ - const anyOfTypes = value.anyOf.map((schema: any) => + const anyOfTypes = (value.anyOf as unknown[]).map((schema) => jsonSchemaPropertiesToTSTypes(schema), );Also applies to: 78-80, 86-89, 91-104
packages/runtime/src/client.ts (2)
130-136: SSR safety: guard globalThis.location usageIf createClient is imported server‑side, globalThis.location may be undefined. Add a guard or accept a base URL via init for non‑browser contexts.
Example:
- const watchUrl = new URL(watchPathname, globalThis.location.origin); + const base = (globalThis as any)?.location?.origin ?? ""; + const watchUrl = new URL(watchPathname, base);
23-30: Reduce any: prefer unknown with constrained genericsType surfaces use any to satisfy mapping; consider unknown and tightening with generic constraints to preserve safety.
Sketch:
-type SubscribeMethods<T> = { +type SubscribeMethods<T extends Record<string, (...a: unknown[]) => unknown>> = { [K in keyof T as K extends `DECO_RESOURCE_${string}_READ` ? SubscribeMethodName<ExtractResourceName<K>> : never]: K extends `DECO_RESOURCE_${string}_READ` - ? // oxlint-disable-next-line no-explicit-any - T[K] extends (...args: any) => any + ? T[K] extends (...args: infer _A) => infer _R ? (args: { id: string } | { uri: string }) => AsyncIterableIterator<{ uri: string; data: ExtractReadData<Awaited<ReturnType<T[K]>>>; }> : never : never; }; -export type MCPClient<T> = { - // oxlint-disable-next-line no-explicit-any - [K in keyof T]: T[K] extends (...args: any) => any +export type MCPClient<T extends Record<string, (...a: unknown[]) => unknown>> = { + [K in keyof T]: T[K] extends (...args: infer _A) => infer _R ? ( args: Parameters<T[K]>[0], init?: CustomInit, ) => Promise<Awaited<ReturnType<T[K]>>> : never; } & SubscribeMethods<T>;Also applies to: 33-41
apps/api/tail.ts (3)
427-433: Make OTEL header parsing robust (trim and single ‘=’ split)Current split can mis-handle values containing '=' and lacks trimming.
Apply this diff:
-const otelHeadersToObject = (headers: string) => { - const headersParts = headers - .split(",") - .map((kv) => kv.split("=") as [string, string]); - return Object.fromEntries(headersParts); -}; +const otelHeadersToObject = (headers: string) => { + const entries = headers.split(",").map((kv) => { + const i = kv.indexOf("="); + const k = (i === -1 ? kv : kv.slice(0, i)).trim(); + const v = (i === -1 ? "" : kv.slice(i + 1)).trim(); + return [k, v] as const; + }); + return Object.fromEntries(entries); +};
454-456: Avoid noisy console logs in API pathAlways logging OTLP response status can be chatty. Gate behind a DEBUG flag or lower log level.
Example:
- }).then((res) => { - console.log("Traces response:", res.status); - }); + }).then((res) => { + if (process.env.DEBUG?.includes("tail")) { + console.log("Traces response:", res.status); + } + });(Apply similarly to Logs.)
Also applies to: 467-468
471-492: Parallelize per-event ingests to reduce latencyTraces, logs, and exceptions can be sent concurrently.
Example:
- await ingestLogs(requestLog); - if (event.traces && event.traces.length > 0) { - const traceData = convertTailTraces(event); - traceData && (await ingestTraces(traceData)); - } - if (event.logs && event.logs.length > 0) { - const logData = convertTailLogs(event); - logData && (await ingestLogs(logData)); - } - if (event.exceptions && event.exceptions.length > 0) { - const exceptionData = convertTailExceptions(event); - exceptionData && (await ingestLogs(exceptionData)); - } + const ops: Array<Promise<unknown>> = [ingestLogs(requestLog)]; + if (event.traces?.length) { + const d = convertTailTraces(event); + if (d) ops.push(ingestTraces(d)); + } + if (event.logs?.length) { + const d = convertTailLogs(event); + if (d) ops.push(ingestLogs(d)); + } + if (event.exceptions?.length) { + const d = convertTailExceptions(event); + if (d) ops.push(ingestLogs(d)); + } + await Promise.all(ops);packages/create-deco/index.js (2)
18-26: Spawn the exact Node binary via process.execPathMore robust than hardcoding "node" (PATH issues). Keep Bun path when actually running under Bun.
Apply this diff:
function detectRuntime() { if (typeof Bun !== "undefined") { return "bun"; } - return "node"; + return process.execPath || "node"; } @@ -const child = spawn(runtime, [decoCli, ...commandArgs], { +const child = spawn(runtime, [decoCli, ...commandArgs], { stdio: "inherit", cwd: process.cwd(), });Also applies to: 68-71
1-13: Prefer TypeScript in packages/Repo guideline: “Use TypeScript for all code (no JavaScript files)” and kebab‑case in packages/. Consider moving to TS with a tiny build step that outputs the Node‑shebang JS in dist.
As per coding guidelines.
packages/sdk/src/hooks/tools.ts (2)
61-76: Remove any from queryKey by using a typed discriminator helperUse property checks instead of casting to any.
Apply this diff:
export function useTools(connection: MCPConnection, ignoreCache?: boolean) { + const connId = + "url" in connection + ? connection.url + : "tenant" in connection + ? (connection as { tenant: string }).tenant + : "name" in connection + ? (connection as { name: string }).name + : "unknown"; const response = useQuery({ retry: false, queryKey: [ "tools", connection.type, - // oxlint-disable-next-line no-explicit-any - (connection as any).url || - // oxlint-disable-next-line no-explicit-any - (connection as any).tenant || - // oxlint-disable-next-line no-explicit-any - (connection as any).name, + connId, ignoreCache, ],
48-59: Avoid any in callTool paramstoolCallArgs is already typed; cast to a safer indexable type rather than any.
Apply this diff:
return client.INTEGRATIONS_CALL_TOOL({ ...("id" in connection ? { id: connection.id } : { connection }), - // oxlint-disable-next-line no-explicit-any - params: toolCallArgs as any, + params: toolCallArgs as Record<string, unknown>, });packages/runtime/src/deprecated.ts (1)
7-9: Minor: avoid default any in genericYou can default to z.ZodTypeAny to drop the disable comment.
Apply this diff:
-// oxlint-disable-next-line no-explicit-any -export interface DeprecatedEnv<TSchema extends z.ZodTypeAny = any> { +export interface DeprecatedEnv<TSchema extends z.ZodTypeAny = z.ZodTypeAny> {apps/web/src/components/settings/profile.tsx (1)
1-1: Scope down the lint disable; replaceanywith precise types.File-wide
oxlint-disable no-explicit-anyis over-broad. Type the inputs and error locally, then drop the disable.-/* oxlint-disable no-explicit-any */ +// (keep file clean — avoid broad disables) -function getCountryConfig(country: any): Country { +function getCountryConfig( + country: { dial_code: string } & Record<string, unknown>, +): Country { - onError: (err: any) => - setError(err.message || "Failed to update profile"), + onError: (err: unknown) => + setError(err instanceof Error ? err.message : "Failed to update profile"),Also applies to: 30-31, 113-115
apps/api/src/api.ts (1)
153-156: Removeanyby narrowing the error type.No need for an
anycast here; narrow to the field you need and drop the disable.- // oxlint-disable-next-line no-explicit-any - const cause = (err as any as { detail?: unknown }).detail; + const cause = (err as { detail?: unknown }).detail;apps/web/src/hooks/analytics.ts (1)
31-34: Useunknown[]and drop the disable.Tighten the generic without losing flexibility.
- // oxlint-disable-next-line no-explicit-any - <T extends (...args: any[]) => void>(callback: T) => + <T extends (...args: unknown[]) => void>(callback: T) =>packages/ai/src/agent/provider-options.ts (1)
8-10: Preferunknownoveranyfor the options bag.Keeps it type-safe while remaining flexible; the disable becomes unnecessary.
- // oxlint-disable-next-line no-explicit-any - const opts: Record<string, any> = {}; + const opts: Record<string, unknown> = {};apps/web/src/components/views/internal-resource-list.tsx (2)
144-146: Bare catch is fine; consider debug loggingOptional: log at debug level to aid support without user noise.
-} catch { - // ignore; default capabilities remain false -} +} catch { + // ignore; default capabilities remain false + // console.debug("Capabilities probe failed"); +}
185-196: Fallback path is good; consider capturing error for observabilityKeeping the fallback, add a debug trace so repeated failures are diagnosable.
-} catch { +} catch { // fallback: internal detail + // console.debug("OpenItem remote view resolution failed; falling back to internal detail");packages/cli/src/lib/session.ts (1)
95-99: Differentiate ENOENT vs other unlink errors (optional)Currently logs “Session file not found” for all errors. Consider checking code === "ENOENT" and logging actual error otherwise.
-} catch { - console.warn("Session file not found"); +} catch (e) { + const code = (e as { code?: string }).code; + console.warn(code === "ENOENT" ? "Session file not found" : `Failed to delete session file: ${String(e)}`); }packages/sdk/src/mcp/deconfig/branch.ts (3)
823-833: notifyWatchers always emits “added” even for modificationsFor entries in patch.added that overwrite existing paths, emit type "modified" to match FileChangeEvent. Today watchers cannot distinguish adds vs updates.
Example adjustment:
for (const [path, metadata] of Object.entries(patch.added)) { const type = this.state.tree[path] ? "modified" : "added"; // compute before mutating state, or capture a prePatchTree events.push({ type, path, metadata, timestamp: patch.timestamp, patchId: patch.id }); }You may need to compute type before updating this.state.tree in patch() for correctness. Based on learnings.
52-54: Prefer unknown over any for metadata bagKeeps unsafeness localized and aligns with “prefer specific types” guidance.
- // oxlint-disable-next-line no-explicit-any - metadata: Record<string, any>; + metadata: Record<string, unknown>;
1453-1455: Remove unnecessary any cast in diffThis narrower cast keeps types intact without disabling lint.
-// oxlint-disable-next-line no-explicit-any -const aMeta = otherTree[path] as any as FileMetadata; // desired +const aMeta = otherTree[path] as FileMetadata | undefined; // desiredpackages/runtime/src/workflow.ts (1)
56-61: Avoid {} generic; remove ban-types suppressionUse Record<string, never> (or unknown) instead of
{}to drop the disable.-// oxlint-disable-next-line ban-types -public override ctx: DurableObjectState<{}>, +public override ctx: DurableObjectState<Record<string, never>>,apps/web/src/components/chat/rich-text.tsx (2)
149-159: Type wrapper props instead of anyKeep strong types without relaxing extension expectations.
-// oxlint-disable-next-line no-explicit-any -return function MentionNodeWrapper(props: any) { +import type { ComponentProps } from "react"; +return function MentionNodeWrapper( + props: ComponentProps<typeof MentionNode>, +) {
163-190: Type dropdown wrapper; consider stable closure for appendIntegrationToolTyping: same as above. For stability, if appendIntegrationTool can change, wrap it in a ref to avoid stale captures while keeping the memo’s empty deps.
-// oxlint-disable-next-line no-explicit-any -return function MentionDropdownWrapper(props: any) { +import type { ComponentProps } from "react"; +return function MentionDropdownWrapper( + props: ComponentProps<typeof MentionDropdown>, +) {Optional stabilization:
// before useMemo const appendToolRef = useRef(appendIntegrationTool); useEffect(() => { appendToolRef.current = appendIntegrationTool; }, [appendIntegrationTool]); // inside wrappedCommand appendToolRef.current(item.tool.integration.id, readToolNameOrToolName);Confirm appendIntegrationTool is stable; if yes, typing fix alone is enough.
packages/cli/src/lib/config.ts (1)
111-113: Prefer debug logging instead of fully silent catch in readWranglerConfigSilent failures make troubleshooting harder. Gate a debug log behind an env flag.
- } catch { - return {}; - } + } catch (e) { + if (process.env.DEBUG?.includes("deco:config")) { + console.debug(`readWranglerConfig: failed to read ${configPath}`, e); + } + return {}; + }apps/web/src/components/json-schema/components/type-select-field.tsx (2)
70-73: Remove any by typing the field path with react-hook-formTyping name as FieldPath lets you drop the any cast and lint suppression.
Apply:
- const selectedOption = options.find( - // oxlint-disable-next-line no-explicit-any - (option: OptionItem) => option.value === form.getValues(name as any)?.value, - ); + const selectedOption = options.find( + (option: OptionItem) => option.value === form.getValues(name as FieldPath<T>)?.value, + );And update the prop type to avoid downstream casts (outside this hunk):
-interface TypeSelectFieldProps<T extends FieldValues = FieldValues> { - name: string; +interface TypeSelectFieldProps<T extends FieldValues = FieldValues> { + name: FieldPath<T>;As per coding guidelines.
132-134: Type-safe setValue without anyLeverage PathValue to type the assigned object.
Add import:
-import type { FieldPath, FieldValues, UseFormReturn } from "react-hook-form"; +import type { FieldPath, FieldValues, UseFormReturn, PathValue } from "react-hook-form";Then:
- // oxlint-disable-next-line no-explicit-any - form.setValue(name as FieldPath<T>, { value: connection.id } as any); + form.setValue( + name as FieldPath<T>, + { value: connection.id } as PathValue<T, FieldPath<T>>, + );If PathValue is too strict for your form schema, keep a narrow helper:
function setOptionValue<P extends FieldPath<T>>(path: P, id: string) { form.setValue(path, { value: id } as unknown as PathValue<T, P>); }As per coding guidelines.
apps/api/src/auth/index.ts (2)
103-126: Consider logging the error for debugging.While removing the unused catch parameter is appropriate for lint compliance, the error is completely discarded. This can make debugging authentication issues difficult.
Consider logging the error:
- } catch { + } catch (error) { + console.error("Failed to create magic link:", error); return { error: "" }; }
183-208: Consider logging the error for debugging.Similar to the previous catch block, the error is completely discarded, making it difficult to diagnose authentication failures.
Consider logging the error:
- } catch { + } catch (error) { + console.error("Failed to send magic link:", error); return ctx.json({ error: "" }, 400); }packages/runtime/src/index.ts (1)
191-195: Redundant nullish coalescing; simplify boolean.
(A || B) ?? falseis always boolean;?? falseis unnecessary.- env["IS_LOCAL"] = - (url?.startsWith("http://localhost") || - url?.startsWith("http://127.0.0.1")) ?? - false; + env["IS_LOCAL"] = + url?.startsWith("http://localhost") || + url?.startsWith("http://127.0.0.1");packages/ai/src/agent.ts (1)
243-246: Remove non-null assertion on user; pass optional value without disabling rule.
metadata?.user!defeats type safety and required a disable. Prefer forwarding optional user.- // TODO(@viktormarinho): Remove the ! - // eslint-disable-next-line @typescript-eslint/no-non-null-asserted-optional-chain - user: metadata?.user!, + user: metadata?.user,If
PrincipalExecutionContext.useris non-optional, consider making it optional or providing a safe default upstream. Based on learnings.package.json (3)
16-16: Make build script cross‑platform.Using
PWD=$PWD/apps/webis POSIX‑only. Prefer workspace build.- "build": "PWD=$PWD/apps/web npm run build", + "build": "npm run build --workspace=apps/web",
9-9: Consider failing on warnings to keep lint strict.Optional: enforce zero warnings in CI.
- "lint": "oxlint", + "lint": "oxlint --max-warnings=0",
18-18: Clean script misses Bun’s default lock file.Bun uses
bun.lockb. Consider removing both.- "clean": "rm -f deno.lock package-lock.json bun.lock && rm -rf node_modules apps/*/node_modules packages/*/node_modules", + "clean": "rm -f deno.lock package-lock.json bun.lock bun.lockb && rm -rf node_modules apps/*/node_modules packages/*/node_modules",
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
package.json(2 hunks)packages/cli/package.json(1 hunks)packages/runtime/jsr.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/runtime/jsr.json
🧰 Additional context used
📓 Path-based instructions (1)
packages/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep shared logic (UI kit, SDK, runtime, CLI tooling) under packages/
Files:
packages/cli/package.json
🔇 Additional comments (5)
packages/cli/package.json (1)
3-3: Version bump is correctly applied.The minor version increment aligns with the PR's broader tooling migration and follows semantic versioning conventions.
package.json (4)
2-2: Verify the package name change is intentional.The package name has changed from
@deco/chatto@decocms/admin, which is a breaking change for any existing consumers. Verify that:
- This rename aligns with the PR's stated objectives (which focus on oxlint migration)
- The change is coordinated with npm registry and any existing package publications
- Downstream dependencies have been updated or will be notified
9-9: Lint script migration is clean.Direct replacement of
deno lintwithoxlintaligns with the PR's tooling migration goals. Ensure oxlint configuration exists in.oxlintrc.jsonor equivalent.
11-15: New test, formatting, and docs scripts are well-structured.The additions follow standard patterns and align with the tooling migration goals:
"test"and"test:watch"enable vitest-based testing workflows"fmt:check"supports format verification"docs:dev"and"docs:deploy"restore/add documentation workflows
33-33: Oxlint version is appropriately pinned.The exact version pinning (1.23.0) is consistent with other build/dev tooling (biome, vitest, wrangler) and ensures reproducible linting behavior across environments.
- Reintroduced nullish coalescing in multiple spread operations across various files to ensure safe handling of potentially undefined values. - Updated components and utility functions in `apps/web`, `packages/ai`, `packages/cli`, `packages/runtime`, and `packages/sdk` to maintain consistency and prevent runtime errors. These changes enhance the robustness of the code by ensuring that undefined values do not lead to unintended behavior.
- Deleted the `NULLISH_COALESCING_REMOVALS.md` file, which documented the nullish coalescing removals from spread operations. - This cleanup reflects the completion of the related refactoring work and helps maintain a tidy documentation structure. No further action is required as the relevant changes have been implemented in the codebase.
…xlintrc.json - Turned off the `unicorn/no-useless-fallback-in-spread` rule in the linting configuration to prevent unnecessary warnings during development. - This adjustment aligns the linting rules with current project needs and enhances the developer experience. No further action is required as this change is purely a configuration update.
1 participant
Note: This movement also takes in account that we want to use Vite+ once its available, for linting formatting and everything. Oxlint is what Vite+ will use under the hood, besides being an improvement from the current deno lint since users no longer need another dependency to develop on the repository.
deno lintwithoxlintfor linting.bun.lockbto reflect changes in dependencies.These changes streamline the development process and improve code maintainability.
What is this contribution about?
Screenshots/Demonstration
Review Checklist
Summary by CodeRabbit
Release Notes
New Features
Chores