Fix diagnostic snippet source path resolution#19
Merged
warunalakshitha merged 2 commits intoballerina-platform:mainfrom Apr 1, 2026
Merged
Fix diagnostic snippet source path resolution#19warunalakshitha merged 2 commits intoballerina-platform:mainfrom
warunalakshitha merged 2 commits intoballerina-platform:mainfrom
Conversation
|
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:
📝 WalkthroughWalkthroughThese changes add error handling to shared file loading and introduce a helper function for resolving source file paths in diagnostics printing. The file-tree store now gracefully handles failures when loading shared files, while the diagnostics printer uses a new utility to determine the correct source file path based on available context. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 6
🧹 Nitpick comments (8)
apps/web/src/styles.css (1)
153-160: Redundantfont-sanson both*andbody.The
font-sansis now applied to both the universal*selector (Line 155) andbody(Line 158). The*selector already coversbody, making thebodydeclaration redundant. Consider removingfont-sansfrom one location.Option: Remove from body if keeping on *
body { - `@apply` font-sans bg-background text-foreground; + `@apply` bg-background text-foreground; }Note: Using
*for font-family is more explicit but less performant than relying on inheritance fromhtmlorbody. If all children should inherit the font, applying tobodyalone and relying on CSS inheritance is generally preferred.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/styles.css` around lines 153 - 160, The CSS repeats font-sans on both the universal selector (*) and body inside the `@layer` base block; remove the redundant font-sans from one place—preferably delete font-sans from the * rule (or alternatively remove it from body) so only a single declaration applies (refer to the `@layer` base block and the selectors "*" and "body" to locate the change).packages/wasm/package.json (1)
4-7: Add directory creation tobuildscript to ensuredistexists before compilation.The
go build -o dist/ballerina.wasmcommand will fail if thedistdirectory doesn't exist. Sincecleanremoves this directory, explicitly create it during the build:Proposed fix
"scripts": { "clean": "rm -rf dist", - "build": "GOOS=js GOARCH=wasm go build -o dist/ballerina.wasm ." + "build": "mkdir -p dist && GOOS=js GOARCH=wasm go build -o dist/ballerina.wasm ." }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wasm/package.json` around lines 4 - 7, Update the "build" script in package.json to ensure the dist directory is created before running the Go build: modify the scripts.build entry (paired with the existing "clean" script) so it creates the dist directory if missing and then runs GOOS=js GOARCH=wasm go build -o dist/ballerina.wasm .; this guarantees the output path exists and prevents build failures when "clean" removed dist.apps/web/package.json (1)
9-9: Consider cross-platform compatibility for the copy script.The
cpcommand is Unix-specific and won't work on Windows. Since Turbo manages this task viaapps/web/turbo.jsonand CI runs on Ubuntu, this is acceptable if Windows development isn't a requirement. However, for broader compatibility, consider using a cross-platform alternative.♻️ Optional: Cross-platform alternative using Node.js built-in
-"copy:wasm": "cp ../../packages/wasm/dist/ballerina.wasm public/ballerina.wasm" +"copy:wasm": "node -e \"require('fs').cpSync('../../packages/wasm/dist/ballerina.wasm', 'public/ballerina.wasm')\""Or add a small copy script using
shxor similar cross-platform tool.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/package.json` at line 9, The "copy:wasm" npm script uses the Unix-only `cp` command; replace it with a cross-platform approach by either (a) installing a cross-platform CLI like "shx" and updating the "copy:wasm" script to use "shx cp ../../packages/wasm/dist/ballerina.wasm public/ballerina.wasm", or (b) implement a tiny Node-based copy script (e.g., a script file referenced by the "copy:wasm" package.json script that uses fs.copyFileSync) and update the "copy:wasm" script to call that file; update package.json to add the new devDependency or script entry and ensure turbo.json/task invocation still points to the "copy:wasm" script name.apps/web/src/components/ui/checkbox.tsx (1)
8-26: Consider forwarding ref for focus management.The
Checkboxcomponent doesn't forward a ref, which may limit programmatic focus control or integration with form libraries that need ref access to the underlying checkbox element. In React 19,refis a regular prop, so you can simply destructure and pass it.♻️ Optional: forward ref to the primitive
-function Checkbox({ className, ...props }: CheckboxPrimitive.Root.Props) { +function Checkbox({ className, ref, ...props }: CheckboxPrimitive.Root.Props) { return ( <CheckboxPrimitive.Root + ref={ref} data-slot="checkbox"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/ui/checkbox.tsx` around lines 8 - 26, The Checkbox component should forward its ref so consumers can programmatically focus the underlying DOM node; update the Checkbox declaration to use React.forwardRef (keep the same props shape) so it accepts a ref param and pass that ref into <CheckboxPrimitive.Root ref={ref} ...>, ensuring the ref type matches the primitive root element (use the appropriate React/CheckboxPrimitive Root element/ref type); keep all existing props spread and className handling and leave CheckboxPrimitive.Indicator and HugeiconsIcon unchanged.apps/web/src/hooks/use-share.ts (1)
33-37: Resettingprocessed.currentwhen share is absent may cause re-processing.When
sharebecomes falsy (e.g., afterdropShareParam()), this code setsprocessed.current = null. However, if the component re-renders before the navigation completes andshareis still present momentarily, this could cause the same share to be processed again.Consider only resetting when
shareis absent and not when!ready:Proposed fix
React.useEffect(() => { - if (!ready || !share) { - processed.current = null; + if (!ready) return; + if (!share) { + if (processed.current !== null) processed.current = null; return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/hooks/use-share.ts` around lines 33 - 37, The effect in use-share currently resets processed.current whenever either !ready or !share, which can cause re-processing; update the React.useEffect in use-share to only clear processed.current when share is falsy (e.g., after dropShareParam()) and avoid clearing on !ready — e.g., check share first and if (!share) set processed.current = null and return, then separately return early on !ready so the same share isn't nulled while navigation is pending; modify the effect around processed.current, ready, and share in useShare hook accordingly.apps/web/src/components/ui/sonner.tsx (1)
58-65: Consider using a valid CSS value for--border-radius.The value
"none"is a string, which may not be the intended CSS value for border-radius. If you want no border radius, use"0"instead. The string"none"is not a valid CSS<length>value for border-radius.Proposed fix
style={ { "--normal-bg": "var(--popover)", "--normal-text": "var(--popover-foreground)", "--normal-border": "var(--border)", - "--border-radius": "none", + "--border-radius": "0", } as React.CSSProperties }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/ui/sonner.tsx` around lines 58 - 65, The inline style for the Sonner UI uses an invalid CSS value for border-radius ("none"); update the style object in the component that sets "--border-radius" (the style passed to the Sonner/toast root in apps/web/src/components/ui/sonner.tsx) to use a valid CSS length such as "0" or "0px" instead of "none" and keep the cast to React.CSSProperties intact so the property remains typed correctly.apps/web/src/lib/file-tree-utils.ts (1)
18-21: Minor: Redundant kind check.The
dir.kind !== "dir"check on line 20 is redundant since thefindpredicate on line 19 already ensuresn.kind === "dir". However, TypeScript may need this for type narrowing, so it's acceptable to keep.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/lib/file-tree-utils.ts` around lines 18 - 21, The code performs a redundant runtime check `dir.kind !== "dir"` after using find with `n.kind === "dir"`, so replace the find call with a proper type predicate to satisfy TypeScript and then remove the second check: change the predicate to something like `nodes.find((n): n is Dir => n.kind === "dir" && n.name === seg)` (use your Dir type), then you can safely remove `dir.kind !== "dir'` and keep using `dir.children` and `nodes = dir.children ?? [];` referencing `segments`, `nodes`, and `dir`.apps/web/src/lib/share.ts (1)
15-23: Await the stream writer promises instead of discarding them.
writer.write()andwriter.close()return promises per the Streams API spec. Usingvoidhere bypasses these operations and can lead to unhandled errors. Awaiting them ensures proper error handling and ordering—errors from the writable side are surfaced before consuming the readable stream.♻️ Suggested change
async function compressAndEncode(text: string): Promise<string> { const bytes = new TextEncoder().encode(text); const stream = new CompressionStream("gzip"); const writer = stream.writable.getWriter(); - void writer.write(bytes); - void writer.close(); + await writer.write(bytes); + await writer.close(); const compressed = new Uint8Array( await new Response(stream.readable).arrayBuffer(), ); @@ async function decodeAndDecompress(b64: string): Promise<string> { const bytes = Uint8Array.from(atob(b64), (c) => c.charCodeAt(0)); const stream = new DecompressionStream("gzip"); const writer = stream.writable.getWriter(); - void writer.write(bytes); - void writer.close(); + await writer.write(bytes); + await writer.close(); const decompressed = await new Response(stream.readable).arrayBuffer(); return new TextDecoder().decode(decompressed); }Also applies to: lines 33-40
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/lib/share.ts` around lines 15 - 23, In compressAndEncode, don't discard the promises from stream.writable.getWriter(); await the writer.write(bytes) and await writer.close() so write/close errors surface and ordering is correct; update the same pattern in the other compression block in this file (the second occurrence of writer.write/writer.close) to await those promises as well and keep using the resulting Response(stream.readable).arrayBuffer() only after the writer has resolved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/biome.json`:
- Around line 5-13: Replace the accidental re-include pattern "!!dist/" in the
"includes" array with the exclusion pattern "!dist/" so the build output
directory is excluded from Biome checks; update the includes entry that
currently reads "!!dist/" to "!dist/" in the includes array to prevent
linting/formatting from processing dist/.
In `@apps/web/src/components/app-sidebar.tsx`:
- Around line 381-382: The check noLocalFiles && index === 0 inside
localTree.map is unreachable because localTree.map only runs when
localTree.length > 0; instead compute a flag like shouldAutoExpandFirst =
localTree.length === 0 && remoteTree.length > 0 and use that flag when rendering
the remoteTree (or whichever map runs when localTree is empty) so the first
visible entry can auto-expand; update both occurrences that currently use
noLocalFiles (the localTree.map block and the similar block around the other
occurrence) to reference shouldAutoExpandFirst and use index === 0 there.
In `@apps/web/src/hooks/use-share.ts`:
- Around line 43-65: The promise returned by decodeSharePayload(share) lacks
rejection handling; update the call site to add a .catch handler (or use
try/catch with async/await) for decodeSharePayload to handle thrown errors,
clear processed.current, call dropShareParam(), and surface the error (e.g.,
toast.error or processLogger) so rejections don't go unhandled; keep existing
success-path logic that calls loadSharedFiles(payload.root,
payload.openRelativePath), uses processed.current = loaded ? share : null,
toasts on failure, calls openFile(openPath) and showNotice() when appropriate,
and ensure dropShareParam() always runs even on error.
In `@apps/web/src/lib/fs/fs-roots.ts`:
- Around line 1-7: fs-roots.ts currently imports join from path-utils which in
turn imports LOCAL_ROOT/SHARED_ROOT creating a circular dependency; fix it by
removing the dependency on join in fs-roots.ts and either inline the simple join
logic (e.g., concatenate with "/") when computing EXAMPLES_ROOT and SHARED_ROOT
or move join into a new low-level utility module that both fs-roots.ts and
path-utils.ts can import; update the definitions of EXAMPLES_ROOT and
SHARED_ROOT (which use TEMP_ROOT) to use the inlined/relocated join so they no
longer import join from path-utils and break the cycle.
In `@apps/web/src/routes/__root.tsx`:
- Around line 12-15: ThemeProvider is currently rendered without configuring the
theme attribute, but your Tailwind dark styles expect a .dark class; update the
ThemeProvider usage to include attribute="class" so next-themes toggles the
.dark class (modify the ThemeProvider element that wraps Outlet and Toaster).
Ensure you pass attribute="class" on the ThemeProvider component so class-based
dark mode is applied.
In `@packages/wasm/diagnostics_printer.go`:
- Line 25: The code uses filepath.Join to build paths for io/fs reads (e.g., the
call to filepath.Join at diagnostics_printer.go line where the FS read occurs);
change that call to path.Join so the logical FS path uses forward slashes
(import the "path" package and remove or keep "path/filepath" as needed),
ensuring all fs.FS ReadFile/Open calls use path.Join instead of filepath.Join to
avoid OS-specific separators breaking reads on Windows.
---
Nitpick comments:
In `@apps/web/package.json`:
- Line 9: The "copy:wasm" npm script uses the Unix-only `cp` command; replace it
with a cross-platform approach by either (a) installing a cross-platform CLI
like "shx" and updating the "copy:wasm" script to use "shx cp
../../packages/wasm/dist/ballerina.wasm public/ballerina.wasm", or (b) implement
a tiny Node-based copy script (e.g., a script file referenced by the "copy:wasm"
package.json script that uses fs.copyFileSync) and update the "copy:wasm" script
to call that file; update package.json to add the new devDependency or script
entry and ensure turbo.json/task invocation still points to the "copy:wasm"
script name.
In `@apps/web/src/components/ui/checkbox.tsx`:
- Around line 8-26: The Checkbox component should forward its ref so consumers
can programmatically focus the underlying DOM node; update the Checkbox
declaration to use React.forwardRef (keep the same props shape) so it accepts a
ref param and pass that ref into <CheckboxPrimitive.Root ref={ref} ...>,
ensuring the ref type matches the primitive root element (use the appropriate
React/CheckboxPrimitive Root element/ref type); keep all existing props spread
and className handling and leave CheckboxPrimitive.Indicator and HugeiconsIcon
unchanged.
In `@apps/web/src/components/ui/sonner.tsx`:
- Around line 58-65: The inline style for the Sonner UI uses an invalid CSS
value for border-radius ("none"); update the style object in the component that
sets "--border-radius" (the style passed to the Sonner/toast root in
apps/web/src/components/ui/sonner.tsx) to use a valid CSS length such as "0" or
"0px" instead of "none" and keep the cast to React.CSSProperties intact so the
property remains typed correctly.
In `@apps/web/src/hooks/use-share.ts`:
- Around line 33-37: The effect in use-share currently resets processed.current
whenever either !ready or !share, which can cause re-processing; update the
React.useEffect in use-share to only clear processed.current when share is falsy
(e.g., after dropShareParam()) and avoid clearing on !ready — e.g., check share
first and if (!share) set processed.current = null and return, then separately
return early on !ready so the same share isn't nulled while navigation is
pending; modify the effect around processed.current, ready, and share in
useShare hook accordingly.
In `@apps/web/src/lib/file-tree-utils.ts`:
- Around line 18-21: The code performs a redundant runtime check `dir.kind !==
"dir"` after using find with `n.kind === "dir"`, so replace the find call with a
proper type predicate to satisfy TypeScript and then remove the second check:
change the predicate to something like `nodes.find((n): n is Dir => n.kind ===
"dir" && n.name === seg)` (use your Dir type), then you can safely remove
`dir.kind !== "dir'` and keep using `dir.children` and `nodes = dir.children ??
[];` referencing `segments`, `nodes`, and `dir`.
In `@apps/web/src/lib/share.ts`:
- Around line 15-23: In compressAndEncode, don't discard the promises from
stream.writable.getWriter(); await the writer.write(bytes) and await
writer.close() so write/close errors surface and ordering is correct; update the
same pattern in the other compression block in this file (the second occurrence
of writer.write/writer.close) to await those promises as well and keep using the
resulting Response(stream.readable).arrayBuffer() only after the writer has
resolved.
In `@apps/web/src/styles.css`:
- Around line 153-160: The CSS repeats font-sans on both the universal selector
(*) and body inside the `@layer` base block; remove the redundant font-sans from
one place—preferably delete font-sans from the * rule (or alternatively remove
it from body) so only a single declaration applies (refer to the `@layer` base
block and the selectors "*" and "body" to locate the change).
In `@packages/wasm/package.json`:
- Around line 4-7: Update the "build" script in package.json to ensure the dist
directory is created before running the Go build: modify the scripts.build entry
(paired with the existing "clean" script) so it creates the dist directory if
missing and then runs GOOS=js GOARCH=wasm go build -o dist/ballerina.wasm .;
this guarantees the output path exists and prevents build failures when "clean"
removed dist.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5e5a77c6-f237-4bf6-a53e-3cb683614424
⛔ Files ignored due to path filters (3)
apps/web/public/ballerina.svgis excluded by!**/*.svgbun.lockis excluded by!**/*.lockpackages/wasm/go.sumis excluded by!**/*.sum
📒 Files selected for processing (80)
.github/workflows/ci.yml.github/workflows/deploy.yml.gitignore.gitmodulesapps/web/.gitignoreapps/web/biome.jsonapps/web/components.jsonapps/web/index.htmlapps/web/package.jsonapps/web/src/assets/examples.jsonapps/web/src/components/ansi.tsxapps/web/src/components/app-sidebar.tsxapps/web/src/components/code-editor.tsxapps/web/src/components/editor.tsxapps/web/src/components/file-route-sync.tsxapps/web/src/components/file-tree-dialog.tsxapps/web/src/components/share-notice-dialog.tsxapps/web/src/components/ui/button.tsxapps/web/src/components/ui/checkbox.tsxapps/web/src/components/ui/collapsible.tsxapps/web/src/components/ui/dialog.tsxapps/web/src/components/ui/dropdown-menu.tsxapps/web/src/components/ui/input.tsxapps/web/src/components/ui/progress.tsxapps/web/src/components/ui/separator.tsxapps/web/src/components/ui/sheet.tsxapps/web/src/components/ui/sidebar.tsxapps/web/src/components/ui/skeleton.tsxapps/web/src/components/ui/sonner.tsxapps/web/src/components/ui/tooltip.tsxapps/web/src/hooks/use-ballerina.tsapps/web/src/hooks/use-copy-share-link.tsapps/web/src/hooks/use-mobile.tsapps/web/src/hooks/use-share-notice.tsapps/web/src/hooks/use-share.tsapps/web/src/lib/file-tree-utils.tsapps/web/src/lib/fs/core/abstract-fs.tsapps/web/src/lib/fs/core/file-node-utils.tsapps/web/src/lib/fs/core/file-node.types.tsapps/web/src/lib/fs/core/fs.interface.tsapps/web/src/lib/fs/core/path-utils.tsapps/web/src/lib/fs/ephemeral-fs.tsapps/web/src/lib/fs/fs-roots.tsapps/web/src/lib/fs/layered-fs.tsapps/web/src/lib/fs/local-storage-fs.tsapps/web/src/lib/router-utils.tsapps/web/src/lib/share.tsapps/web/src/lib/utils.tsapps/web/src/main.tsxapps/web/src/providers/fs-provider.tsxapps/web/src/routeTree.gen.tsapps/web/src/routes/$.tsxapps/web/src/routes/__root.tsxapps/web/src/stores/editor-store.tsapps/web/src/stores/file-tree-store.tsapps/web/src/styles.cssapps/web/src/types/wasm-types.d.tsapps/web/src/wasm_exec.jsapps/web/tsconfig.app.jsonapps/web/tsconfig.jsonapps/web/tsconfig.node.jsonapps/web/turbo.jsonapps/web/vite.config.tsbiome.jsonpackage.jsonpackages/wasm/.gitignorepackages/wasm/ballerina-lang-gopackages/wasm/biome.jsonpackages/wasm/diagnostics_printer.gopackages/wasm/go.modpackages/wasm/local_storage_fs.gopackages/wasm/main_wasm.gopackages/wasm/package.jsonpackages/wasm/turbo.jsonscripts/example_gen/main.goturbo.jsonweb/src/assets/examples.jsonweb/src/components/file-route-sync.tsxweb/src/lib/fs/core/path-utils.tsweb/src/routes/__root.tsx
💤 Files with no reviewable changes (5)
- biome.json
- web/src/routes/__root.tsx
- web/src/assets/examples.json
- web/src/components/file-route-sync.tsx
- web/src/lib/fs/core/path-utils.ts
snelusha
force-pushed
the
fix/package-diagnostics
branch
2 times, most recently
from
e46907a to
c771f5f
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
apps/web/src/components/share-notice-dialog.tsx (1)
53-55: Avoid remountingDialogvia dynamickey.
key={String(open)}remounts the whole dialog on every toggle. Prefer resettingpermanentexplicitly when opening, while keeping the dialog instance stable.♻️ Proposed refactor
export function ShareNoticeDialog({ open, onDismiss, onDismissPermanently, }: ShareNoticeDialogProps) { const [permanent, setPermanent] = React.useState(false); + React.useEffect(() => { + if (open) setPermanent(false); + }, [open]); const handleDismiss = () => permanent ? onDismissPermanently() : onDismiss(); return ( <Dialog - key={String(open)} open={open} onOpenChange={(next) => !next && handleDismiss()} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/share-notice-dialog.tsx` around lines 53 - 55, Remove the dynamic key on the Dialog (delete key={String(open)}) so the Dialog instance isn't remounted on every toggle; instead, keep the Dialog stable and explicitly reset your "permanent" state when the dialog is opened inside the existing onOpenChange handler (use the setter for your permanent state—e.g., setPermanent(false) or setIsPermanent(false) when next is true), and keep the existing onOpenChange={(next) => !next && handleDismiss()} logic for closing; this preserves component instance stability while still resetting the permanent flag when opening.apps/web/src/lib/fs/core/file-node-utils.ts (1)
13-38: Consider usingjoin()for path construction consistency.Line 34 uses template literal
${path}/${entry.name}for path construction, while other parts of the codebase (including this file at lines 47, 54-55) use thejoin()utility. Usingjoin()consistently ensures proper handling of edge cases like trailing slashes.♻️ Suggested change
const children = (fs.readDir(path) ?? []) - .map((entry) => toFileNode(fs, `${path}/${entry.name}`)) + .map((entry) => toFileNode(fs, join(path, entry.name))) .filter((n): n is FileNode => n !== null);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/lib/fs/core/file-node-utils.ts` around lines 13 - 38, The path concatenation in toFileNode uses a template literal `${path}/${entry.name}` which can mishandle trailing slashes; replace that construction with the project's path join utility (e.g., join(path, entry.name)) when calling toFileNode recursively, and add or reuse the join import if it's not already imported so toFileNode consistently uses join for all path assembly.apps/web/src/lib/share.ts (1)
81-106: Consider adding a maximum payload size check.The decoder accepts arbitrarily large payloads. While
toFileNode(used during encoding) has no size limits either, extremely large shared trees could:
- Exceed URL length limits (~2000 chars for some browsers)
- Cause memory pressure during decode
This is a minor concern since the playground likely has practical limits on file tree size, but a sanity check could prevent abuse.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/lib/share.ts` around lines 81 - 106, The decoder decodeSharePayload currently accepts arbitrarily large decoded JSON from decodeAndDecompress; add a sanity check after decoding (inside decodeSharePayload, right after const json = await decodeAndDecompress(encoded)) to enforce a maximum decoded payload size (e.g. define a MAX_DECODED_PAYLOAD_BYTES constant) and return null if json.length exceeds it (or trim/reject before JSON.parse); this protects against extremely large payloads and memory/URL limits while preserving existing parsing logic in the isFileNode / root handling paths.apps/web/src/lib/fs/core/path-utils.ts (1)
24-33: Thejoin()function doesn't normalize.or..segments.The
join()function only filters out empty segments but doesn't handle.(current directory) or..(parent directory) segments. While callers liketoFileNodeandisSafeRelativePathguard against these, any direct use ofjoin()with user input containing these segments could produce unexpected paths.This is acceptable if all callers validate inputs beforehand, which appears to be the case in this PR.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/lib/fs/core/path-utils.ts` around lines 24 - 33, The join(...) implementation should normalize "." and ".." segments so callers can't accidentally get unexpected paths; update the join function to split segments, iterate with a stack: ignore "." entries, pop the stack for ".." (but if stack is empty and the path is absolute keep it at root, or for relative allow leading ".." entries), push normal segment names, and then rebuild the path preserving the leading "/" if segments[0] started with "/" — adjust join to perform this normalization so usages like toFileNode and isSafeRelativePath (and any direct callers) receive normalized paths.apps/web/src/components/ui/sonner.tsx (1)
11-11: ImportCSSPropertiesexplicitly for clarity and maintainability.While
React.CSSPropertiesworks due to implicit type availability from Vite's React plugin, explicit imports make dependencies clear and improve maintainability. Additionally,"--border-radius": "none"is not a valid CSS value;border-radiusexpects length/percentage values or the keywordnone, not the string"none". Use"0px"instead.Suggested change
import { Toaster as Sonner } from "sonner"; +import type { CSSProperties } from "react"; import { HugeiconsIcon } from "@hugeicons/react"; @@ - style={ - { - "--normal-bg": "var(--popover)", - "--normal-text": "var(--popover-foreground)", - "--normal-border": "var(--border)", - "--border-radius": "none", - } as React.CSSProperties - } + style={ + { + "--normal-bg": "var(--popover)", + "--normal-text": "var(--popover-foreground)", + "--normal-border": "var(--border)", + "--border-radius": "0px", + } as CSSProperties + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/ui/sonner.tsx` at line 11, Import CSSProperties explicitly from React and use it for any inline style typings instead of relying on implicit React.CSSProperties (update the import list alongside ToasterProps import), and change the invalid CSS value "--border-radius": "none" to a valid value such as "0px" wherever the style object is defined (e.g. in the Sonner/Toaster style object used by the component). Ensure the type annotations reference CSSProperties and the style object uses "0px" for the custom property so linting/type checks pass.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/components/ui/sonner.tsx`:
- Line 60: Remove the invalid CSS custom property "--border-radius": "none" from
the Sonner config in apps/web/src/components/ui/sonner.tsx; instead, control
toast border radius via Sonner's classNames (e.g., classNames: { toast:
'rounded-none' }), toastOptions.style (e.g., borderRadius: '0'), or by using the
unstyled prop—update the Toaster/sonner configuration (the object where
"--border-radius" is set) to delete that line and apply one of the supported
approaches (classNames or toastOptions.style) to achieve no border radius.
In `@apps/web/src/stores/file-tree-store.ts`:
- Around line 299-309: wrap the call to _fs().graftSharedTree inside
loadSharedFiles in a try/catch so exceptions from graftSharedTree/insertSubtree
are handled; move the set(...) that updates tempTree/localTree into the try
block so state is only mutated on success, and in the catch log the error (or
use the project logger) and return { loaded: false, openPath: null } so callers
(e.g. the use-share.ts .then() consumer) get a safe failure result instead of an
uncaught exception.
---
Nitpick comments:
In `@apps/web/src/components/share-notice-dialog.tsx`:
- Around line 53-55: Remove the dynamic key on the Dialog (delete
key={String(open)}) so the Dialog instance isn't remounted on every toggle;
instead, keep the Dialog stable and explicitly reset your "permanent" state when
the dialog is opened inside the existing onOpenChange handler (use the setter
for your permanent state—e.g., setPermanent(false) or setIsPermanent(false) when
next is true), and keep the existing onOpenChange={(next) => !next &&
handleDismiss()} logic for closing; this preserves component instance stability
while still resetting the permanent flag when opening.
In `@apps/web/src/components/ui/sonner.tsx`:
- Line 11: Import CSSProperties explicitly from React and use it for any inline
style typings instead of relying on implicit React.CSSProperties (update the
import list alongside ToasterProps import), and change the invalid CSS value
"--border-radius": "none" to a valid value such as "0px" wherever the style
object is defined (e.g. in the Sonner/Toaster style object used by the
component). Ensure the type annotations reference CSSProperties and the style
object uses "0px" for the custom property so linting/type checks pass.
In `@apps/web/src/lib/fs/core/file-node-utils.ts`:
- Around line 13-38: The path concatenation in toFileNode uses a template
literal `${path}/${entry.name}` which can mishandle trailing slashes; replace
that construction with the project's path join utility (e.g., join(path,
entry.name)) when calling toFileNode recursively, and add or reuse the join
import if it's not already imported so toFileNode consistently uses join for all
path assembly.
In `@apps/web/src/lib/fs/core/path-utils.ts`:
- Around line 24-33: The join(...) implementation should normalize "." and ".."
segments so callers can't accidentally get unexpected paths; update the join
function to split segments, iterate with a stack: ignore "." entries, pop the
stack for ".." (but if stack is empty and the path is absolute keep it at root,
or for relative allow leading ".." entries), push normal segment names, and then
rebuild the path preserving the leading "/" if segments[0] started with "/" —
adjust join to perform this normalization so usages like toFileNode and
isSafeRelativePath (and any direct callers) receive normalized paths.
In `@apps/web/src/lib/share.ts`:
- Around line 81-106: The decoder decodeSharePayload currently accepts
arbitrarily large decoded JSON from decodeAndDecompress; add a sanity check
after decoding (inside decodeSharePayload, right after const json = await
decodeAndDecompress(encoded)) to enforce a maximum decoded payload size (e.g.
define a MAX_DECODED_PAYLOAD_BYTES constant) and return null if json.length
exceeds it (or trim/reject before JSON.parse); this protects against extremely
large payloads and memory/URL limits while preserving existing parsing logic in
the isFileNode / root handling paths.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6628d4e1-4b20-465d-8656-76c4e166e1bb
⛔ Files ignored due to path filters (3)
apps/web/public/ballerina.svgis excluded by!**/*.svgbun.lockis excluded by!**/*.lockpackages/wasm/go.sumis excluded by!**/*.sum
📒 Files selected for processing (75)
.github/workflows/ci.yml.github/workflows/deploy.yml.gitignore.gitmodulesapps/web/.gitignoreapps/web/biome.jsonapps/web/components.jsonapps/web/index.htmlapps/web/package.jsonapps/web/src/assets/examples.jsonapps/web/src/components/ansi.tsxapps/web/src/components/app-sidebar.tsxapps/web/src/components/code-editor.tsxapps/web/src/components/editor.tsxapps/web/src/components/file-route-sync.tsxapps/web/src/components/file-tree-dialog.tsxapps/web/src/components/share-notice-dialog.tsxapps/web/src/components/ui/button.tsxapps/web/src/components/ui/checkbox.tsxapps/web/src/components/ui/collapsible.tsxapps/web/src/components/ui/dialog.tsxapps/web/src/components/ui/dropdown-menu.tsxapps/web/src/components/ui/input.tsxapps/web/src/components/ui/progress.tsxapps/web/src/components/ui/separator.tsxapps/web/src/components/ui/sheet.tsxapps/web/src/components/ui/sidebar.tsxapps/web/src/components/ui/skeleton.tsxapps/web/src/components/ui/sonner.tsxapps/web/src/components/ui/tooltip.tsxapps/web/src/hooks/use-ballerina.tsapps/web/src/hooks/use-copy-share-link.tsapps/web/src/hooks/use-mobile.tsapps/web/src/hooks/use-share-notice.tsapps/web/src/hooks/use-share.tsapps/web/src/lib/file-tree-utils.tsapps/web/src/lib/fs/core/abstract-fs.tsapps/web/src/lib/fs/core/file-node-utils.tsapps/web/src/lib/fs/core/file-node.types.tsapps/web/src/lib/fs/core/fs.interface.tsapps/web/src/lib/fs/core/path-utils.tsapps/web/src/lib/fs/ephemeral-fs.tsapps/web/src/lib/fs/fs-roots.tsapps/web/src/lib/fs/layered-fs.tsapps/web/src/lib/fs/local-storage-fs.tsapps/web/src/lib/router-utils.tsapps/web/src/lib/share.tsapps/web/src/lib/utils.tsapps/web/src/main.tsxapps/web/src/providers/fs-provider.tsxapps/web/src/routeTree.gen.tsapps/web/src/routes/$.tsxapps/web/src/routes/__root.tsxapps/web/src/stores/editor-store.tsapps/web/src/stores/file-tree-store.tsapps/web/src/styles.cssapps/web/src/types/wasm-types.d.tsapps/web/src/wasm_exec.jsapps/web/tsconfig.app.jsonapps/web/tsconfig.jsonapps/web/tsconfig.node.jsonapps/web/turbo.jsonapps/web/vite.config.tsbiome.jsonpackage.jsonpackages/wasm/.gitignorepackages/wasm/ballerina-lang-gopackages/wasm/biome.jsonpackages/wasm/diagnostics_printer.gopackages/wasm/go.modpackages/wasm/local_storage_fs.gopackages/wasm/main_wasm.gopackages/wasm/package.jsonpackages/wasm/turbo.jsonturbo.json
💤 Files with no reviewable changes (1)
- biome.json
✅ Files skipped from review due to trivial changes (13)
- apps/web/src/main.tsx
- packages/wasm/.gitignore
- packages/wasm/biome.json
- packages/wasm/package.json
- packages/wasm/turbo.json
- apps/web/biome.json
- apps/web/src/styles.css
- .gitmodules
- .github/workflows/deploy.yml
- apps/web/src/lib/fs/fs-roots.ts
- apps/web/turbo.json
- package.json
- turbo.json
🚧 Files skipped from review as they are similar to previous changes (16)
- .gitignore
- apps/web/src/lib/fs/ephemeral-fs.ts
- apps/web/src/routes/__root.tsx
- packages/wasm/diagnostics_printer.go
- apps/web/src/routes/$.tsx
- .github/workflows/ci.yml
- apps/web/src/hooks/use-copy-share-link.ts
- apps/web/src/hooks/use-share-notice.ts
- apps/web/package.json
- apps/web/src/components/ui/checkbox.tsx
- apps/web/src/components/file-route-sync.tsx
- apps/web/src/hooks/use-share.ts
- apps/web/src/lib/file-tree-utils.ts
- apps/web/src/assets/examples.json
- apps/web/src/lib/fs/layered-fs.ts
- apps/web/src/components/app-sidebar.tsx
snelusha
force-pushed
the
fix/package-diagnostics
branch
from
9e6f05b to
a180052
Compare
snelusha
force-pushed
the
fix/package-diagnostics
branch
from
a180052 to
b24dd0e
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/src/stores/file-tree-store.ts (1)
310-311: Consider logging the caught error for debuggability.On Line 310–Line 311, the exception is swallowed. Adding a structured log here would help diagnose malformed shared trees or grafting failures in production.
Suggested tweak
- } catch (e) { + } catch (e) { + console.error("[FileTreeStore] loadSharedFiles failed", e); return { loaded: false, openPath: null }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/stores/file-tree-store.ts` around lines 310 - 311, Add a structured log inside the catch that currently returns { loaded: false, openPath: null } so the swallowed exception is recorded; e.g., inside that catch block call your logger (console.error or the app logger) with a clear message like "file-tree-store: failed to load shared tree" and include the caught error object (e) and any relevant context (such as the path or id being processed) before returning the same { loaded: false, openPath: null } so failures in parsing/grafting are discoverable in logs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/web/src/stores/file-tree-store.ts`:
- Around line 310-311: Add a structured log inside the catch that currently
returns { loaded: false, openPath: null } so the swallowed exception is
recorded; e.g., inside that catch block call your logger (console.error or the
app logger) with a clear message like "file-tree-store: failed to load shared
tree" and include the caught error object (e) and any relevant context (such as
the path or id being processed) before returning the same { loaded: false,
openPath: null } so failures in parsing/grafting are discoverable in logs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a2851f62-f766-40a3-91bd-3046aca57f92
📒 Files selected for processing (2)
apps/web/src/stores/file-tree-store.tspackages/wasm/diagnostics_printer.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
$subject
Changes
This pull request addresses error handling and path resolution issues in diagnostic snippet processing across two key areas:
Web Store (File Tree)
loadSharedFilesfunction by wrapping shared tree grafting operations in try/catch logicloaded: falsestate instead of throwing exceptions, improving application stabilityDiagnostics Printer (Go)
snippetSourcePathutility function to properly resolve the actual file paths for reading diagnostic snippetsTogether, these changes improve the reliability of diagnostic snippet retrieval by implementing proper error handling and fixing the source path resolution logic.