Conversation
…box-render pipeline (mock to functional) (#1642) Turns App Studio's Build/Publish views from a hardcoded mockup into a working pipeline, reusing the Game Studio / userspace-apps machinery end to end. 1. Real generation. generate-app.ts streams the taos-agent with a file-block system prompt (app-authoring-prompt.ts) and parses the response with Game Studio's existing parseFileBlocks() convention. An empty parse, or a response missing an index.html entry point, falls back to a minimal honest starter page with a visible notice instead of faking success. 2. Generic packaging. New POST /api/userspace-apps/package builds a .taosapp (manifest.yaml + files) from an in-memory file map, mirroring routes/games.py's package_game but for callers with no server-side project store of their own. 3. BuildView now runs the whole pipeline on a single Build click: generate, analyze via the existing static security gate (a critical finding blocks the rest), package, install tagged "ai-generated", then renders the REAL installed app through SandboxedAppWindow -- install-then-render, so the actual bundle/CSP/broker and the server-side install gate all apply, not a client-side preview. The hardcoded "Chore Quest" mock and cosmetic capability chips are gone. 4. PublishView shows the app actually built in the Build view (handed off via a small module-level build session, since AppStudioApp only ever mounts one view at a time) instead of a hardcoded demo source. "Publish to my Store" and "Share with family" both install the real package (there's no separate public-store or family-sharing backend yet, same posture as Game/Web Studio's Share flow); "Export package" downloads it. The old cosmetic permission toggles are removed since the pipeline always packages permissions: [] in v1 and toggles that controlled nothing would now be actively misleading next to a real install action. Deferred: permission inference from the generated app (manifest always requests no permissions in v1) and an editable project store for in-progress builds. Docs-Reviewed: new files are App Studio feature source (real generation, packaging client, build-session hand-off) for the existing app; the package route is a generic addition to the existing userspace_apps.py route file, not a new route module. No new app or route surface, so no README addition is warranted.
… before install (#169) (#1645) * feat(licensing): weight-license metadata + non-commercial accept gate before install (#169) App-catalog service manifests labeled model-backed services with only their CODE license (MIT), which quietly implied the model WEIGHTS were permissive too. For musicgen, musicgpt, and flux-fill the weights are non-commercial (Meta's CC-BY-NC 4.0 MusicGen weights; the FLUX.1-dev non-commercial license), so a user could install and use them commercially without ever seeing the restriction. Slice 1 (honesty): - Add weights_license and license_class fields to AppManifest (dataclass + from_file mapping) alongside the existing code license. - Correct the 3 manifests: keep license: MIT, add weights_license + license_class: non-commercial. Other services were audited and left as code-license-only (generic runtimes that run user-chosen weights, or custom community licenses that permit commercial use below a threshold, which is a different category from non-commercial). - Surface license/weights_license/license_class in the /api/store/catalog payload and render a "Non-commercial weights" badge on Store cards. - Add tests/test_services_manifests.py and extend tests/test_registry.py. Slice 2 (arms-length accept-gate): - LicenseAcceptancesStore keyed (user_id, app_id, license_id); wired onto app.state like the other stores. - install-v2 returns 412 needs_license_acceptance for a non-commercial-weights service the user has not accepted; a follow-up install with accepted: true records the acceptance then proceeds. Applies to admins too. - Frontend LicenseAcceptDialog: on 412, show the license dialog and re-POST with accepted: true on agree; block install until accepted. Also unblocks #115 (RK image-gen flagged install) via the same gate. Docs-Reviewed: new file is a StoreApp sub-component (LicenseAcceptDialog), not a new desktop app; no user-facing docs affected * fix(store): start install-progress poller on license-accept install; clearer empty weights-license label Folds two Kilo review findings on #1645: - handleLicenseAccept did not set busy=true, so the card's install-progress poller (keyed on busy, cleared when the 412 gate returned) never ran during the accepted install and the card sat idle behind the dialog. Flip busy back on for the second POST. - LicenseAcceptDialog now renders a clear fallback label when weights_license is empty instead of a blank "Weights license:" line.
…locking sync POST (#1646) * fix(video-studio): async generation job (enqueue + poll) instead of blocking sync POST POST /api/video/generate used to block for the full multi-minute generation with no job id, no progress, and no way to recover from a client timeout or disconnect. It now enqueues a job and returns {job_id, status: "queued"} (202) immediately, running the actual backend call in a background task tracked in app.state._background_tasks (same pattern agent_chat_router.py already uses for fire-and-forget work). A new GET /api/video/jobs/{job_id} reports queued/running/done/error, with the saved video's filename/path in result once done. Job state is persisted in a small VideoJobStore(BaseStore), mirroring ConversionManager/TrainingManager. The store is a lazily-created singleton on app.state, mirroring routes/jobs.py's _get_queue, so no app.py lifespan wiring was needed. If no backend is configured at all, generate still fails fast with the existing 503 rather than enqueueing a doomed job. Once a backend is configured, backend-side failures (connect error, timeout, bad response) now surface on the job instead of the original request. VideoStudioApp.tsx polls the job endpoint on an interval with a capped attempt count, showing the existing generating/elapsed-time state without blocking the UI thread on a long-lived fetch. * fix(video-studio): harden video job store + poll path against Kilo findings - Use the full uuid4 hex as the job id instead of an 8-char truncation. The id is a persistent PRIMARY KEY, so a shortened id risked a collision (IntegrityError leaking as a 500 on enqueue). Add a test that two rapidly created jobs get distinct 32-char ids. - get_job now maps columns via an aiosqlite.Row dict factory instead of zipping cursor.description positionally, so it is not coupled to column order. - Guard json.loads on result_json in the status poll: a corrupt blob now downgrades to an error status with a clear message instead of 500ing the poller. - Make the background job's error-recording exception-safe and guarantee a terminal state: error recording is best-effort (logs on failure) and the done-status write is wrapped so a failed status write cannot leave the job stuck in "running". Add a test that an unexpected backend exception marks the job "error".
…pt gate, Video async generation) (#1647)
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. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ 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 (40)
📝 WalkthroughWalkthroughThis PR adds non-commercial weight licensing metadata to app manifests, backed by a new SQLite ChangesNon-commercial weights licensing gate
App Studio build/publish pipeline
Video Studio async job flow
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant AppCard
participant InstallV2 as install-v2 route
participant LicenseStore as LicenseAcceptancesStore
AppCard->>InstallV2: POST install-v2 (accepted=false)
InstallV2->>LicenseStore: has_accepted(user, app, license)
LicenseStore-->>InstallV2: false
InstallV2-->>AppCard: 412 needs_license_acceptance
AppCard->>InstallV2: POST install-v2 (accepted=true)
InstallV2->>LicenseStore: record_acceptance(user, app, license)
InstallV2-->>AppCard: 200 install success
sequenceDiagram
participant BuildView
participant GenerateApp as generateApp
participant AnalyzeAppSource
participant PackageAPI as packageUserspaceApp
participant InstallAPI as installUserspaceApp
BuildView->>GenerateApp: generateApp(prompt)
GenerateApp-->>BuildView: files
BuildView->>AnalyzeAppSource: analyzeAppSource(files)
AnalyzeAppSource-->>BuildView: findings
BuildView->>PackageAPI: packageUserspaceApp(name, files)
PackageAPI-->>BuildView: package file
BuildView->>InstallAPI: installUserspaceApp(package)
InstallAPI-->>BuildView: appId
BuildView->>BuildView: setBuildSession(session)
sequenceDiagram
participant VideoStudioApp
participant VideoRoute as /api/video/generate
participant JobStore as VideoJobStore
participant JobsRoute as /api/video/jobs/{job_id}
VideoStudioApp->>VideoRoute: POST generate request
VideoRoute->>JobStore: create_job()
VideoRoute-->>VideoStudioApp: 202 {job_id, status: queued}
loop poll until done or error
VideoStudioApp->>JobsRoute: GET job status
JobsRoute->>JobStore: get_job(job_id)
JobsRoute-->>VideoStudioApp: status/progress/result
end
VideoStudioApp->>VideoStudioApp: render video or error
Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 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}
| </h2> | ||
| </div> | ||
| <div className="px-5 py-4 space-y-2.5"> | ||
| <p className="text-[13px] text-shell-text-secondary leading-relaxed">{text}</p> |
There was a problem hiding this comment.
WARNING: The accept-gate dialog never shows the actual license text. It renders only the one-sentence text summary and a short weightsLicense label (e.g. "CC-BY-NC 4.0"), with an "Agree & Install" button. Users cannot read the terms they are agreeing to — there is no link to the full CC-BY-NC 4.0 text and no scrollable body. Storing this as accepted_at makes the user's consent the system's only record of having accepted, so the gate is only as strong as the informed-consent UX. Consider either (a) fetching the full license text from a known URL (the Creative Commons canonical pages, for example) and rendering it in a scrollable region before the Agree button is enabled, or (b) at minimum embedding the canonical short-form text in the response and requiring scroll-to-bottom before enable. As-is, the acceptance is legally and product-wise weak.
| {"error": f"file '{fname}' exceeds the {_MAX_BUILD_FILE_BYTES}-byte limit"}, status_code=413 | ||
| ) | ||
|
|
||
| suffix = "".join(random.choices(string.ascii_lowercase + string.digits, k=4)) |
There was a problem hiding this comment.
SUGGESTION: package_app derives app_id from f"{_slugify(name)}-{suffix}" with a 4-char alphanumeric suffix (≈1.7M space). Combined with the deterministic slug, two different inputs that collapse to the same slug only need ~√1.7M ≈ 1300 generations for a 50% birthday-collision probability — and on collision the second request silently overwrites the first's package directory before analysis runs (or, worse, if the first user already installed and the second races through analysis before the install, the second's content can be served under the first's id). Consider a longer suffix (≥8 chars) or, better, include the install's monotonic counter / random uuid4 truncated, so collisions are vanishingly unlikely regardless of how popular a slug becomes.
| // Only arm the interval if the first poll didn't already resolve | ||
| // (finishGenerating clears pollAbortRef.current on any terminal state). | ||
| if (pollAbortRef.current) { | ||
| pollTimerRef.current = setInterval(poll, POLL_INTERVAL_MS); |
There was a problem hiding this comment.
SUGGESTION: setInterval(poll, POLL_INTERVAL_MS) fires poll on a fixed 2 s cadence, but poll is async and awaits a network round-trip. If a single poll takes longer than POLL_INTERVAL_MS (slow backend, transient DNS, GC pause), setInterval does not back off — it queues the next invocation and can fire overlapping in-flight fetches that race the abortController.signal. On a stall that eventually resolves, the late poll that observed a terminal state calls finishGenerating() and clears pollAbortRef.current to null, but already-queued subsequent intervals can still fire before the interval handle is cleared (the clear only happens via clearInterval inside stopPolling → finishGenerating, which is called after the slow await). Use a self-rescheduling setTimeout chain (poll().finally(() => { if (pollAbortRef.current) pollTimerRef.current = setTimeout(poll, POLL_INTERVAL_MS); })) so each poll arms the next only after the previous settles.
| async def get_job(self, job_id: str) -> dict | None: | ||
| # Map columns by name via a dict row factory rather than zipping | ||
| # cursor.description positionally -- decoupled from column order. | ||
| self._db.row_factory = aiosqlite.Row |
There was a problem hiding this comment.
SUGGESTION: get_job sets self._db.row_factory = aiosqlite.Row on every call. It's idempotent in isolation but surprising: (a) the other two methods in this class (create_job, update_job) use cursor.fetchone() / execute(...) returning positional rows and never need a row factory, so it's only used by get_job — but assigning it on the shared _db connection inside one method means any concurrent caller that hasn't read this code may suddenly get back Row objects where they previously got tuples; and (b) the write paths' self._db.execute(...) return value is otherwise untyped, but now row_factory is mutated under them too. Either set the row factory once in init() (mirroring license_acceptances_store.py's approach) or open a dedicated cursor with a per-cursor factory, so this side effect stays local.
|
|
||
| from tinyagentos.base_store import BaseStore | ||
|
|
||
| SCHEMA = """ |
There was a problem hiding this comment.
WARNING: The license_acceptances schema records only (user_id, app_id, license_id, accepted_at). There is no IP address, user agent, license-text snapshot, or accepted-version marker. license_id keys off weights_license or manifest.id in the route, but weights_license is a free-text label in the manifest (see registry.py) and could be edited later without bumping the id. Two practical risks: (1) if the catalog maintainer rewords "CC-BY-NC 4.0" → "CC-BY-NC-4.0", the new wording is treated as a different license and silently re-prompts every user who accepted the old wording, even though the legal text didn't change; (2) if a user later disputes "I never agreed to that license", the system has no evidence of what they agreed to (no text snapshot, no IP). At minimum, hash the canonicalized license text into license_id, and capture request metadata (IP, UA, route name) at acceptance time so the record is defensible.
Code Review SummaryStatus: 5 Issues Found | Recommendation: Address before next release Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (40 files)
Fix these issues in Kilo Cloud Reviewed by minimax-m3 · Input: 70.2K · Output: 10.6K · Cached: 1.8M |
1 participant
Promotes beta.28 (App Studio real, licensing accept-gate, Video async) to master.
Summary by CodeRabbit
New Features
Bug Fixes