Conversation
…ap word processor (#1565) * rename(office-studio): rename Office Suite to Office Studio Renames the app id, display name, component, view directory, and store catalog entry from Office Suite to Office Studio across the frontend and backend allowlist/version/trust maps. Drops the unimplemented Data rail item from the app's left rail. The /api/office/* routes and the office doc kind values (write/calc/db/slides) are unchanged, they are storage kinds, not the app id. * feat(office-studio): real Tiptap word processor for the Write view Replaces the plain textarea in the Write view with a Tiptap editor (StarterKit + Underline + Link, all MIT). The existing toolbar buttons now drive real editor commands with active-state styling: bold, italic, underline, a paragraph/H1/H2/H3 heading control, bullet and ordered lists, and a link insert/edit prompt. The editor's HTML is persisted through the existing POST/PUT /api/office/docs calls (the content column is opaque TEXT, so no schema change), and loads back into the editor when a doc is opened. Adds a view header matching Images Studio's design bar, keeps the A4-style paper layout but swaps the hardcoded light-mode colors for the shell-* theme tokens so it now follows dark/light scheme, and adds aria-label/aria-pressed/focus-visible affordances across the toolbar. vitest.setup.ts gains a Range.getClientRects/getBoundingClientRect polyfill (JSDOM does not implement either), since ProseMirror calls them to scroll the selection into view after every transaction. * fix(office-studio): fold review findings (XSS link guard, rename migration, toolbar re-render) - CRITICAL: block dangerous link schemes. The Write view now validates link URLs against an http/https/mailto allowlist both when a link is created (setLink) and when content is parsed (Link.configure validate), so a stored document can no longer round-trip a javascript:/data: URL back through /api/office/docs and execute it. - The Office Suite -> Office Studio app-id rename now migrates pre-existing install/runtime rows on boot (installed_apps + app_runtime), so a user who installed office-suite keeps the app installed as office-studio. Adds a test. - Replace the manual forceToolbarUpdate counter with useSyncExternalStore subscribing to editor transactions. * fix(office-studio): rename migration must not clobber a newer target row UPDATE OR REPLACE would delete an existing office-studio install/runtime row and replace it with the older office-suite data if both coexist. Use UPDATE OR IGNORE then DELETE so the newer target row is preserved and only the stale old-id row is dropped. Adds a test for the both-rows case.
…xt, shapes, layers, zoom, undo, export) (#1566) * chore(desktop): add konva + react-konva for the Design Studio canvas Phase 1 of turning Design Studio into a real editor needs an actual 2D scene graph with selection/transform support. Konva + react-konva (both MIT) give us that without hand-rolling hit testing and drag/resize. * feat(design-studio): model the canvas as a real element scene graph Replace the single-purpose CanvasImageElement with a proper union of text/image/rect/ellipse/line elements (position, size, rotation, z-index, visibility, plus type-specific styling). Add the supporting pieces the editor needs: factory functions for placing new elements centered on the artboard, an undo/redo history stack over the elements array, and a small hook that loads an HTMLImageElement for Konva's <Image> node. * feat(design-studio): add the Konva node renderer, layers list, and properties panel CanvasNode renders one element as the matching Konva primitive (Text, Image, Rect, Ellipse, Line) wired for select/drag/resize+rotate, with a soft selection glow. LayersPanel lists elements by z-order with visibility toggle, reorder, and delete. PropertiesPanel reflects and edits the selected element's type-specific fields (font/color/align for text, fill/stroke for shapes) plus a read-only position/size summary. * feat(design-studio): wire the Design view to a real Konva canvas editor Replace the static mock artboard with a working Stage/Layer setup: click to select with a Transformer for move/resize/rotate, Delete to remove, Cmd/Ctrl+D to duplicate, double-click text to edit inline, a wired left toolbar (text/shape/circle/line/image-upload), zoom (buttons, dropdown, wheel, fit-to-screen) and space/middle-drag pan, and a PNG export that downloads the artboard at native resolution via the content layer's toDataURL regardless of the current zoom. The empty state now says "Add an element or generate with Magic" and Undo/Export disable themselves when there is nothing to do. * feat(design-studio): make Magic images and Templates land on the real canvas placeOnCanvas now builds a proper ImageElementData via the element factory instead of a static absolute div, so images generated in Magic become manipulable Konva nodes. Template cards are wired: picking one sets the artboard to that template's size, clears the canvas, and switches to Design. * test(design-studio): cover add/select/undo-redo/export/templates on the canvas react-konva renders to a real canvas 2D context, which jsdom does not implement, so mock it the same way the existing ExcalidrawBoard test does: lightweight stand-ins that still forward click handlers and expose just enough of the Konva node API for the app's own logic to run. Covers adding elements from the toolbar, selecting one and seeing its properties, undo/redo changing the element count, Cmd/Ctrl+D duplicate and Delete, PNG export firing a real download, and template selection resetting the canvas at the new artboard size. * docs(design-studio): note doc-gate trailer for new in-app files The diff gate flags any added/deleted path under desktop/src/apps/*/** as a possible new/removed app. This PR only adds component files inside the existing Design Studio app (CanvasNode.tsx, LayersPanel.tsx, PropertiesPanel.tsx, elementFactory.ts, useElementHistory.ts, useHtmlImage.ts) alongside DesignView.tsx, DesignStudioApp.tsx, and TemplatesView.tsx, which the gate already ignores as plain modifications. Docs-Reviewed: new files are components inside the existing Design Studio app, not a new or removed app; no user-facing doc surface changes * fix(design-studio): address code-review findings on the canvas editor - Don't hijack Delete/Backspace when a <select> (the zoom picker) or other form control is focused: isTypingTarget now also treats SELECT as typing. - Keep the inline text-edit overlay anchored when the user zooms/pans during an edit by adding zoom + stagePos to the editorStyle memo deps. - Cap the undo/redo history at 100 snapshots so a long session with image data URLs stays memory-bounded. - Clear a failed image load (onerror) so a broken src doesn't leave a stale bitmap on the Konva node. - Confirm before a template pick clears a non-empty canvas, so an accidental click doesn't wipe unsaved work; add tests for both confirm and cancel. The reported export-is-zoom-dependent concern was checked against the running app: exporting at 25%, 100%, and 200% (with a non-zero, panned stage origin) all produce a correct full-bleed 1080x1350 PNG. The stage transform is baked into the layer's toDataURL, and the zoom/pixelRatio compensation is correct, so no change was made there. Docs-Reviewed: review-fix touching existing Design Studio components; no app added or removed and no user-facing doc surface changes * fix(design-studio): export the artboard at a fixed crop, independent of pan/zoom exportPng cropped the content layer using the current stagePos/zoom (x/y/width/height derived from the on-screen transform), but the layer's canvas is only ever drawn at the stage's live pan/zoom. Once the artboard was scaled up past the visible viewport, or panned so part of it sat off-screen, that crop missed content and produced clipped/offset PNGs. Reset the stage to an untransformed 1:1 view for the instant of capture, grab the artboard at its true pixel size with a fixed pixelRatio, then restore whatever pan/zoom the user had. The result no longer depends on the view at all. Also extends the react-konva test mock to track a fake stage's scale/position so a new test can assert the crop and transform-at-capture are correct after zooming to 200% (which also pans, since the app re-centers around the viewport), and adds a keydown-guard regression test covering the zoom <select>. * fix(design-studio): surface failed image loads, fix numeric input parsing useHtmlImage cleared a failed image load on error but didn't expose that state, so a broken Magic/upload image just rendered blank with no signal to the user. It now returns a failed flag, and CanvasNode renders a dashed placeholder rect in its place instead of nothing. PropertiesPanel's font-size and stroke-width inputs used `Number(v) || fallback`, which silently discards a valid 0 (or an in-progress empty field) by falling back to the previous value. Parse with an explicit NaN check instead so the update only gets dropped on genuinely invalid input. * fix(design-studio): cap both undo/redo stacks; log failed image loads - The MAX_HISTORY cap was only applied to undoStack in commit; the pushes in undo (redoStack) and redo (undoStack) were unbounded. Route every push through a pushCapped helper so both stacks stay bounded. - useHtmlImage now logs a diagnostic on image load failure so a broken upload/Magic image is traceable, in addition to the failed flag the caller already uses to render a placeholder.
…tor + templates + preview + static export) (#1567) * feat(web-studio): backend site store + /api/web/sites CRUD routes Add WebSiteStore (mirrors OfficeDocStore) persisting sites(id, title, content, created_at, updated_at) where content is the opaque site-model JSON. Register a web router with POST/GET/GET{id}/PUT{id}/DELETE at /api/web/sites, wire the store into app.state alongside office_docs, and add web-studio to the apps allowlist, version map (1.0.0) and trust map (first-party). Includes store + route tests. * feat(web-studio): section-based website builder app (shell + views) Add the Web Studio app: a studio shell (Generate / Templates / Edit / Preview / Export rail) over a section-based site model. Sections: hero, features (3-up), textBlock, gallery, cta, contact, footer, each a styled block. Views: prompt-to-template Generate, a 6-template gallery, a visual Edit view (select, inline contentEditable text, image swap, add/delete/ reorder sections, live palette + font theming), a responsive Desktop/ Tablet/Mobile Preview, and a self-contained static-HTML Export with download. Saved-sites sidebar persists via /api/web/sites. * feat(web-studio): register app + Store catalog entry + tests Register web-studio in the app registry (optional studio app, layout- template icon) and flip its Store Studios catalog card from soon to available at 1.0.0. Add WebStudioApp tests (rail renders, generate seeds sections, add/remove/reorder mutate the model, inline edit commits, static HTML export + download) and update the StudiosView test now that every taOS studio has shipped. * docs(web-studio): document Web Studio in the README app list Adds the Web Studio (AI-assisted Wix-style site builder) to the studios section and the implemented-optional-apps list. Docs-Reviewed: the new /api/web/sites route module is an app-scoped CRUD store that mirrors the existing office_docs pattern exactly; it introduces no new cross-agent coordination surface, so docs/agent-coordination.md needs no change. * fix(web-studio): close TOCTOU on site id + bound request size and JSON parsing WebSiteStore.create() used to SELECT for an unused id then INSERT, leaving a race window where two concurrent creates could pick the same id. The PRIMARY KEY is now the actual source of truth: INSERT is attempted directly and a colliding id is retried with a fresh one. The /api/web/sites routes also raised an unhandled 500 on a malformed JSON body and had no cap on content size, letting a single request bloat the sites table with an oversized data URI. Both endpoints now return 400 on invalid JSON and 413 when content exceeds a 5 MB cap. * test(web-studio): cover id-collision retry, bad JSON and oversized content Docs-Reviewed: adds tests only for the existing /api/web/sites route module covered by 2be97bb; no new cross-agent coordination surface. * fix(web-studio): guard against data loss and corrupted/oversized content - Confirm before New/Generate/Templates discard unsaved in-editor edits. - Validate a saved site's shape before trusting it; a corrupted record now falls back to a blank site with a visible error instead of throwing during JSON.parse. - Reject oversized or non-image uploads in the section image picker with a visible message, and stop an unhandled promise rejection when reading a file fails. - Use crypto.randomUUID() for section ids instead of Math.random(). - Add exhaustive default branches to the section-type switches in the static HTML exporter and the canvas renderer so an unrecognized section type fails loudly (export) or renders visibly (canvas) rather than silently disappearing. * test(web-studio): cover discard-confirm, corrupted-site fallback and image validation * fix(doc-gate): reword README so it stops false-positiving as a bad path The doc-gate's Layer-A invariants scan pulls scripts/tinyagentos/docs/desktop path-like tokens out of README.md and asserts each exists on disk. The Web Studio blurb's "desktop/tablet/mobile" read as a desktop/ path token to that scanner (it has no notion of prose vs. path), so it failed with 'references desktop/tablet/mobile which does not exist' even though the branch already carries a real README edit and a Docs-Reviewed trailer for the routes rule. Rewording to 'desktop, tablet, and mobile' removes the false path token without touching the gate script. * fix(web-studio): fold second-round review findings - openSite now honors the confirm-discard guard so switching sites cannot silently drop unsaved edits - saveSite rejects an over-cap site (mirrors the backend MAX_CONTENT_BYTES) with a clear message instead of failing only at PUT with a raw 413 - isValidSite validates each section shape (id + type), so a corrupted sections array no longer passes as a wall of unsupported-section placeholders - web_sites: log id collisions before retrying so an entropy regression is diagnosable rather than surfacing only as a RuntimeError - routes/web _parse_json also handles empty bodies / wrong Content-Type and rejects non-object JSON as 400 instead of letting it 500
…i-sheet, CSV, persistence) (#1568) * feat(office-studio): add fortune-sheet as the Calc spreadsheet engine @fortune-sheet/react (MIT) is a DOM-rendered, React-native spreadsheet component with a built-in formula parser (SUM, AVERAGE, IF, cell refs, etc.), multi-sheet support, and an imperative ref API. It replaces the static mock grid in the Calc view. * feat(office-studio): add spreadsheet CSV, address and workbook helpers Pure, unit-tested helpers for the Calc view: A1-style cell addressing, CSV export/import (RFC 4180 quoting), and workbook JSON serialization for save/load round trips. Also adds regression tests against the fortune-sheet formula engine itself (SUM, dependent recompute, AVERAGE/MIN/MAX/COUNT/IF, arithmetic). * feat(office-studio): real spreadsheet Calc view Replace the decorative Calc mock with a working spreadsheet on top of fortune-sheet's Workbook grid: - Editable cells, keyboard navigation, range selection via the real grid component. - A custom formula bar that reflects and edits the active cell's formula/value and recomputes dependents on commit. - Multiple sheets: add, rename (double-click a tab), delete, and switch, with a custom tab bar (fortune-sheet's own toolbar/formula bar/sheet tabs are hidden so the chrome matches shell design tokens). - Sort ascending/descending and a text filter on the selected column. - CSV export of the active sheet and CSV import into it. - Persistence: the whole workbook (all sheets, cell data and formulas) serializes to JSON and saves via the existing /api/office/docs store under kind "calc", reusing the doc sidebar list/new/open/save pattern from the Write view. No backend changes needed since `content` is already opaque TEXT. Also fixes a real bug found while testing: passing a freshly constructed `hooks` object to Workbook on every render put it into a render/settings-change feedback loop (confirmed by a runaway CPU spin in a full mount). Hooks are now memoized so their identity is stable. Updates the Calc smoke test in OfficeStudioApp.test.tsx, which asserted on the old mock's static Total row, to check the new formula bar, sheet tab, and save/new affordances instead. * fix(office-studio): emit trailing CRLF from CSV export, reject unterminated quotes on import sheetToCsv was missing the trailing CRLF that Excel/Sheets expect after the last row. parseCsv silently swallowed the rest of the file into a single field when a quoted field was never closed; it now throws a descriptive error instead, which CalcView surfaces to the user. * fix(office-studio): validate sheet shape when parsing saved workbook content parseWorkbookContent only checked that sheets was a non-empty array. Corrupted saved content (a null entry, a non-Sheet object, or celldata/row/ column of the wrong type) would pass through untouched and crash the Workbook component downstream. Each sheet is now validated before use, and any invalid shape falls back to a blank workbook. * fix(office-studio): make the column sort comparator handle mixed types deterministically The inline comparator in CalcView coerced any string through Number(), including blank cells (Number("") is 0), which quietly sorted empty cells among numeric values instead of alongside text. Extracted into a testable compareCellValues helper: numeric compare only when both sides parse as finite numbers, locale string compare otherwise. * fix(office-studio): fold CalcView review findings - importCsv wraps its body in try/catch and surfaces failures via setError, instead of letting a rejected promise vanish behind the fire-and-forget call in handleFileChange. - renameSheetTab reads the tab list back from the workbook after renaming, instead of relying solely on the afterUpdateSheetName hook to keep the sidebar in sync. - exportCsv reads both the sheet name and its cell data from the same wb.getSheet() snapshot, so they can no longer disagree. - refreshFormulaBar is no longer called from inside a setSelection updater; afterUpdateCell now reads the latest selection off a ref instead. - clearFilter recomputes the currently-hidden rows from the workbook's config.rowhidden instead of trusting potentially stale hiddenRows state. - afterAddSheet only activates the new sheet if the workbook doesn't already report it active, avoiding a redundant double-activation. - afterActivateSheet defers the formula bar refresh to the next tick and confirms the workbook reports the new sheet as active first, instead of reading cells before the sheet switch is visible. - The Sort buttons are now disabled (with a tooltip) when the active sheet has fewer than 3 rows, instead of silently no-opping. - The hidden CSV file input uses sr-only instead of display:none, so its aria-label is actually reachable by assistive tech. - Documented why saveDoc falls back to workbookData only when the workbook ref genuinely isn't mounted yet. * fix(office-studio): access the workbook ref through a stable accessor in formula engine tests Tests dereferenced wbRef.current directly at each call site. Added a small currentWb(ref) helper that throws if the ref isn't attached yet, so the "always read fresh, never cache" intent documented at the top of the file is enforced by a single helper rather than repeated non-null assertions. * chore(office-studio): note doc-gate review for the Calc modules The new files live under the existing desktop/src/apps/officestudio app; no desktop app was added or removed. The doc-gate's app glob also matches new files inside an existing app folder (a known imprecision), so this records the review. Docs-Reviewed: new modules under the existing officestudio app (Calc spreadsheet engine, CSV, sort helpers); no app added or removed, so README needs no change. * fix(office-studio): guard getSheet() access and clear the activation timer - optional-chain getSheet() in afterSelectionChange and the deferred afterActivateSheet callback so a transient undefined sheet cannot throw - track the afterActivateSheet setTimeout and clear it on the next activation and on unmount, so it can never setFormulaValue/setRowCount after the component is gone
…, present mode, PDF export, persistence) (#1569) * chore(desktop): add jspdf for Slides PDF export jsPDF (MIT) composes rasterized slide pages into a single downloadable PDF for the new Slides presentation editor. modern-screenshot, already a dependency, handles the rasterization side. * feat(office-studio): add the Slides deck data model Deck/Slide types, five layouts (title, title+content, two-column, section-header, blank), and pure mutation helpers (add/remove/reorder/ update a slide) plus JSON serialize/parse with validation so a corrupted saved deck falls back to a fresh one instead of crashing the view. Docs-Reviewed: new files live under the existing officestudio app (desktop/src/apps/officestudio/slides/), not a new app, so README's app list is unaffected -- known glob imprecision in the apps doc-gate rule. * feat(office-studio): add slide rendering, present mode, and PDF export SlideCanvas renders a slide at a fixed 640x360 design size and scales it to fit its container, so the thumbnail rail, editor preview, present mode, and PDF export capture all show identical proportions from one component. PresentMode is the fullscreen presentation surface, matching Game Studio's PlayView pattern: an always-visible Exit control plus Escape, so the user is never trapped. pdfExport rasterizes each slide (via modern-screenshot, already a dependency) and composes the pages into one PDF (via jsPDF). Both are MIT licensed. * feat(office-studio): wire Slides into a real presentation editor Replaces the static 5-slide mock with a working editor: a thumbnail rail (add/delete/move slide), a live preview driven by the selected slide's real state, a layout picker that switches live, title/body/ bullets/notes fields, image insert with MIME + size validation, a Present button (fullscreen with keyboard nav), an Export PDF button, and save/load through /api/office/docs with kind "slides" -- mirroring the doc sidebar pattern already used by Write and Calc.
…frontend mock) (#1548) (#1571) * fix(models): wire ModelsApp download to the real backend The Models app download flow was a pure frontend mock: DownloadProgress animated a fake percentage with setInterval + Math.random and called onDone without ever hitting the network, handleDownload just added the model id to a Set, and handleDownloadDone fabricated a downloaded entry that vanished the next time /api/models was refetched. Wire it up to the real endpoints instead: - carry a default variantId (smallest variant by size_mb, falling back to the first) into AvailableModel from the /api/models catalog - handleDownload now POSTs /api/models/download with {app_id, variant_id} and records the returned download_id - DownloadProgress now polls GET /api/models/downloads/{id} every second and drives the bar from the real percent instead of a timer - on status "complete" the model list is refetched from /api/models instead of fabricating a downloaded entry, so the Downloaded Models list reflects backend truth and survives reopening the app - on status "error" (or a failed POST) the backend error is shown on the download card with a Retry button - a model with no variant (e.g. the offline MOCK_AVAILABLE fallback) surfaces a clear "no downloadable variant" error instead of faking success - isDownloaded now also honours the backend's own has_downloaded_variant verdict, since the union of controller/worker/cloud downloads is keyed by filename/host rather than by catalog model id * test(models): cover the real ModelsApp download flow Adds ModelsApp.download.test.tsx pinning #1548: mocks fetch and asserts Download issues POST /api/models/download with the correct {app_id, variant_id}, the progress bar reflects polled percent, a polled status "error" surfaces the error with a Retry button, and a polled status "complete" triggers a real /api/models refetch instead of a fabricated entry. Also covers the no-variant fallback case. * fix(models): harden the download poll (transient tolerance, abort, no races, error card) Fold review findings on the download wiring: - a single failed poll no longer flips a still-running backend download to error and prompts a duplicate re-download; only a real backend status of error, or 3 consecutive poll misses, ends the download - the poll fetch now uses an AbortController and a cancelled flag so an in-flight poll after unmount cannot call setState / refetch on a gone view - replaced setInterval with a self-scheduling awaited poll so a slow response can never overlap or clobber a fresher one - the Available Models card now shows an actionable Retry (not a stuck disabled Downloading...) when a download has errored Docs-Reviewed: frontend-only change plus a new test file inside the existing Models app; no desktop app or API route added or removed, so README needs no change.
* chore(release): 1.0.0-beta.19 Ships the Creative Studios buildout: - Office Studio (rename): real Write (rich text), Calc (formula spreadsheet), Slides (presentations) - Design Studio: real Konva canvas editor - Web Studio: new AI-assisted website builder * docs(changelog): note the model-download fix under beta.19 (#1548)
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
👋 Thanks for the PR! This one targets See CONTRIBUTING.md for the branch model. |
|
Warning Review limit reached
Next review available in: 10 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (68)
✨ 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 |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| (title, content, now, site_id), | ||
| ) | ||
| await self._db.commit() | ||
| return await self.get(site_id) |
There was a problem hiding this comment.
WARNING: TOCTOU + silent failure in update(). If the row is deleted between the UPDATE and the subsequent self.get(site_id), this returns None. The PUT route at tinyagentos/routes/web.py:109 then serializes that as JSON null with HTTP 200 instead of 404. Check cursor.rowcount after the UPDATE (or propagate None to the route) and have the route return 404 when the store returns None.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| exists = await cur.fetchone() is not None | ||
| if not exists: | ||
| return False | ||
| await self._db.execute("DELETE FROM sites WHERE id = ?", (site_id,)) |
There was a problem hiding this comment.
WARNING: delete() has a select-then-delete race that leaks the real outcome to the caller. Use cursor.rowcount from the DELETE directly so a concurrent deletion between the SELECT and DELETE surfaces as False instead of a misleading True. This matches the pattern already used in tinyagentos/installed_apps.py.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| return content_error | ||
|
|
||
| site = await store.update(site_id=site_id, title=title, content=content) | ||
| return site |
There was a problem hiding this comment.
WARNING: PUT handler returns store.update(...) directly. When update() returns None (see comment on tinyagentos/web_sites.py:109), FastAPI will respond 200 with body null instead of 404. Add an explicit if site is None: return JSONResponse({"error": "not found"}, status_code=404).
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| # newer row is preserved, and the stale old-id row is then | ||
| # dropped. Never REPLACE, which would clobber the newer row. | ||
| await self._db.execute( | ||
| f"UPDATE OR IGNORE {table} SET app_id = ? WHERE app_id = ?", |
There was a problem hiding this comment.
SUGGESTION: Don't interpolate table into SQL via f-string even from a hardcoded tuple — this pattern silently bypasses static SQL-injection analysis and makes future maintainers copy it. Use an explicit dispatch (separate installed_apps / app_runtime SQL statements) so the table name is a literal in the SQL string.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| return next; | ||
| }); | ||
| setDownloaded((prev) => [ | ||
| const handleDownload = useCallback(async (model: AvailableModel) => { |
There was a problem hiding this comment.
WARNING: handleDownload has no re-entry guard. A double-click (or a Retry arriving while the previous POST is still in flight) launches two POST /api/models/download requests, creating two backend jobs; the second response overwrites the first downloadId, orphaning the first server-side job forever. Early-return when downloading[model.id]?.status is already starting or downloading before issuing the POST.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| if (typeof s.title !== "string") return false; | ||
| if (typeof s.body !== "string") return false; | ||
| if (!Array.isArray(s.bullets) || !s.bullets.every((b) => typeof b === "string")) return false; | ||
| if (s.imageDataUri !== undefined && typeof s.imageDataUri !== "string") return false; |
There was a problem hiding this comment.
WARNING: isValidSlide type-checks imageDataUri as a string but doesn't cap its length. MAX_IMAGE_BYTES = 5 * 1024 * 1024 is enforced on upload in the editor but never re-validated when a saved deck loads from the backend, so a 50 MB data URL sneaks through and blows past the original cap on every reload. Cap imageDataUri.length (e.g. < 8 MB) in isValidSlide or strip oversized images when loading.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| const trimmed = url.trim(); | ||
| if (!trimmed) return false; | ||
| const scheme = trimmed.match(/^([a-z][a-z0-9+.-]*):/i); | ||
| if (!scheme) return true; |
There was a problem hiding this comment.
WARNING: isSafeHref returns true for any value without a scheme prefix, including protocol-relative URLs like //evil.com/path. Those resolve against the document's origin in a browser and can navigate the Electron webview to an attacker-controlled origin. Reject scheme-relative URLs explicitly.
Suggested change
| if (!scheme) return true; | |
| if (!scheme) return !trimmed.startsWith("//"); |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| if (!confirmDiscard()) return; | ||
| setError(null); | ||
| try { | ||
| const res = await fetch(`/api/web/sites/${encodeURIComponent(id)}`, { credentials: "include" }); |
There was a problem hiding this comment.
WARNING: openSite awaits fetch then calls setSite/setActiveId with no cancelled/mounted guard. If the user closes the app or starts another open/save while this one is in flight, the late response overwrites the newer in-memory state with the older fetched site. Pair with a monotonically-increasing sequence id (or an AbortController) so stale responses are discarded.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| } | ||
| const payload = { title: site.title.trim() || "Untitled site", content }; | ||
| const url = activeId ? `/api/web/sites/${encodeURIComponent(activeId)}` : "/api/web/sites"; | ||
| const res = await fetch(url, { |
There was a problem hiding this comment.
WARNING: saveSite reads activeId live to decide POST vs PUT. Sequence: Save (POST in flight) -> user clicks "New" (activeId = null) -> user clicks Save again (another POST). When the first POST resolves it sets activeId = id1, linking the freshly-edited emptySite() to the previous server record. Snapshot the target URL, method, and serialized payload at call entry instead of reading live state.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| const deleteSite = async (id: string) => { | ||
| setError(null); | ||
| try { | ||
| const res = await fetch(`/api/web/sites/${encodeURIComponent(id)}`, { |
There was a problem hiding this comment.
WARNING: deleteSite is missing the same cancelled/unmount guard as openSite. If the user navigates away mid-flight, setError/setActiveId/the implicit loadList() will run on an unmounted tree, and an in-flight DELETE may resolve after the user has already started a new site.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 19 Issues Found | Recommendation: Address before merge Overview
Reviewed 70 changed files (release Highlights:
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (70 files)
Fix these issues in Kilo Cloud Reviewed by minimax-m3 · Input: 112.3K · Output: 24K · Cached: 4.1M |
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
1 participant
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.
Promotes dev to master for 1.0.0-beta.19.