Conversation
- Add "studios" to NavId union and NAV array (after "apps", Sparkles icon) - Add studioState field to CatalogApp type for installed/available/soon lifecycle - Add STUDIOS_APPS catalog entries (type "studio") with cover gradients - Add NAV_TYPE_MAP entry for "studios" -> ["studio"] - Wire StudiosView into the content switcher (alongside discover/community) - New StudiosView component: hero card (Coding Studio), 4x2 taOS Studios grid with Soon badges, community studios horizontal scroll, layout chips row - StudiosView.test.tsx: 6 tests covering headings, cards, Soon badges, hero
Adds CodingStudioApp as an optional platform app with three views: BuildView (file tree + syntax editor + build log panel), TemplatesView (hero prompt + 8 template cards), and PreviewView (URL bar + device toggle + simulated todo app + dev console). Registered in app-registry at launchpadOrder 13.25, optional: true. Includes 6 vitest tests covering titlebar, rail items, and view switching.
First-pass static UI: Studio (transport bar, track list, timeline with clip blocks, right inspector, piano roll), Compose (AI prompt + style chips + results list), Sounds (filter pills + 4-col instrument grid). Matches music-studio-mock.html layout and follows CodingStudioApp shell pattern (46px titlebar, 68px rail, shell tokens, muted track palette). 9/9 vitest tests pass, tsc clean, vite build succeeds.
Canvas-editor shell matching the approved mock: Design/Templates/Magic views, left element rail, artboard with selection box, properties panel with Magic edits chip bar, 8-card template grid, and AI prompt tile.
Static first-pass UI matching the approved office-suite-mock.html design. Follows CodingStudioApp shell pattern: 46px titlebar, 68px icon rail, per-view subfolder components, shell tokens throughout.
…es/Publish views Static first-pass UI matching the approved mock. Follows the canonical studio shell pattern (46px titlebar, 68px icon rail, per-view subdir). No registry wiring -- lead handles that separately. - AppStudioApp.tsx: shell with Build/Templates/Publish/SDK rail - appstudio/BuildView.tsx: checkerboard sandbox preview (Chore Quest app-in-window), build log steps, capabilities panel, model pill, prompt bar - appstudio/TemplatesView.tsx: hero + 8-card template grid - appstudio/PublishView.tsx: app identity header, capability toggles (Workspace/Notifications/Household), safety note, publish side panel - AppStudioApp.test.tsx: 5 tests covering titlebar, rail items, default Build view, Templates switch, and Publish switch with capability rows
Register Coding/Design/Music/App/Office as optional desktop apps. Extend the optional-frontend-app allowlist to cover them so /api/apps/optional/ install accepts them. Wire the Store Studios section: Get installs via the optional-app endpoint and emits APP_OPTIONAL_CHANGED (the launcher surfaces it at once); Open launches the app via the process store. Flip the four newly built studios from 'soon' to 'available'; Web Studio stays 'soon'.
…nstall wired + on Pi for review
feat(studios): Creative Studios -- Store Studios section + 5 studio apps + install wiring
…ace-app-packages decision + recon running
…recon saved + state snapshot
…ages spec drafted (awaiting sign-off)
* feat(apps): per-app versioning + Updates UI surface (#89 P1) * fix(apps): address gitar findings on #89 P1 - Store: hoist the taOS Apps updates section above the grid so optional-app updates show even when framework updates also exist (was nested in the filtered.length===0 empty-state branch and hidden otherwise) - name/icon for optional apps now come from the app registry (getApp), so the five studios resolve proper names/icons instead of raw ids + generic icon - _semver_tuple pads to (major,minor,patch) so '1.0' and '1.0.0' compare equal, and returns (0,0,0) on parse failure so it never masks a real update
…signing, web-first); Automation Studio spec signed off + deferred
#967 fix-forward, CodeRabbit)
* feat(userspace): re-integrate web app-runtime foundation onto dev (#89 P3a) * fix(userspace): fold CodeRabbit findings on the app-runtime foundation (#89 P3a) * fix(userspace): zip-bomb size caps + reject app.files.write to the jail root (#89 P3a) - package.py: cap declared uncompressed total, per-member size, and member count before extraction (zip-bomb defense, gitar Security finding) - install_app: bound the upload/fetch read to a max size, return 413 if exceeded - broker.py: app.files.write to the jail root or any existing dir now returns invalid_path instead of an uncaught IsADirectoryError (500); path containment uses is_relative_to - tests for the zip-bomb cap and the jail-root write guard
…P3b building; nit sweep done
…4 on unknown app (security review) A caller could previously grant a userspace app any capability regardless of what its manifest requested. Now the granted set is intersected with permissions_requested, and unknown apps return 404. Test package updated to request the caps it exercises.
…b) (#972) * feat(userspace): trust-aware CSP, capabilities, and theme injection for first-party packages (#89 P3b) - store.py: add trust column (TEXT DEFAULT 'community') with _post_init migration (PRAGMA table_info + ALTER TABLE -- same pattern as knowledge_store and agent_registry_store); install() accepts trust kwarg - routes/userspace_apps.py: public install endpoint always writes trust='community' (first-party comes only from internal seed/P2 sig path); serve_bundle picks _BUNDLE_CSP_FIRST_PARTY vs _BUNDLE_CSP by app trust; broker route passes full GATED_CAPS set as granted for first-party apps, per-grant set for community apps - SandboxedAppWindow.tsx: accepts trust prop; posts taosTheme tokens into the iframe on load and on scheme changes for first-party apps only; community apps receive no theme injection - userspace-apps.ts: UserspaceAppRow gains optional trust field; toAppManifest threads it into the component closure for SandboxedAppWindow - taos-app-sdk.js: adds taos.theme.get() / taos.theme.subscribe(cb) backed by the taosTheme postMessage; community apps never receive messages so the API simply returns an empty object for them - tests: 10 new backend tests (store migration, public endpoint enforces community, CSP by trust, broker grants for first-party vs community, GATED_CAPS unit); 8 new frontend tests (theme injection on/off by trust, SDK theme message handling, trust field in toAppManifest) * test(userspace): community-grant test requests the cap it grants (reconcile with set_permissions security fix) * fix(userspace): close trust-retention on reinstall + fold CodeRabbit #972 findings CRITICAL: the install UPSERT did not update trust on conflict, so a public install (always community) of an existing first-party app id would overwrite the bundle while keeping first-party privileges. Fixed two ways: - store.install UPSERT now sets trust=excluded.trust (a community reinstall downgrades trust; re-seeding first-party stays first-party) - the public install endpoint rejects (409) replacing an app already installed as first-party, so a trusted studio's bundle cannot be clobbered at all Also: SDK guards taosTheme against non-object/array payloads; regression tests for the 409 reject + the UPSERT trust update; SDK array-rejection test.
… complete (P3b #972 merged); P4 plan
…4a) (#973) * feat(userspace): first-party reference .taosapp + boot-seeding (#89 P4a) Adds tinyagentos/userspace/seed/welcome/ (manifest.yaml + index.html), a minimal first-party reference app that exercises the SDK theme API (get + subscribe), a kv round-trip, and a notify call. Adds tinyagentos/userspace/seed.py exposing seed_bundled_apps, which builds each seed subdirectory into an in-memory .taosapp zip, validates it through extract_package, and calls store.install with trust="first-party". Idempotent: skips apps already installed at the same version; re-seeds on a version bump. A seeding error is caught and logged -- it does not crash startup. Wires seed_bundled_apps into the app.py lifespan immediately after the userspace store and data store are initialised. The call is wrapped in a try/except so a seeding failure only emits a warning. Adds tests/userspace/test_seed.py covering: install as first-party, idempotency, version-bump re-seed, missing seed_dir is silent, bundle route returns first-party CSP, and the real bundled taos-welcome app for all of the above. * fix(userspace): seed idempotency requires first-party trust + clears stale files (CodeRabbit/gitar #973) - the skip-if-same-version check now also requires trust=first-party, so a community row claiming a seeded id is re-seeded to first-party rather than skipped (CodeRabbit Major) - a re-seed rmtree's the extracted app dir before extracting, so a smaller new version cannot inherit stale files from the old one (gitar edge case) - tests for both
…st+package+seeding all on dev, holding for Jay
The controller and worker migrated to uv (pyproject.toml + uv.lock); CI installs with uv sync --frozen and there is no pip requirements manifest. dependabot was auto-detecting the uv project independently of the stale pip entry and opening uv-group PRs against the default branch (master), since no config entry pinned their target-branch. Converting the pip entry to a uv ecosystem entry keeps Python dependency PRs on dev.
…aunchpad (#89) Wires the userspace app runtime into the desktop launcher: installed .taosapp packages (the seeded first-party welcome app plus community apps) now appear in an Apps section and open into sandboxed windows. Registry id namespaced as userspace:<id> to avoid shadowing built-in apps; syncUserspaceApps reconciles by id preserving manifest identity (no remount of open windows) and clears prefetch state on removal; useInstalledUserspaceApps mirrors the services hook and cancels in-flight refreshes. Store-side install/manage UI is a separate follow-up.
…does not overlap the workspace preview (#975)
…S); first grok dogfood loop closed end to end
… field is empty (#976)
…tion (#978) Fetch active external-selfjoin registry entries and render matching project members under External / Connected agents with an external tag, while deployed container agents stay in the main Members list.
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. |
📝 WalkthroughWalkthroughThis PR adds creative studio desktop apps and store surfaces, introduces userspace app runtime and desktop integration, expands optional app catalog and update displays, updates project and messaging UI behavior, suppresses docs-only update notifications, and includes README, status, and Dependabot changes. ChangesCreative studios and optional app catalog
Userspace app runtime and desktop integration
Project members and messaging fixes
Update suppression, docs, and repository maintenance
Sequence Diagram(s)sequenceDiagram
participant Desktop as Desktop shell
participant Iframe as Userspace iframe
participant API as userspace_apps router
participant Broker as userspace broker
participant Store as userspace data store
Desktop->>Iframe: load sandboxed bundle + theme tokens
Iframe->>Desktop: postMessage capability request
Desktop->>API: POST /api/userspace-apps/{appId}/broker
API->>Broker: handle_capability(...)
Broker->>Store: app-scoped kv/table/file operation
Store-->>Broker: result
Broker-->>API: broker response
API-->>Desktop: JSON result
Desktop-->>Iframe: postMessage reply
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
Comment on lines
+19
to
+33
| def is_safe_public_url(url: str) -> bool: | ||
| """True only if url is http(s) and every resolved IP is a public address. | ||
|
|
||
| Rejects private, loopback, link-local, reserved, unspecified and multicast | ||
| addresses (covers 127/8, ::1, 10/8, 172.16/12, 192.168/16, 169.254/16, | ||
| 0.0.0.0, etc.). | ||
| """ | ||
| try: | ||
| parsed = urlparse(url) | ||
| except ValueError: | ||
| return False | ||
| if parsed.scheme not in _ALLOWED_SCHEMES or not parsed.hostname: | ||
| return False | ||
| try: | ||
| infos = socket.getaddrinfo(parsed.hostname, parsed.port or None) |
There was a problem hiding this comment.
⚠️ Security: SSRF guard bypassable via DNS rebinding (TOCTOU)
is_safe_public_url() resolves the hostname with socket.getaddrinfo and validates the resulting IPs, but the subsequent fetch in install_app (httpx.AsyncClient(...).get(url)) performs its OWN independent DNS resolution. An attacker who controls a domain can return a public IP during the guard check and a blocked internal address (e.g. 169.254.169.254 cloud-metadata, 127.0.0.1, RFC1918) at fetch time — the classic DNS-rebinding TOCTOU. follow_redirects=False does not close this gap because the very first request re-resolves the host.
The guard correctly handles redirects and IP-literal hosts, but the time-of-check vs time-of-use split means the validation result is not the address actually connected to.
Fix: resolve once, validate the resolved IP(s), then connect to that pinned IP rather than re-resolving. One approach is to pass the validated IP to httpx and set the original Host header / SNI, or use a custom httpx transport/resolver that returns only the pre-validated address. The guard should return the resolved IP so the caller can pin it.
Resolve+validate once and connect to the pinned IP to eliminate the rebinding window.:
# url_guard: return the validated IP so the caller can pin it
def resolve_safe_public_ip(url: str) -> str | None:
parsed = urlparse(url)
if parsed.scheme not in _ALLOWED_SCHEMES or not parsed.hostname:
return None
try:
infos = socket.getaddrinfo(parsed.hostname, parsed.port or None)
except (socket.gaierror, UnicodeError, OSError):
return None
chosen = None
for info in infos:
ip = ip_address(info[4][0])
if (ip.is_private or ip.is_loopback or ip.is_link_local
or ip.is_reserved or ip.is_unspecified or ip.is_multicast):
return None
chosen = chosen or str(ip)
return chosen
# caller: connect to the pinned IP, keep Host header = original hostname
Was this helpful? React with 👍 / 👎
Comment on lines
+36
to
+50
| if capability == "app.kv.get": | ||
| return {"result": await data_store.kv_get(app_id, args["key"])} | ||
| if capability == "app.kv.set": | ||
| await data_store.kv_set(app_id, args["key"], args.get("value")) | ||
| return {"result": True} | ||
| if capability == "app.kv.delete": | ||
| await data_store.kv_delete(app_id, args["key"]) | ||
| return {"result": True} | ||
| if capability == "app.kv.keys": | ||
| return {"result": await data_store.kv_keys(app_id)} | ||
| if capability == "app.table.insert": | ||
| return {"result": await data_store.table_insert(app_id, args["table"], args.get("row", {}))} | ||
| if capability == "app.table.query": | ||
| return {"result": await data_store.table_query(app_id, args["table"], args.get("where"))} | ||
| if capability == "app.table.delete": |
There was a problem hiding this comment.
💡 Edge Case: Broker capability args missing required keys cause 500s
Several capability handlers index required args directly: args["key"] (app.kv.get/set/delete), args["table"] (app.table.insert/query/delete) and args["id"] (app.table.delete). A sandboxed app that sends a malformed message (valid capability but omitting a required key) triggers an unhandled KeyError, which surfaces as an HTTP 500 from /broker rather than a structured {"error": ...} response. Combined with body = await request.json() in the broker route having no try/except (unlike install_app/set_permissions), invalid JSON from the iframe also produces a 500. These are reachable from untrusted community apps and are inconsistent with the graceful error contract the rest of the broker follows.
Suggest validating/.get()-ing required args and returning {"error": "invalid_args", ...}, and wrapping the broker route's request.json() in the same try/except used by the other endpoints.
Validate required args and return a structured error instead of raising KeyError.:
if capability == "app.kv.get":
key = args.get("key")
if not isinstance(key, str):
return {"error": "invalid_args", "capability": capability}
return {"result": await data_store.kv_get(app_id, key)}
# ...apply the same guard pattern to kv.set/delete and table.insert/query/delete
Was this helpful? React with 👍 / 👎
|
Note Your trial team has used its Gitar budget, so automatic reviews are paused. Upgrade now to unlock full capacity. Comment "Gitar review" to trigger a review manually. Code Review
|
| Compact |
|
Was this helpful? React with 👍 / 👎 | Gitar
List active registry agents under External / Connected agents and add them as native members by handle when selected.
Wait for the external registry to load before classifying unknown members into the main list, avoiding a brief flash when external agents jump sections. Hide canvas and lead controls on external rows.
…tasks shipped), update method tested, promotion held on SSRF
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| @@ -0,0 +1 @@ | |||
| /Volumes/NVMe/Users/jay/Development/tinyagentos/desktop/node_modules No newline at end of file | |||
There was a problem hiding this comment.
This is an absolute symlink to /Volumes/NVMe/User. Committing host-specific symlinks will break checkouts on other machines and can leak local environment assumptions. Remove the symlink and add node_modules to .gitignore instead.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
|
|
||
| # Relaxed CSP for first-party packages (studios). Still sandboxed -- NEVER | ||
| # add allow-same-origin; that would collapse the opaque-origin isolation and | ||
| # let the frame access session cookies. The relaxations over community: |
There was a problem hiding this comment.
If the request body contains "granted": null, body.get("granted", []) returns None and this list comprehension raises TypeError, producing a 500. Validate that granted is a list of strings before iterating.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| "connect-src 'self'; " | ||
| "frame-ancestors 'self'; base-uri 'none'" | ||
| ) | ||
|
|
There was a problem hiding this comment.
This broker route calls request.json() without handling invalid JSON or non-object JSON. A malformed broker request can therefore raise and return a 500 instead of a controlled 400 response.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| store = request.app.state.userspace_apps | ||
| if package is not None: | ||
| data = await package.read(_MAX_PACKAGE_BYTES + 1) | ||
| else: |
There was a problem hiding this comment.
Raw app_id is used to construct a filesystem path. Since app_id comes from the path parameter, validate it against the installed app record or use a store-owned app directory before resolving/deleting; otherwise crafted ids can target sibling app directories.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| url = body.get("source_url") | ||
| if not url: | ||
| return JSONResponse({"error": "source_url or package required"}, status_code=400) | ||
| # SSRF guard: only fetch public http(s) hosts, and do not follow |
There was a problem hiding this comment.
Same concern here: raw app_id is resolved into a bundle-serving path before validating that it identifies the installed app. A crafted id with path separators can target another app directory under the same root.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| (app_id, table, json.dumps(row))) | ||
| await self._db.commit() | ||
| return cur.lastrowid | ||
|
|
There was a problem hiding this comment.
where is assumed to be a mapping. If an app calls table.query(table, where="x") or passes a list, this raises AttributeError and returns a 500. Validate where is None or a mapping before iterating.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| def parse_manifest(text: str) -> dict: | ||
| try: | ||
| data = yaml.safe_load(text) or {} | ||
| except yaml.YAMLError as exc: |
There was a problem hiding this comment.
decode("utf-8") can raise UnicodeDecodeError, which is not caught here. Catch it and return PackageError("manifest.yaml is not valid UTF-8") so malformed packages get a controlled 400 response.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| try: | ||
| zf = zipfile.ZipFile(io.BytesIO(data)) | ||
| except zipfile.BadZipFile as exc: | ||
| raise PackageError("not a valid .taosapp (zip) archive") from exc |
There was a problem hiding this comment.
Required manifest fields are only checked for truthiness. A manifest like id: 123 passes this check but can cause TypeError later when building apps_root / manifest["id"]. Validate required fields as strings, and validate permissions as a list of strings.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| # app_dir itself must stay within apps_root (defends against a crafted id) | ||
| if not app_dir.is_relative_to(apps_root) or app_dir == apps_root: | ||
| raise PackageError(f"unsafe path in package: id {manifest['id']!r}") | ||
| app_dir.mkdir(parents=True, exist_ok=True) |
There was a problem hiding this comment.
This extracts members directly into app_dir while iterating. If a later member is rejected, the app directory may be left partially populated. Pre-validate all member paths first, or extract to a temporary directory and atomically move it into place only after the package is fully accepted.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 13 Issues Found | Recommendation: Address before merge Overview
Incremental review covered the latest docs-only update suppression changes. No previous issues were resolved in the new commit; the 11 earlier findings are carried forward below. Fix link: Fix these issues in Kilo Cloud Issue Details (click to expand)CRITICAL
WARNING
SUGGESTION
Other Observations (not in diff)Unchanged files carried forward from the previous review:
Files Reviewed (3 files)
Previous Review Summary (commit 9bfd008)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 9bfd008)SummaryReviewed PR #979 in read-only mode after fetching the latest PR state, diff, and existing comments. I posted 9 new inline review comments focused on security, runtime error handling, and package extraction safety. New Review Comments
Existing Review CommentsExisting comments on Review StatusThis PR needs fixes before merge. The highest-risk items are raw Kilo fix link: https://app.kilo.ai/cloud-agent-fork/review/a9f07fc0-3d6a-4142-8a1c-0b5543897419 Reviewed by nex-n2-pro:free · 1,340,108 tokens |
When the tracked branch tip only changes documentation (docs/ or *.md), suppress the Update available notification and has_updates=false on update-check. Code or mixed diffs still report normally.
| paths = [line.strip() for line in out.splitlines() if line.strip()] | ||
| if not paths: | ||
| return False | ||
| return all(is_documentation_path(p) for p in paths) |
There was a problem hiding this comment.
SUGGESTION: git diff --name-only can miss mode-only changes
changes_are_docs_only() classifies a diff from file names alone. If a commit changes documentation plus only the executable bit of a code file, --name-only will not list the code file, so the mixed commit can be treated as docs-only. Use git diff --name-status or --summary and treat non-documentation paths or mode changes as non-doc to keep update suppression conservative.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| (current or "")[:7], | ||
| new_commit[:7], | ||
| ) | ||
| return |
There was a problem hiding this comment.
SUGGESTION: Docs-only skip short-circuits framework polling
When changes_are_docs_only() returns true, _run_once() returns before poll_frameworks(...). That delays framework release checks until the next cycle whenever the tracked branch has a docs-only commit. Move framework polling before the docs-only return, or return only from the repository-update notification block, so framework polling remains independent of repo commit classification.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Treat paths as documentation only when they end in a doc extension or live under docs/ without a code/config extension, so files like docs/scripts/foo.py no longer suppress real update notifications.
…#971) The install-from-URL path validated source_url against private/loopback/ link-local ranges, then let httpx re-resolve the hostname at fetch time. A host that resolved public during the check could resolve to an internal IP at connect time (DNS-rebind TOCTOU), defeating the guard. resolve_safe_public_ip() now resolves the host once and returns a validated public IP. The install fetch connects to that pinned IP and carries the original Host header + TLS SNI, so vhost routing and cert validation still work but the client never re-resolves. follow_redirects stays off. Also centralises required-arg validation in the broker so a missing key returns {"error": "missing_arg"} instead of an uncaught KeyError (500).
fix(userspace): pin install fetch to validated IP (DNS-rebind SSRF #971)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/userspace/test_routes.py (1)
123-137:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThe test does not verify the “no extracted directory remains” security guarantee.
This test only checks DB state. It should also verify the rejected container bundle is not retrievable, otherwise filesystem leftovers can pass unnoticed.
Proposed test hardening
async def test_container_install_rejected_with_no_stored_state(client): @@ # No app row stored. rows = (await client.get("/api/userspace-apps")).json() assert all(a["app_id"] != "ctapp" for a in rows) + # No bundle content should be reachable after rejection. + bundle = await client.get("/api/userspace-apps/ctapp/bundle/index.html") + assert bundle.status_code == 404🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/userspace/test_routes.py` around lines 123 - 137, The test function test_container_install_rejected_with_no_stored_state verifies that no app row is stored in the database after a rejected container installation, but it does not verify the corresponding filesystem guarantee that no extracted directory remains. Add an assertion or verification step after the database check to confirm that the extracted container bundle directory is not accessible or does not exist, ensuring the complete security guarantee mentioned in the comment about leaving no app row and no extracted directory behind.
♻️ Duplicate comments (1)
tinyagentos/routes/userspace_apps.py (1)
256-256:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUnhandled JSON parsing error in broker endpoint.
await request.json()can raise on invalid or non-JSON request bodies, resulting in a 500 instead of a controlled 400 response. Wrap in try/except as done in other endpoints.🐛 Proposed fix
`@router.post`("/api/userspace-apps/{app_id}/broker") async def broker(request: Request, app_id: str): store = request.app.state.userspace_apps app = await store.get(app_id) if app is None or not app["enabled"]: return JSONResponse({"error": "app not found or disabled"}, status_code=404) - body = await request.json() + try: + body = await request.json() + except Exception: + return JSONResponse({"error": "invalid JSON body"}, status_code=400) # First-party apps have all gated capabilities pre-authorised -- no per-cap🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tinyagentos/routes/userspace_apps.py` at line 256, The `await request.json()` call in the broker endpoint lacks error handling for malformed or invalid JSON in the request body. Wrap the line `body = await request.json()` in a try/except block to catch JSON parsing exceptions and return a controlled 400 Bad Request response instead of allowing the exception to propagate as a 500 error. Follow the same pattern already implemented in other endpoints in the codebase for consistency.
🟡 Minor comments (11)
desktop/src/apps/MessagesApp.tsx-2761-2761 (1)
2761-2761:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHandle empty-string topics in fallback logic.
Line 2761 uses
??, sotopic: ""won’t fall back todescription. That misses the “empty topic uses description” behavior and can still render blank topic text.Proposed fix
- topic: currentChannel.topic ?? currentChannel.description ?? "", + topic: (currentChannel.topic && currentChannel.topic.trim().length > 0) + ? currentChannel.topic + : (currentChannel.description ?? ""),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@desktop/src/apps/MessagesApp.tsx` at line 2761, The fallback logic for the topic field at line 2761 uses the nullish coalescing operator `??` which only handles null and undefined values, not empty strings. When currentChannel.topic is an empty string, it will not fall back to currentChannel.description as intended, resulting in blank topic text being rendered. Replace the nullish coalescing operator with a truthy check (using the logical OR operator `||` or a ternary conditional) that treats both null/undefined AND empty strings as falsy values, ensuring that empty topics properly fall back to the description value.desktop/src/apps/StoreApp/index.tsx-933-935 (1)
933-935:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRefresh optional catalog state after initial mount to avoid stale Updates results.
Line 933 fetches
optionalCatalogonly once, but it drives both the optional updates list and the “up to date” empty state later. In-session optional install/uninstall changes can leave this view stale until reload.Also applies to: 1154-1156, 1182-1187
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@desktop/src/apps/StoreApp/index.tsx` around lines 933 - 935, The optional catalog is fetched only once on component mount, causing the state to become stale when users install or uninstall optional apps during the session. Extract the fetch logic that retrieves the optional catalog from the current useEffect into a separate refresh function, then call this function not only during the initial mount but also after optional app installations and uninstalls are completed. This ensures the optionalCatalog state is refreshed in-session whenever the app list changes, keeping the optional updates list and "up to date" empty state synchronized with actual installations/uninstalls.desktop/src/apps/StoreApp/index.tsx-1154-1176 (1)
1154-1176:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winApply the active search query to optional-update rows in the Updates tab.
Optional rows are rendered without search filtering, while the standard updates list is filtered. This can produce contradictory UI (optional rows visible while “No matches” is shown).
Suggested fix
- {activeNav === "updates" && (() => { - const updatableOptional = optionalCatalog.filter((e) => e.installed && e.update_available); + {activeNav === "updates" && (() => { + const q = search.trim().toLowerCase(); + const updatableOptional = optionalCatalog.filter((e) => { + if (!e.installed || !e.update_available) return false; + if (!searching) return true; + const label = (getApp(e.id)?.name ?? e.id).toLowerCase(); + return label.includes(q); + }); if (updatableOptional.length === 0) return null; return ( @@ - ) : activeNav === "updates" && filtered.length === 0 ? ( - optionalCatalog.some((e) => e.installed && e.update_available) ? null : ( + ) : activeNav === "updates" && filtered.length === 0 ? ( + optionalCatalog.some((e) => { + if (!e.installed || !e.update_available) return false; + if (!searching) return true; + const label = (getApp(e.id)?.name ?? e.id).toLowerCase(); + return label.includes(search.trim().toLowerCase()); + }) ? null : (Also applies to: 1182-1187
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@desktop/src/apps/StoreApp/index.tsx` around lines 1154 - 1176, The updatableOptional list in the updates tab is not filtered by the active search query, causing optional update rows to display even when the search produces no matches elsewhere. Apply the same search filtering logic used for the main filtered updates list to the updatableOptional filter. The updatableOptional.filter call on line 1155 should include an additional condition that matches against the search query in the same way the main updates list is filtered. Also apply the same fix to the optional update section mentioned at lines 1182-1187 to ensure consistency across all optional update displays in the Updates tab.desktop/src/apps/DesignStudioApp.tsx-66-69 (1)
66-69:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winElements rail item maps to DesignView instead of an elements surface.
Selecting Elements currently shows the Design surface, so navigation state and content are inconsistent. Add an
ElementsViewplaceholder (or rename/remove the rail item) until the dedicated surface is ready.Suggested minimal fix
- {view === "elements" && <DesignView />} + {view === "elements" && ( + <div className="flex flex-1 items-center justify-center text-sm text-shell-text-tertiary"> + Elements view coming soon + </div> + )}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@desktop/src/apps/DesignStudioApp.tsx` around lines 66 - 69, The conditional rendering in DesignStudioApp.tsx incorrectly maps the "elements" view to DesignView instead of a dedicated ElementsView component. Replace the line that renders DesignView when view equals "elements" with a corresponding ElementsView component. Either create a new ElementsView placeholder component or use an existing appropriate component for the elements surface. This ensures that selecting the Elements rail item displays the correct content instead of duplicating the Design view.desktop/src/apps/CodingStudioApp.tsx-66-69 (1)
66-69:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCode rail item maps to the wrong surface.
Clicking Code renders
BuildView, so two rail items lead to the same content. Either add a dedicatedCodeViewplaceholder or temporarily remove/rename the rail item to avoid misleading navigation.Suggested minimal fix
- {view === "code" && <BuildView />} + {view === "code" && ( + <div className="flex flex-1 items-center justify-center text-sm text-shell-text-tertiary"> + Code view coming soon + </div> + )}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@desktop/src/apps/CodingStudioApp.tsx` around lines 66 - 69, The conditional rendering in the CodingStudioApp component has a bug where the "code" view renders the same BuildView component as the "build" view, creating duplicate navigation paths. Replace the line that renders BuildView when view === "code" with either a dedicated CodeView component or remove the code branch entirely if CodeView is not yet implemented. Ensure each view option in the conditional rendering maps to a unique content surface.desktop/src/apps/officesuite/WriteView.tsx-113-115 (1)
113-115:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate stale studio names in the sample copy.
Lines 113-115 mention “Images Studio and Game Studio,” which does not match the studio set introduced here. This can confuse users during demos/tests.
Suggested fix
- Images Studio and Game Studio are available now. Coding Studio is rolling out, with - Design, Music, App, and Office studios close behind. Each one installs from the Store + Coding, Design, Music, App, and Office studios are available now. Each one installs from the Store in a single click.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@desktop/src/apps/officesuite/WriteView.tsx` around lines 113 - 115, The sample copy text in the WriteView component on lines 113-115 references outdated studio names "Images Studio and Game Studio" that do not match the current studio set being described in the surrounding text which mentions "Coding Studio," "Design," "Music," "App," and "Office" studios. Update the text in this section to reference the correct, current studio names instead of the stale ones to ensure consistency and avoid confusing users during demos or testing.README.md-224-224 (1)
224-224:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate Creative Studios wording to match shipped functionality.
Line 224 still says Coding/App/Design/Music/Office studios are “on the way,” but this PR introduces those studio apps as optional installable shells. Please revise this sentence so release docs reflect actual availability.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` at line 224, The sentence in the README describes Coding, App, Design, Music, and Office studios as "on the way," but this PR introduces them as optional installable shells that are now available. Update the wording to reflect that these studio apps are now available as optional installations rather than future functionality, while keeping the clarification about App Studio being taOS's own app builder.desktop/src/apps/MusicStudioApp.tsx-56-68 (1)
56-68:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd active-state semantics for the Export rail button.
Line 56-68 should expose active navigation state like the other rail items; currently
Exportnever setsaria-current, so assistive tech loses parity with Studio/Compose/Sounds/Mixer.Suggested patch
<button type="button" aria-label="Export" + aria-current={view === "export" ? "page" : undefined} onClick={() => setView("export")} className={`flex h-[46px] w-[46px] flex-col items-center justify-center gap-0.5 rounded-xl text-[9px] font-semibold transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-accent/40 ${🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@desktop/src/apps/MusicStudioApp.tsx` around lines 56 - 68, The Export button in the MusicStudioApp component lacks active-state semantics that other rail items provide. Add an aria-current attribute to the Export button that sets it to "page" when view equals "export" and is false or omitted otherwise. This will ensure assistive technologies can properly announce the active navigation state, providing parity with the other rail buttons like Studio, Compose, Sounds, and Mixer.tests/userspace/test_data_store.py-7-7 (1)
7-7:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSplit multi-statement lines to satisfy Ruff E702 and keep tests readable.
These two lines currently chain statements with semicolons.
Suggested fix
- s = UserspaceDataStore(tmp_path / "d.db"); await s.init() + s = UserspaceDataStore(tmp_path / "d.db") + await s.init() @@ - s = UserspaceDataStore(tmp_path / "d.db"); await s.init() + s = UserspaceDataStore(tmp_path / "d.db") + await s.init()Also applies to: 22-22
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/userspace/test_data_store.py` at line 7, Split the chained statements separated by semicolons into separate lines. On line 7, split the UserspaceDataStore instantiation and the await s.init() call into two distinct lines. Apply the same refactoring to line 22 where a similar multi-statement line exists with a semicolon separator. This will satisfy the Ruff E702 rule and improve test readability.Source: Linters/SAST tools
tests/userspace/test_broker.py-6-7 (1)
6-7:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSplit the one-line helper statements to satisfy Ruff E702.
Proposed fix
async def _store(tmp_path): - s = UserspaceDataStore(tmp_path / "d.db"); await s.init(); return s + s = UserspaceDataStore(tmp_path / "d.db") + await s.init() + return s🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/userspace/test_broker.py` around lines 6 - 7, The _store function violates Ruff E702 by having multiple statements on a single line separated by semicolons. Split the single line into three separate lines within the function body, placing the UserspaceDataStore initialization, the await s.init() call, and the return statement each on their own line.Source: Linters/SAST tools
tinyagentos/routes/userspace_apps.py-26-54 (1)
26-54:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCommunity and first-party CSP are identical.
_BUNDLE_CSPand_BUNDLE_CSP_FIRST_PARTYare character-for-character identical. The comment mentions "relaxations over community" but none are actually applied. Either differentiate them as intended or consolidate into a single constant.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tinyagentos/routes/userspace_apps.py` around lines 26 - 54, The constants _BUNDLE_CSP and _BUNDLE_CSP_FIRST_PARTY are currently identical despite the detailed comment explaining intended relaxations for first-party packages. Either consolidate these two constants into a single _BUNDLE_CSP constant if they should be the same, or modify _BUNDLE_CSP_FIRST_PARTY to apply the documented relaxations mentioned in the comment above it (such as the connect-src and style-src adjustments) to differentiate it from the community CSP policy.
🧹 Nitpick comments (7)
desktop/src/apps/ProjectsApp/AddAgentDialog.tsx (1)
60-72: Backend already protects against duplicate member additions via database constraints; consider optimizing the client-side guard to prevent redundant requests.The render-snapshot-based
submittingcheck allows rapid double-triggers to issue multipleaddNative/addClonePOSTs. However, the backend'sON CONFLICT(project_id, member_id) DO NOTHINGclause silently deduplicates these requests at the database layer, preserving the originaladded_attimestamp. While data integrity is protected, preventing redundant calls improves efficiency and UX. The suggested fix using a synchronoususeRefguard is a reasonable optimization to avoid wasted network requests.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@desktop/src/apps/ProjectsApp/AddAgentDialog.tsx` around lines 60 - 72, The addExternal function's check for the submitting state is vulnerable to race conditions on rapid clicks since state updates are asynchronous, allowing multiple API calls to be issued before the state is actually updated. Add a synchronous useRef guard that is checked and set immediately at the start of the addExternal function before the submitting state check. Create a ref variable that tracks whether a request is currently in flight and synchronously set it to true before making the API call, then set it back to false in the finally block. This synchronous guard will reliably prevent redundant addNative calls regardless of how quickly the user clicks.desktop/src/apps/MusicStudioApp.test.tsx (1)
63-79: ⚡ Quick winAdd explicit tests for Mixer and Export rail states.
Current suite exercises Studio/Compose/Sounds, but Line 63-79 still leaves
mixerandexportpaths unverified. Add two small tests that click each rail item and assert botharia-current="page"and the expected placeholder text.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@desktop/src/apps/MusicStudioApp.test.tsx` around lines 63 - 79, Add two new test cases following the same pattern as the existing "switches to Sounds view on rail click" test to verify the Mixer and Export views. For each new test, call renderApp(), use fireEvent.click() to click the respective rail button (Mixer and Export), then verify that the aria-current attribute is set to "page" on the navigation button and assert that the expected heading or content for that view is defined using screen.getByRole() or screen.getByText(). This ensures complete coverage of all rail navigation states.tests/userspace/test_seed.py (2)
171-171: ⚡ Quick winAvoid hardcoding the real bundled app version in this test.
Pinning
"1.0.0"here creates avoidable churn whenevertinyagentos/userspace/seed/welcome/manifest.yamlis intentionally version-bumped.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/userspace/test_seed.py` at line 171, The test assertion in the test_seed.py file is hardcoding the version string "1.0.0" which creates maintenance burden when the actual version in tinyagentos/userspace/seed/welcome/manifest.yaml is updated. Instead of hardcoding the version value in the assertion, dynamically load the version from the manifest.yaml file and use that loaded value in the comparison. This way, the test will automatically use the current version from the manifest without requiring manual updates whenever the version is bumped.
79-89: ⚡ Quick winStrengthen idempotency assertions against timestamp-granularity false passes.
installed_atuses second-level time resolution, so an unintended reseed can still pass if both writes happen within the same second. Consider patching time in this test to make regressions deterministic.Also applies to: 183-190
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/userspace/test_seed.py` around lines 79 - 89, The installed_at timestamp assertion is vulnerable to false passes due to second-level time resolution granularity. Mock or patch the system time in the test to return controlled, deterministic timestamps so that the first and second seed operations would produce different installed_at values if an unintended reseed actually occurred. Apply this time mocking to both the initial seeding operation before the installed_at_first assertion and the subsequent seed_bundled_apps call, ensuring they use different mocked timestamps to make the idempotency check reliable.tests/userspace/test_immutability.py (1)
7-9: 💤 Low valueDuplicate test coverage.
test_native_app_type_rejectedduplicates the same test intests/userspace/test_package.py:33-37. Consider removing this duplicate to avoid maintenance burden.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/userspace/test_immutability.py` around lines 7 - 9, The test_native_app_type_rejected function in this file is a duplicate of an existing test in tests/userspace/test_package.py that covers the same functionality. Remove the entire test_native_app_type_rejected function from the current file to eliminate the duplicate test coverage and reduce maintenance burden.tests/userspace/conftest.py (1)
55-58: ⚡ Quick winFail fast if fixture auth bootstrap fails to create the admin user.
The empty-string fallback for
user_idcan silently hide broken fixture setup and produce invalid session state. Assert user creation succeeded before session creation.Proposed change
app.state.auth.setup_user("admin", "Test Admin", "", "testpass") record = app.state.auth.find_user("admin") - uid = record["id"] if record else "" - token = app.state.auth.create_session(user_id=uid, long_lived=True) + assert record is not None, "fixture setup failed: admin user missing" + token = app.state.auth.create_session(user_id=record["id"], long_lived=True)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/userspace/conftest.py` around lines 55 - 58, The code silently defaults uid to an empty string when find_user("admin") fails to find the user, which can hide broken fixture setup and produce invalid session state. After the find_user call that retrieves the record, add an assertion to ensure the record exists and is not None before extracting the id value. This will cause the fixture to fail fast and clearly indicate that admin user creation or lookup failed, rather than proceeding with an invalid empty-string user_id to the create_session call.tests/userspace/test_update_consent.py (1)
21-25: ⚡ Quick winMake the permission-delta assertion exact.
"app.memory" in new_permissionscan still pass if previously granted permissions are incorrectly re-flagged. Assert status and exact delta.Proposed assertion tightening
r = await client.post("/api/userspace-apps/install", files={"package": ("t.taosapp", _zip("[app.net, app.memory]"), "application/zip")}) + assert r.status_code == 200, r.text body = r.json() assert body["needs_consent"] is True - assert "app.memory" in body["new_permissions"] + assert set(body["new_permissions"]) == {"app.memory"}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/userspace/test_update_consent.py` around lines 21 - 25, The assertion for body["new_permissions"] is too loose and only checks that "app.memory" exists within the set, which doesn't catch if previously granted permissions are incorrectly re-flagged as new. Replace the membership check with an exact assertion that verifies new_permissions equals exactly the expected set of permissions, ensuring only the truly new permission "app.memory" is included and no previously granted permissions are incorrectly present.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@desktop/node_modules`:
- Line 1: The desktop/node_modules directory is currently a tracked symbolic
link pointing to an absolute macOS-specific path that will break the repository
on other machines and CI/CD environments. Remove this symlink from git tracking
by using git rm --cached on the desktop/node_modules entry, then add
desktop/node_modules/ to the .gitignore file to prevent it from being tracked in
the future. After making these changes, verify that the project builds
successfully by running npm ci in the desktop directory to ensure dependencies
are properly installed without relying on the symlink.
In `@desktop/src/apps/designstudio/MagicView.tsx`:
- Around line 79-93: The interactive result tiles are currently rendered as div
elements instead of semantic button elements, making them inaccessible for
keyboard navigation. Replace the div element that wraps each result in the
RESULTS.map function (the one with key={label} and the className containing
"cursor-pointer") with a button element. Preserve all existing className and
style attributes, and ensure the button maintains the same visual appearance and
interactivity as the current div implementation.
In `@desktop/src/apps/officesuite/SlidesView.tsx`:
- Around line 30-52: Replace the div element that has role="button" with a
native HTML button element. The button should retain the key prop, tabIndex,
aria-label, className, and style attributes from the original div. Remove the
role="button" attribute since it's now redundant on a real button element. Keep
the span child element that displays the slide index and the s.label text
content as-is. This will automatically provide full keyboard support
(Enter/Space handling) and better accessibility without requiring manual event
handling.
In `@desktop/src/apps/ProjectsApp/ProjectMembers.tsx`:
- Around line 107-143: Add error handling and busy state management to the three
async handlers in the member permission UI. For the "Can edit canvas" checkbox
onChange handler (calling canvasApi.setPermission), the "Lead" checkbox onChange
handler (calling projectsApi.members.setLead), and the remove button onClick
handler (calling projectsApi.members.remove), wrap each API call in try-catch
blocks to handle errors gracefully, add a loading/busy state variable to disable
the input elements during the API call, and display error feedback to the user
when the API call fails so the UI remains consistent and prevents duplicate
requests from rapid clicks.
In `@desktop/src/apps/SettingsApp/UpdatesPanel.tsx`:
- Line 3: The namespace import of `lucide-react` in UpdatesPanel.tsx forces the
entire icon library into the bundle even though only specific optional-app icons
are used. Replace the `import * as LucideIcons` statement with explicit named
imports of only the icons actually referenced in the optional-app icon lookup
logic (around lines 41-49), then create a simple object map that maps icon names
to the imported icon components, eliminating the need for dynamic pascal-case
lookup via LucideIcons[].
In `@desktop/src/lib/__tests__/userspace-apps.test.ts`:
- Around line 5-8: The test assertions for the toAppManifest function are
expecting raw app IDs like "todo", but the function now returns namespaced IDs
in the format "userspace:${app_id}". Update the expect statements that check
m.id to expect "userspace:todo" instead of "todo" at both locations mentioned
(around line 7 and line 42) to match the current behavior of toAppManifest.
In `@desktop/src/registry/app-registry.ts`:
- Around line 203-222: The syncUserspaceApps function only appends new userspace
manifests but never updates existing ones, which prevents refreshed metadata
(name, icon), component closures, and trust information from being applied after
catalog changes. Modify the logic to not only add missing manifests with
apps.push(m), but also replace existing userspace manifests in the apps array
with their updated counterparts from the incoming manifests map to ensure
metadata and component closures are kept current.
In `@tinyagentos/auto_update.py`:
- Around line 203-221: The _DOC_EXTENSIONS tuple includes .txt which is too
broad and can incorrectly classify non-documentation files like dependency lists
or runtime configuration files as documentation, suppressing legitimate updates.
Remove the .txt extension from the _DOC_EXTENSIONS tuple to make the
is_documentation_path function classifier more conservative and only treat true
documentation file types as documentation-only paths.
In `@tinyagentos/userspace/broker.py`:
- Around line 34-46: The argument validation in the capability dispatcher only
checks for key presence in the _required dictionary but does not validate
argument types, allowing malformed values (non-string keys, non-int ids,
non-dict headers) to cause uncaught exceptions downstream. Extend the validation
logic that iterates through _required.get(capability, ()) to also check that
each argument has the correct expected type (e.g., key/path as string, id as
integer, headers as dict) and return a clean error response with
"invalid_arg_type" error when type validation fails, similar to the existing
missing_arg error handling.
In `@tinyagentos/userspace/data_store.py`:
- Line 72: The dictionary construction in the append statement for the out list
uses the wrong order of precedence, allowing the "id" key from the data
dictionary to override the authoritative rid value. In the line where out.append
is called with a dictionary containing "id": rid and **data unpacked, reverse
the order of precedence by placing the authoritative rid assignment after the
data unpacking so that {**data, "id": rid} ensures rid always takes precedence
over any "id" key that might exist in the data dictionary.
In `@tinyagentos/userspace/sdk/taos-app-sdk.js`:
- Around line 12-19: The message event listener in the anonymous callback does
not validate the origin of incoming messages before resolving pending RPC calls.
Add an origin validation check on the event object (e.origin) before processing
the message and resolving the promise. Only proceed with resolving the pending
call when the message originates from a trusted/expected origin, preventing
spoofed messages from different frames from hijacking in-flight RPC calls.
- Around line 29-34: The call function creates pending promises that are stored
in the pending map but have no timeout mechanism, causing memory leaks if broker
responses never arrive. Add a timeout handler within the call function that
rejects the promise and removes the entry from the pending map after a
reasonable duration (such as 30 seconds). Use setTimeout to trigger this
cleanup, and ensure the timeout is cleared if the response arrives before the
timeout expires by storing the timeout ID alongside the resolve function in the
pending map entry.
- Around line 40-67: The gated capability methods like agent.ask and
memory.search currently extract and return only r.result, which causes
permission_denied error responses to become undefined or hidden from callers.
Normalize error handling by modifying the gated capability methods (agent.ask at
line 62 and memory.search at line 63) to return the full response envelope
instead of extracting r.result, ensuring that error objects like {error:
"permission_denied"} are properly visible to callers. This should be consistent
with how other methods handle responses to provide uniform error detection
across all SDK methods.
In `@tinyagentos/userspace/seed.py`:
- Around line 68-73: The shutil.rmtree call in the re-seed block uses app_id to
construct the path without first validating that it stays within apps_root,
creating a security vulnerability where a malicious manifest id could delete
files outside the intended directory. Add path validation before the
shutil.rmtree call (before line 71) to ensure the resolved path stays within
apps_root, similar to how extract_package validates paths. Check that resolving
apps_root / app_id results in a path that is within apps_root and reject or
sanitize app_id values that would escape the directory.
---
Outside diff comments:
In `@tests/userspace/test_routes.py`:
- Around line 123-137: The test function
test_container_install_rejected_with_no_stored_state verifies that no app row is
stored in the database after a rejected container installation, but it does not
verify the corresponding filesystem guarantee that no extracted directory
remains. Add an assertion or verification step after the database check to
confirm that the extracted container bundle directory is not accessible or does
not exist, ensuring the complete security guarantee mentioned in the comment
about leaving no app row and no extracted directory behind.
---
Minor comments:
In `@desktop/src/apps/CodingStudioApp.tsx`:
- Around line 66-69: The conditional rendering in the CodingStudioApp component
has a bug where the "code" view renders the same BuildView component as the
"build" view, creating duplicate navigation paths. Replace the line that renders
BuildView when view === "code" with either a dedicated CodeView component or
remove the code branch entirely if CodeView is not yet implemented. Ensure each
view option in the conditional rendering maps to a unique content surface.
In `@desktop/src/apps/DesignStudioApp.tsx`:
- Around line 66-69: The conditional rendering in DesignStudioApp.tsx
incorrectly maps the "elements" view to DesignView instead of a dedicated
ElementsView component. Replace the line that renders DesignView when view
equals "elements" with a corresponding ElementsView component. Either create a
new ElementsView placeholder component or use an existing appropriate component
for the elements surface. This ensures that selecting the Elements rail item
displays the correct content instead of duplicating the Design view.
In `@desktop/src/apps/MessagesApp.tsx`:
- Line 2761: The fallback logic for the topic field at line 2761 uses the
nullish coalescing operator `??` which only handles null and undefined values,
not empty strings. When currentChannel.topic is an empty string, it will not
fall back to currentChannel.description as intended, resulting in blank topic
text being rendered. Replace the nullish coalescing operator with a truthy check
(using the logical OR operator `||` or a ternary conditional) that treats both
null/undefined AND empty strings as falsy values, ensuring that empty topics
properly fall back to the description value.
In `@desktop/src/apps/MusicStudioApp.tsx`:
- Around line 56-68: The Export button in the MusicStudioApp component lacks
active-state semantics that other rail items provide. Add an aria-current
attribute to the Export button that sets it to "page" when view equals "export"
and is false or omitted otherwise. This will ensure assistive technologies can
properly announce the active navigation state, providing parity with the other
rail buttons like Studio, Compose, Sounds, and Mixer.
In `@desktop/src/apps/officesuite/WriteView.tsx`:
- Around line 113-115: The sample copy text in the WriteView component on lines
113-115 references outdated studio names "Images Studio and Game Studio" that do
not match the current studio set being described in the surrounding text which
mentions "Coding Studio," "Design," "Music," "App," and "Office" studios. Update
the text in this section to reference the correct, current studio names instead
of the stale ones to ensure consistency and avoid confusing users during demos
or testing.
In `@desktop/src/apps/StoreApp/index.tsx`:
- Around line 933-935: The optional catalog is fetched only once on component
mount, causing the state to become stale when users install or uninstall
optional apps during the session. Extract the fetch logic that retrieves the
optional catalog from the current useEffect into a separate refresh function,
then call this function not only during the initial mount but also after
optional app installations and uninstalls are completed. This ensures the
optionalCatalog state is refreshed in-session whenever the app list changes,
keeping the optional updates list and "up to date" empty state synchronized with
actual installations/uninstalls.
- Around line 1154-1176: The updatableOptional list in the updates tab is not
filtered by the active search query, causing optional update rows to display
even when the search produces no matches elsewhere. Apply the same search
filtering logic used for the main filtered updates list to the updatableOptional
filter. The updatableOptional.filter call on line 1155 should include an
additional condition that matches against the search query in the same way the
main updates list is filtered. Also apply the same fix to the optional update
section mentioned at lines 1182-1187 to ensure consistency across all optional
update displays in the Updates tab.
In `@README.md`:
- Line 224: The sentence in the README describes Coding, App, Design, Music, and
Office studios as "on the way," but this PR introduces them as optional
installable shells that are now available. Update the wording to reflect that
these studio apps are now available as optional installations rather than future
functionality, while keeping the clarification about App Studio being taOS's own
app builder.
In `@tests/userspace/test_broker.py`:
- Around line 6-7: The _store function violates Ruff E702 by having multiple
statements on a single line separated by semicolons. Split the single line into
three separate lines within the function body, placing the UserspaceDataStore
initialization, the await s.init() call, and the return statement each on their
own line.
In `@tests/userspace/test_data_store.py`:
- Line 7: Split the chained statements separated by semicolons into separate
lines. On line 7, split the UserspaceDataStore instantiation and the await
s.init() call into two distinct lines. Apply the same refactoring to line 22
where a similar multi-statement line exists with a semicolon separator. This
will satisfy the Ruff E702 rule and improve test readability.
In `@tinyagentos/routes/userspace_apps.py`:
- Around line 26-54: The constants _BUNDLE_CSP and _BUNDLE_CSP_FIRST_PARTY are
currently identical despite the detailed comment explaining intended relaxations
for first-party packages. Either consolidate these two constants into a single
_BUNDLE_CSP constant if they should be the same, or modify
_BUNDLE_CSP_FIRST_PARTY to apply the documented relaxations mentioned in the
comment above it (such as the connect-src and style-src adjustments) to
differentiate it from the community CSP policy.
---
Duplicate comments:
In `@tinyagentos/routes/userspace_apps.py`:
- Line 256: The `await request.json()` call in the broker endpoint lacks error
handling for malformed or invalid JSON in the request body. Wrap the line `body
= await request.json()` in a try/except block to catch JSON parsing exceptions
and return a controlled 400 Bad Request response instead of allowing the
exception to propagate as a 500 error. Follow the same pattern already
implemented in other endpoints in the codebase for consistency.
---
Nitpick comments:
In `@desktop/src/apps/MusicStudioApp.test.tsx`:
- Around line 63-79: Add two new test cases following the same pattern as the
existing "switches to Sounds view on rail click" test to verify the Mixer and
Export views. For each new test, call renderApp(), use fireEvent.click() to
click the respective rail button (Mixer and Export), then verify that the
aria-current attribute is set to "page" on the navigation button and assert that
the expected heading or content for that view is defined using
screen.getByRole() or screen.getByText(). This ensures complete coverage of all
rail navigation states.
In `@desktop/src/apps/ProjectsApp/AddAgentDialog.tsx`:
- Around line 60-72: The addExternal function's check for the submitting state
is vulnerable to race conditions on rapid clicks since state updates are
asynchronous, allowing multiple API calls to be issued before the state is
actually updated. Add a synchronous useRef guard that is checked and set
immediately at the start of the addExternal function before the submitting state
check. Create a ref variable that tracks whether a request is currently in
flight and synchronously set it to true before making the API call, then set it
back to false in the finally block. This synchronous guard will reliably prevent
redundant addNative calls regardless of how quickly the user clicks.
In `@tests/userspace/conftest.py`:
- Around line 55-58: The code silently defaults uid to an empty string when
find_user("admin") fails to find the user, which can hide broken fixture setup
and produce invalid session state. After the find_user call that retrieves the
record, add an assertion to ensure the record exists and is not None before
extracting the id value. This will cause the fixture to fail fast and clearly
indicate that admin user creation or lookup failed, rather than proceeding with
an invalid empty-string user_id to the create_session call.
In `@tests/userspace/test_immutability.py`:
- Around line 7-9: The test_native_app_type_rejected function in this file is a
duplicate of an existing test in tests/userspace/test_package.py that covers the
same functionality. Remove the entire test_native_app_type_rejected function
from the current file to eliminate the duplicate test coverage and reduce
maintenance burden.
In `@tests/userspace/test_seed.py`:
- Line 171: The test assertion in the test_seed.py file is hardcoding the
version string "1.0.0" which creates maintenance burden when the actual version
in tinyagentos/userspace/seed/welcome/manifest.yaml is updated. Instead of
hardcoding the version value in the assertion, dynamically load the version from
the manifest.yaml file and use that loaded value in the comparison. This way,
the test will automatically use the current version from the manifest without
requiring manual updates whenever the version is bumped.
- Around line 79-89: The installed_at timestamp assertion is vulnerable to false
passes due to second-level time resolution granularity. Mock or patch the system
time in the test to return controlled, deterministic timestamps so that the
first and second seed operations would produce different installed_at values if
an unintended reseed actually occurred. Apply this time mocking to both the
initial seeding operation before the installed_at_first assertion and the
subsequent seed_bundled_apps call, ensuring they use different mocked timestamps
to make the idempotency check reliable.
In `@tests/userspace/test_update_consent.py`:
- Around line 21-25: The assertion for body["new_permissions"] is too loose and
only checks that "app.memory" exists within the set, which doesn't catch if
previously granted permissions are incorrectly re-flagged as new. Replace the
membership check with an exact assertion that verifies new_permissions equals
exactly the expected set of permissions, ensuring only the truly new permission
"app.memory" is included and no previously granted permissions are incorrectly
present.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2a7090f3-2ef0-4de5-a646-0f2944ef3b83
⛔ Files ignored due to path filters (1)
desktop/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (81)
.github/dependabot.ymlREADME.mddesktop/node_modulesdesktop/src/apps/AppStudioApp.test.tsxdesktop/src/apps/AppStudioApp.tsxdesktop/src/apps/CodingStudioApp.test.tsxdesktop/src/apps/CodingStudioApp.tsxdesktop/src/apps/DesignStudioApp.test.tsxdesktop/src/apps/DesignStudioApp.tsxdesktop/src/apps/MessagesApp.tsxdesktop/src/apps/MusicStudioApp.test.tsxdesktop/src/apps/MusicStudioApp.tsxdesktop/src/apps/OfficeSuiteApp.test.tsxdesktop/src/apps/OfficeSuiteApp.tsxdesktop/src/apps/ProjectsApp/AddAgentDialog.tsxdesktop/src/apps/ProjectsApp/ProjectMembers.tsxdesktop/src/apps/SandboxedAppWindow.tsxdesktop/src/apps/SettingsApp/UpdatesPanel.test.tsxdesktop/src/apps/SettingsApp/UpdatesPanel.tsxdesktop/src/apps/StoreApp/StudiosView.test.tsxdesktop/src/apps/StoreApp/StudiosView.tsxdesktop/src/apps/StoreApp/index.tsxdesktop/src/apps/StoreApp/types.tsdesktop/src/apps/StoreApp/updates-optional.test.tsxdesktop/src/apps/__tests__/SandboxedAppWindow.test.tsxdesktop/src/apps/appstudio/BuildView.tsxdesktop/src/apps/appstudio/PublishView.tsxdesktop/src/apps/appstudio/TemplatesView.tsxdesktop/src/apps/chat/ChannelSettingsPanel.tsxdesktop/src/apps/codingstudio/BuildView.tsxdesktop/src/apps/codingstudio/PreviewView.tsxdesktop/src/apps/codingstudio/TemplatesView.tsxdesktop/src/apps/designstudio/DesignView.tsxdesktop/src/apps/designstudio/MagicView.tsxdesktop/src/apps/designstudio/TemplatesView.tsxdesktop/src/apps/musicstudio/ComposeView.tsxdesktop/src/apps/musicstudio/SoundsView.tsxdesktop/src/apps/musicstudio/StudioView.tsxdesktop/src/apps/officesuite/CalcView.tsxdesktop/src/apps/officesuite/SlidesView.tsxdesktop/src/apps/officesuite/WriteView.tsxdesktop/src/components/Launchpad.tsxdesktop/src/hooks/use-installed-userspace-apps.tsdesktop/src/lib/__tests__/userspace-apps.test.tsdesktop/src/lib/userspace-apps.tsdesktop/src/registry/app-registry.tsdocs/STATUS.mdtests/test_apps_installed.pytests/test_docs_only_update.pytests/userspace/__init__.pytests/userspace/conftest.pytests/userspace/test_broker.pytests/userspace/test_broker_route.pytests/userspace/test_data_store.pytests/userspace/test_e2e.pytests/userspace/test_immutability.pytests/userspace/test_install_security.pytests/userspace/test_package.pytests/userspace/test_routes.pytests/userspace/test_sdk_route.pytests/userspace/test_seed.pytests/userspace/test_store.pytests/userspace/test_trust.pytests/userspace/test_update_consent.pytests/userspace/test_url_guard.pytinyagentos/app.pytinyagentos/auto_update.pytinyagentos/routes/__init__.pytinyagentos/routes/apps.pytinyagentos/routes/settings.pytinyagentos/routes/userspace_apps.pytinyagentos/userspace/__init__.pytinyagentos/userspace/broker.pytinyagentos/userspace/data_store.pytinyagentos/userspace/package.pytinyagentos/userspace/sdk/taos-app-sdk.jstinyagentos/userspace/seed.pytinyagentos/userspace/seed/welcome/index.htmltinyagentos/userspace/seed/welcome/manifest.yamltinyagentos/userspace/store.pytinyagentos/userspace/url_guard.py
| @@ -0,0 +1 @@ | |||
| /Volumes/NVMe/Users/jay/Development/tinyagentos/desktop/node_modules No newline at end of file | |||
There was a problem hiding this comment.
CRITICAL: Remove the host-specific symlink and gitignore node_modules instead.
This symlink points to an absolute macOS-specific path (/Volumes/NVMe/Users/jay/Development/...) and will break checkouts on any other machine, CI/CD pipeline, or Linux deployment. This issue is already documented in the PR objectives ("REPO-HYGIENE BUG: desktop/node_modules is a tracked gitlink ... gitignore it") and in STATUS.md LATEST-22, but it remains unresolved in this PR.
Action required:
- Remove
desktop/node_modulesfrom git tracking:git rm --cached desktop/node_modules - Ensure
desktop/node_modules/is in.gitignore - Verify the repository builds cleanly with
npm ciinstead of relying on the symlink
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@desktop/node_modules` at line 1, The desktop/node_modules directory is
currently a tracked symbolic link pointing to an absolute macOS-specific path
that will break the repository on other machines and CI/CD environments. Remove
this symlink from git tracking by using git rm --cached on the
desktop/node_modules entry, then add desktop/node_modules/ to the .gitignore
file to prevent it from being tracked in the future. After making these changes,
verify that the project builds successfully by running npm ci in the desktop
directory to ensure dependencies are properly installed without relying on the
symlink.
Comment on lines
+79
to
+93
| <div className="grid w-full max-w-[760px] grid-cols-3 gap-[14px]"> | ||
| {RESULTS.map(({ label, gradient }) => ( | ||
| <div | ||
| key={label} | ||
| className="relative cursor-pointer overflow-hidden rounded-[12px] border border-shell-border transition-all hover:-translate-y-[3px]" | ||
| style={{ aspectRatio: "0.8", background: gradient }} | ||
| > | ||
| <div | ||
| className="absolute bottom-0 left-0 right-0 px-[11px] py-[9px] text-[11px] text-white" | ||
| style={{ background: "linear-gradient(transparent, rgba(0,0,0,0.6))" }} | ||
| > | ||
| {label} | ||
| </div> | ||
| </div> | ||
| ))} |
There was a problem hiding this comment.
Use semantic buttons for selectable result tiles.
Line 81 renders interactive tiles as div, so they are not keyboard-activatable. This blocks keyboard navigation on the main “pick a result” action.
Suggested fix
- <div className="grid w-full max-w-[760px] grid-cols-3 gap-[14px]">
+ <div className="grid w-full max-w-[760px] grid-cols-3 gap-[14px]">
{RESULTS.map(({ label, gradient }) => (
- <div
+ <button
key={label}
+ type="button"
+ aria-label={`Use layout: ${label}`}
className="relative cursor-pointer overflow-hidden rounded-[12px] border border-shell-border transition-all hover:-translate-y-[3px]"
style={{ aspectRatio: "0.8", background: gradient }}
>
<div
className="absolute bottom-0 left-0 right-0 px-[11px] py-[9px] text-[11px] text-white"
style={{ background: "linear-gradient(transparent, rgba(0,0,0,0.6))" }}
>
{label}
</div>
- </div>
+ </button>
))}
</div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <div className="grid w-full max-w-[760px] grid-cols-3 gap-[14px]"> | |
| {RESULTS.map(({ label, gradient }) => ( | |
| <div | |
| key={label} | |
| className="relative cursor-pointer overflow-hidden rounded-[12px] border border-shell-border transition-all hover:-translate-y-[3px]" | |
| style={{ aspectRatio: "0.8", background: gradient }} | |
| > | |
| <div | |
| className="absolute bottom-0 left-0 right-0 px-[11px] py-[9px] text-[11px] text-white" | |
| style={{ background: "linear-gradient(transparent, rgba(0,0,0,0.6))" }} | |
| > | |
| {label} | |
| </div> | |
| </div> | |
| ))} | |
| <div className="grid w-full max-w-[760px] grid-cols-3 gap-[14px]"> | |
| {RESULTS.map(({ label, gradient }) => ( | |
| <button | |
| key={label} | |
| type="button" | |
| aria-label={`Use layout: ${label}`} | |
| className="relative cursor-pointer overflow-hidden rounded-[12px] border border-shell-border transition-all hover:-translate-y-[3px]" | |
| style={{ aspectRatio: "0.8", background: gradient }} | |
| > | |
| <div | |
| className="absolute bottom-0 left-0 right-0 px-[11px] py-[9px] text-[11px] text-white" | |
| style={{ background: "linear-gradient(transparent, rgba(0,0,0,0.6))" }} | |
| > | |
| {label} | |
| </div> | |
| </button> | |
| ))} | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@desktop/src/apps/designstudio/MagicView.tsx` around lines 79 - 93, The
interactive result tiles are currently rendered as div elements instead of
semantic button elements, making them inaccessible for keyboard navigation.
Replace the div element that wraps each result in the RESULTS.map function (the
one with key={label} and the className containing "cursor-pointer") with a
button element. Preserve all existing className and style attributes, and ensure
the button maintains the same visual appearance and interactivity as the current
div implementation.
Comment on lines
+30
to
+52
| <div | ||
| key={s.idx} | ||
| role="button" | ||
| tabIndex={0} | ||
| aria-label={`Slide ${s.idx}: ${s.label}`} | ||
| className="relative flex aspect-video cursor-pointer items-center justify-center overflow-hidden rounded-lg border px-1.5 text-center text-[10px] font-bold text-white" | ||
| style={{ | ||
| background: s.bg, | ||
| borderColor: | ||
| s.idx === 1 ? "var(--color-accent, #8b92a3)" : "rgba(255,255,255,0.08)", | ||
| outline: s.idx === 1 ? "2px solid #a9b0c2" : undefined, | ||
| outlineOffset: s.idx === 1 ? 1 : undefined, | ||
| }} | ||
| > | ||
| <span | ||
| className="absolute left-1.5 top-1 text-[8px]" | ||
| style={{ color: "rgba(255,255,255,0.7)" }} | ||
| > | ||
| {s.idx} | ||
| </span> | ||
| {s.label} | ||
| </div> | ||
| ))} |
There was a problem hiding this comment.
Replace div role="button" thumbnails with real <button> elements.
Lines 30-52 create interactive controls that are not fully keyboard-operable (no Enter/Space handling). Use native buttons for built-in keyboard and assistive-tech behavior.
Suggested fix
- {SLIDES.map((s) => (
- <div
+ {SLIDES.map((s) => (
+ <button
key={s.idx}
- role="button"
- tabIndex={0}
+ type="button"
aria-label={`Slide ${s.idx}: ${s.label}`}
className="relative flex aspect-video cursor-pointer items-center justify-center overflow-hidden rounded-lg border px-1.5 text-center text-[10px] font-bold text-white"
style={{
background: s.bg,
borderColor:
s.idx === 1 ? "var(--color-accent, `#8b92a3`)" : "rgba(255,255,255,0.08)",
outline: s.idx === 1 ? "2px solid `#a9b0c2`" : undefined,
outlineOffset: s.idx === 1 ? 1 : undefined,
}}
>
<span
className="absolute left-1.5 top-1 text-[8px]"
style={{ color: "rgba(255,255,255,0.7)" }}
>
{s.idx}
</span>
{s.label}
- </div>
+ </button>
))}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <div | |
| key={s.idx} | |
| role="button" | |
| tabIndex={0} | |
| aria-label={`Slide ${s.idx}: ${s.label}`} | |
| className="relative flex aspect-video cursor-pointer items-center justify-center overflow-hidden rounded-lg border px-1.5 text-center text-[10px] font-bold text-white" | |
| style={{ | |
| background: s.bg, | |
| borderColor: | |
| s.idx === 1 ? "var(--color-accent, #8b92a3)" : "rgba(255,255,255,0.08)", | |
| outline: s.idx === 1 ? "2px solid #a9b0c2" : undefined, | |
| outlineOffset: s.idx === 1 ? 1 : undefined, | |
| }} | |
| > | |
| <span | |
| className="absolute left-1.5 top-1 text-[8px]" | |
| style={{ color: "rgba(255,255,255,0.7)" }} | |
| > | |
| {s.idx} | |
| </span> | |
| {s.label} | |
| </div> | |
| ))} | |
| <button | |
| key={s.idx} | |
| type="button" | |
| aria-label={`Slide ${s.idx}: ${s.label}`} | |
| className="relative flex aspect-video cursor-pointer items-center justify-center overflow-hidden rounded-lg border px-1.5 text-center text-[10px] font-bold text-white" | |
| style={{ | |
| background: s.bg, | |
| borderColor: | |
| s.idx === 1 ? "var(--color-accent, `#8b92a3`)" : "rgba(255,255,255,0.08)", | |
| outline: s.idx === 1 ? "2px solid `#a9b0c2`" : undefined, | |
| outlineOffset: s.idx === 1 ? 1 : undefined, | |
| }} | |
| > | |
| <span | |
| className="absolute left-1.5 top-1 text-[8px]" | |
| style={{ color: "rgba(255,255,255,0.7)" }} | |
| > | |
| {s.idx} | |
| </span> | |
| {s.label} | |
| </button> | |
| ))} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@desktop/src/apps/officesuite/SlidesView.tsx` around lines 30 - 52, Replace
the div element that has role="button" with a native HTML button element. The
button should retain the key prop, tabIndex, aria-label, className, and style
attributes from the original div. Remove the role="button" attribute since it's
now redundant on a real button element. Keep the span child element that
displays the slide index and the s.label text content as-is. This will
automatically provide full keyboard support (Enter/Space handling) and better
accessibility without requiring manual event handling.
Comment on lines
+107
to
+143
| <input | ||
| type="checkbox" | ||
| checked={!!member.can_edit_canvas} | ||
| onChange={async (e) => { | ||
| await canvasApi.setPermission(projectId, member.member_id, e.target.checked); | ||
| onRefresh(); | ||
| onChanged(); | ||
| }} | ||
| /> | ||
| <span className="text-xs">Can edit canvas</span> | ||
| </label> | ||
| )} | ||
| {!isExternal && (member.member_kind === "native" || member.member_kind === "clone") && ( | ||
| <label | ||
| style={{ display: "inline-flex", alignItems: "center", gap: 6 }} | ||
| title="Lead agents see all messages in the project channel, even without being @mentioned." | ||
| > | ||
| <input | ||
| type="checkbox" | ||
| checked={!!member.is_lead} | ||
| aria-label={`Toggle lead for ${label}`} | ||
| onChange={async (e) => { | ||
| await projectsApi.members.setLead(projectId, member.member_id, e.target.checked); | ||
| onRefresh(); | ||
| onChanged(); | ||
| }} | ||
| /> | ||
| <span className="text-xs">Lead</span> | ||
| </label> | ||
| )} | ||
| <button | ||
| type="button" | ||
| onClick={async () => { | ||
| await projectsApi.members.remove(projectId, member.member_id); | ||
| onRefresh(); | ||
| onChanged(); | ||
| }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify thrown API behavior + async handlers without local catches.
rg -nC3 'async function http|if \(!r.ok\)|throw new Error' desktop/src/lib/projects.ts
rg -nC2 'onChange=\{async|onClick=\{async' desktop/src/apps/ProjectsApp/ProjectMembers.tsxRepository: jaylfc/taOS
Length of output: 1245
Add error handling and busy state to member action handlers.
The three async handlers at lines 111, 129, and 140 await API calls that throw on non-OK responses without catching errors. Unhandled rejections can leave the UI inconsistent and provide no feedback to the user. Additionally, rapid clicks can trigger duplicate requests.
Suggested fix
function MemberRow({
@@
}) {
+ const [rowBusy, setRowBusy] = useState(false);
+ const runMemberAction = async (op: () => Promise<unknown>) => {
+ if (rowBusy) return;
+ setRowBusy(true);
+ try {
+ await op();
+ onRefresh();
+ onChanged();
+ } finally {
+ setRowBusy(false);
+ }
+ };
+
return (
@@
<input
type="checkbox"
checked={!!member.can_edit_canvas}
- onChange={async (e) => {
- await canvasApi.setPermission(projectId, member.member_id, e.target.checked);
- onRefresh();
- onChanged();
- }}
+ disabled={rowBusy}
+ onChange={(e) =>
+ void runMemberAction(() =>
+ canvasApi.setPermission(projectId, member.member_id, e.target.checked),
+ )
+ }
/>
@@
<input
type="checkbox"
checked={!!member.is_lead}
+ disabled={rowBusy}
aria-label={`Toggle lead for ${label}`}
- onChange={async (e) => {
- await projectsApi.members.setLead(projectId, member.member_id, e.target.checked);
- onRefresh();
- onChanged();
- }}
+ onChange={(e) =>
+ void runMemberAction(() =>
+ projectsApi.members.setLead(projectId, member.member_id, e.target.checked),
+ )
+ }
/>
@@
<button
type="button"
- onClick={async () => {
- await projectsApi.members.remove(projectId, member.member_id);
- onRefresh();
- onChanged();
- }}
+ disabled={rowBusy}
+ onClick={() =>
+ void runMemberAction(() =>
+ projectsApi.members.remove(projectId, member.member_id),
+ )
+ }
className="text-xs text-red-400 hover:underline"
aria-label={`Remove ${label}`}
>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@desktop/src/apps/ProjectsApp/ProjectMembers.tsx` around lines 107 - 143, Add
error handling and busy state management to the three async handlers in the
member permission UI. For the "Can edit canvas" checkbox onChange handler
(calling canvasApi.setPermission), the "Lead" checkbox onChange handler (calling
projectsApi.members.setLead), and the remove button onClick handler (calling
projectsApi.members.remove), wrap each API call in try-catch blocks to handle
errors gracefully, add a loading/busy state variable to disable the input
elements during the API call, and display error feedback to the user when the
API call fails so the UI remains consistent and prevents duplicate requests from
rapid clicks.
| import { useState, useEffect, useCallback, useRef } from "react"; | ||
| import { Settings, RefreshCw, AlertCircle, Check, ChevronDown, ChevronRight } from "lucide-react"; | ||
| import { Settings, RefreshCw, AlertCircle, Check, ChevronDown, ChevronRight, Package } from "lucide-react"; | ||
| import * as LucideIcons from "lucide-react"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n desktop/src/apps/SettingsApp/UpdatesPanel.tsx | head -60Repository: jaylfc/taOS
Length of output: 2205
🏁 Script executed:
# Check the rest of the file for AppIconGlyph usage and see what icon names are passed
cat -n desktop/src/apps/SettingsApp/UpdatesPanel.tsx | tail -n +50Repository: jaylfc/taOS
Length of output: 23201
🏁 Script executed:
# Find the app registry file
fd -t f "app-registry" desktop/src/registry/Repository: jaylfc/taOS
Length of output: 134
🏁 Script executed:
cat -n desktop/src/registry/app-registry.tsRepository: jaylfc/taOS
Length of output: 20544
🏁 Script executed:
# Check if LucideIcons is used anywhere else in the file
rg "LucideIcons" desktop/src/apps/SettingsApp/UpdatesPanel.tsxRepository: jaylfc/taOS
Length of output: 204
🏁 Script executed:
# Check if there are other files that use the same pattern (namespace import + dynamic lookup)
rg "import \* as.*from.*lucide-react" desktop/src --type ts --type tsxRepository: jaylfc/taOS
Length of output: 83
🏁 Script executed:
# Check if there are other files that use the same pattern (namespace import + dynamic lookup)
rg "import \* as.*from.*lucide-react" desktop/srcRepository: jaylfc/taOS
Length of output: 725
🏁 Script executed:
# Check how other files use their namespace imports - looking for dynamic lookup pattern
rg "icons\[.*\]" desktop/src/components/LaunchpadIcon.tsx -A 2 -B 2Repository: jaylfc/taOS
Length of output: 243
Replace namespace-loaded Lucide icons with explicit optional-app icon map.
Lines 3 and 41–49 use import * as LucideIcons with dynamic pascal-case lookup, forcing the entire icon library into the bundle even though only a fixed set of optional-app icons are ever needed. Replace with explicit named imports of the icons actually used by optional apps and a simple lookup map.
Proposed change
-import { Settings, RefreshCw, AlertCircle, Check, ChevronDown, ChevronRight, Package } from "lucide-react";
-import * as LucideIcons from "lucide-react";
+import {
+ Settings, RefreshCw, AlertCircle, Check, ChevronDown, ChevronRight, Package,
+ Code2, Palette, Music, Blocks, FileText, ScrollText, PlayCircle, Github, AtSign, Globe,
+ type LucideIcon,
+} from "lucide-react";
+
+const OPTIONAL_ICON_MAP: Record<string, LucideIcon> = {
+ "code-2": Code2,
+ palette: Palette,
+ music: Music,
+ blocks: Blocks,
+ "file-text": FileText,
+ "scroll-text": ScrollText,
+ "play-circle": PlayCircle,
+ github: Github,
+ "at-sign": AtSign,
+ globe: Globe,
+};
function AppIconGlyph({ iconName, size = 16 }: { iconName: string; size?: number }) {
- const pascal = iconName
- .split("-")
- .map((s) => s.charAt(0).toUpperCase() + s.slice(1))
- .join("");
- const Glyph = (LucideIcons[pascal as keyof typeof LucideIcons] as LucideIcons.LucideIcon) ?? Package;
+ const Glyph = OPTIONAL_ICON_MAP[iconName] ?? Package;
return <Glyph size={size} />;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@desktop/src/apps/SettingsApp/UpdatesPanel.tsx` at line 3, The namespace
import of `lucide-react` in UpdatesPanel.tsx forces the entire icon library into
the bundle even though only specific optional-app icons are used. Replace the
`import * as LucideIcons` statement with explicit named imports of only the
icons actually referenced in the optional-app icon lookup logic (around lines
41-49), then create a simple object map that maps icon names to the imported
icon components, eliminating the need for dynamic pascal-case lookup via
LucideIcons[].
| data = json.loads(row_json) | ||
| if where and any(data.get(k) != v for k, v in where.items()): | ||
| continue | ||
| out.append({"id": rid, **data}) |
There was a problem hiding this comment.
Prevent row payloads from overriding the authoritative database row id.
data can contain an "id" key and overwrite rid in the returned object, which breaks the table API contract for row identity.
Suggested fix
- out.append({"id": rid, **data})
+ out.append({**data, "id": rid})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tinyagentos/userspace/data_store.py` at line 72, The dictionary construction
in the append statement for the out list uses the wrong order of precedence,
allowing the "id" key from the data dictionary to override the authoritative rid
value. In the line where out.append is called with a dictionary containing "id":
rid and **data unpacked, reverse the order of precedence by placing the
authoritative rid assignment after the data unpacking so that {**data, "id":
rid} ensures rid always takes precedence over any "id" key that might exist in
the data dictionary.
Comment on lines
+12
to
+19
| window.addEventListener("message", (e) => { | ||
| const m = e.data; | ||
| // Broker replies | ||
| if (m && m.taosAppReply != null && pending.has(m.taosAppReply)) { | ||
| const { resolve } = pending.get(m.taosAppReply); | ||
| pending.delete(m.taosAppReply); | ||
| resolve(m); | ||
| } |
There was a problem hiding this comment.
Validate message source before resolving RPC replies.
On Line 15, reply messages are accepted from any window. A different frame can spoof { taosAppReply: id } and resolve in-flight calls with attacker-controlled payloads.
Proposed fix
- window.addEventListener("message", (e) => {
+ window.addEventListener("message", (e) => {
+ if (e.source !== parent) return;
const m = e.data;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| window.addEventListener("message", (e) => { | |
| const m = e.data; | |
| // Broker replies | |
| if (m && m.taosAppReply != null && pending.has(m.taosAppReply)) { | |
| const { resolve } = pending.get(m.taosAppReply); | |
| pending.delete(m.taosAppReply); | |
| resolve(m); | |
| } | |
| window.addEventListener("message", (e) => { | |
| if (e.source !== parent) return; | |
| const m = e.data; | |
| // Broker replies | |
| if (m && m.taosAppReply != null && pending.has(m.taosAppReply)) { | |
| const { resolve } = pending.get(m.taosAppReply); | |
| pending.delete(m.taosAppReply); | |
| resolve(m); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tinyagentos/userspace/sdk/taos-app-sdk.js` around lines 12 - 19, The message
event listener in the anonymous callback does not validate the origin of
incoming messages before resolving pending RPC calls. Add an origin validation
check on the event object (e.origin) before processing the message and resolving
the promise. Only proceed with resolving the pending call when the message
originates from a trusted/expected origin, preventing spoofed messages from
different frames from hijacking in-flight RPC calls.
Comment on lines
+29
to
+34
| function call(capability, args) { | ||
| const id = ++seq; | ||
| return new Promise((resolve) => { | ||
| pending.set(id, { resolve }); | ||
| parent.postMessage({ taosApp: APP_ID, id, capability, args: args || {} }, "*"); | ||
| }); |
There was a problem hiding this comment.
Add timeout/reject cleanup for pending RPC calls.
On Lines 31-33, unresolved calls stay in pending forever if no broker response arrives. This leaks memory and leaves callers hanging.
Proposed fix
- function call(capability, args) {
+ function call(capability, args) {
const id = ++seq;
- return new Promise((resolve) => {
- pending.set(id, { resolve });
+ return new Promise((resolve, reject) => {
+ const timer = setTimeout(() => {
+ pending.delete(id);
+ reject(new Error(`taos_timeout:${capability}`));
+ }, 15000);
+ pending.set(id, {
+ resolve: (msg) => {
+ clearTimeout(timer);
+ resolve(msg);
+ },
+ });
parent.postMessage({ taosApp: APP_ID, id, capability, args: args || {} }, "*");
});
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tinyagentos/userspace/sdk/taos-app-sdk.js` around lines 29 - 34, The call
function creates pending promises that are stored in the pending map but have no
timeout mechanism, causing memory leaks if broker responses never arrive. Add a
timeout handler within the call function that rejects the promise and removes
the entry from the pending map after a reasonable duration (such as 30 seconds).
Use setTimeout to trigger this cleanup, and ensure the timeout is cleared if the
response arrives before the timeout expires by storing the timeout ID alongside
the resolve function in the pending map entry.
Comment on lines
+40
to
+67
| get: (k) => call("app.kv.get", { key: k }).then((r) => r.result), | ||
| set: (k, v) => call("app.kv.set", { key: k, value: v }), | ||
| delete: (k) => call("app.kv.delete", { key: k }), | ||
| keys: () => call("app.kv.keys", {}).then((r) => r.result), | ||
| }, | ||
| table: { | ||
| insert: (t, row) => call("app.table.insert", { table: t, row }).then((r) => r.result), | ||
| query: (t, where) => call("app.table.query", { table: t, where }).then((r) => r.result), | ||
| delete: (t, id) => call("app.table.delete", { table: t, id }), | ||
| }, | ||
| files: { | ||
| read: (p) => call("app.files.read", { path: p }).then((r) => r.result), | ||
| write: (p, content) => call("app.files.write", { path: p, content }), | ||
| }, | ||
| notify: (title, body) => call("app.notify", { title, body }), | ||
| // gated -- resolve to {error:"permission_denied"} if not granted | ||
| net: { fetch: (url, opts) => call("app.net", { path: url, method: (opts && opts.method) || "GET", body: opts && opts.body, headers: opts && opts.headers }) }, | ||
| backend: { | ||
| fetch: (path, opts) => call("app.net", { | ||
| path, | ||
| method: (opts && opts.method) || "GET", | ||
| body: opts && opts.body, | ||
| headers: opts && opts.headers, | ||
| }).then((r) => r.result), | ||
| }, | ||
| agent: { ask: (name, message) => call("app.agent", { name, message }).then((r) => r.result) }, | ||
| memory: { search: (q) => call("app.memory.search", { q }).then((r) => r.result) }, | ||
| // Theme API -- populated only for first-party apps that receive taosTheme |
There was a problem hiding this comment.
Normalize broker error handling across SDK methods.
On Lines 40-67, some methods return r.result, others return raw envelopes. For gated capabilities (e.g., agent.ask, memory.search), { error: "permission_denied" } currently becomes undefined in callers, which hides permission failures.
Proposed fix
+ function unwrap(r) {
+ if (r && r.error) throw new Error(String(r.error));
+ return r ? r.result : undefined;
+ }
+
window.taos = {
@@
- get: (k) => call("app.kv.get", { key: k }).then((r) => r.result),
- set: (k, v) => call("app.kv.set", { key: k, value: v }),
- delete: (k) => call("app.kv.delete", { key: k }),
- keys: () => call("app.kv.keys", {}).then((r) => r.result),
+ get: (k) => call("app.kv.get", { key: k }).then(unwrap),
+ set: (k, v) => call("app.kv.set", { key: k, value: v }).then(unwrap),
+ delete: (k) => call("app.kv.delete", { key: k }).then(unwrap),
+ keys: () => call("app.kv.keys", {}).then(unwrap),
@@
- insert: (t, row) => call("app.table.insert", { table: t, row }).then((r) => r.result),
- query: (t, where) => call("app.table.query", { table: t, where }).then((r) => r.result),
- delete: (t, id) => call("app.table.delete", { table: t, id }),
+ insert: (t, row) => call("app.table.insert", { table: t, row }).then(unwrap),
+ query: (t, where) => call("app.table.query", { table: t, where }).then(unwrap),
+ delete: (t, id) => call("app.table.delete", { table: t, id }).then(unwrap),
@@
- read: (p) => call("app.files.read", { path: p }).then((r) => r.result),
- write: (p, content) => call("app.files.write", { path: p, content }),
+ read: (p) => call("app.files.read", { path: p }).then(unwrap),
+ write: (p, content) => call("app.files.write", { path: p, content }).then(unwrap),
@@
- notify: (title, body) => call("app.notify", { title, body }),
+ notify: (title, body) => call("app.notify", { title, body }).then(unwrap),
@@
- net: { fetch: (url, opts) => call("app.net", { path: url, method: (opts && opts.method) || "GET", body: opts && opts.body, headers: opts && opts.headers }) },
+ net: { fetch: (url, opts) => call("app.net", { path: url, method: (opts && opts.method) || "GET", body: opts && opts.body, headers: opts && opts.headers }).then(unwrap) },
@@
- }).then((r) => r.result),
+ }).then(unwrap),
@@
- agent: { ask: (name, message) => call("app.agent", { name, message }).then((r) => r.result) },
- memory: { search: (q) => call("app.memory.search", { q }).then((r) => r.result) },
+ agent: { ask: (name, message) => call("app.agent", { name, message }).then(unwrap) },
+ memory: { search: (q) => call("app.memory.search", { q }).then(unwrap) },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tinyagentos/userspace/sdk/taos-app-sdk.js` around lines 40 - 67, The gated
capability methods like agent.ask and memory.search currently extract and return
only r.result, which causes permission_denied error responses to become
undefined or hidden from callers. Normalize error handling by modifying the
gated capability methods (agent.ask at line 62 and memory.search at line 63) to
return the full response envelope instead of extracting r.result, ensuring that
error objects like {error: "permission_denied"} are properly visible to callers.
This should be consistent with how other methods handle responses to provide
uniform error detection across all SDK methods.
Comment on lines
+68
to
+73
| # Re-seed (new app, version bump, or a non-first-party row claiming | ||
| # this id): remove any previously extracted files first so a smaller | ||
| # new version cannot inherit stale files from the old one, then extract. | ||
| shutil.rmtree(apps_root / app_id, ignore_errors=True) | ||
| zip_bytes = _build_zip_from_dir(app_dir) | ||
| extract_package(zip_bytes, apps_root) |
There was a problem hiding this comment.
Guard app_id path before rmtree to prevent out-of-root deletion.
extract_package() validates path safety, but it runs after deletion. A malformed manifest id can make shutil.rmtree(apps_root / app_id, ...) target outside apps_root.
Proposed fix
- shutil.rmtree(apps_root / app_id, ignore_errors=True)
+ apps_root_resolved = Path(apps_root).resolve()
+ target_dir = (apps_root_resolved / app_id).resolve()
+ if target_dir == apps_root_resolved or not target_dir.is_relative_to(apps_root_resolved):
+ raise ValueError(f"unsafe bundled app id: {app_id!r}")
+ shutil.rmtree(target_dir, ignore_errors=True)
zip_bytes = _build_zip_from_dir(app_dir)
extract_package(zip_bytes, apps_root)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tinyagentos/userspace/seed.py` around lines 68 - 73, The shutil.rmtree call
in the re-seed block uses app_id to construct the path without first validating
that it stays within apps_root, creating a security vulnerability where a
malicious manifest id could delete files outside the intended directory. Add
path validation before the shutil.rmtree call (before line 71) to ensure the
resolved path stays within apps_root, similar to how extract_package validates
paths. Check that resolving apps_root / app_id results in a path that is within
apps_root and reject or sanitize app_id values that would escape the directory.
jaylfc
added a commit
that referenced
this pull request
Jun 17, 2026
…nomous build team + gate-cron running
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 40 commits of dev to master. Highlights: userspace app system surfaced in the Launchpad (#974) + per-app updates; the multi-agent build collaboration tooling; chat/projects fixes shipped via the autonomous build team (channel settings overlap #975, channel topic #976, channel-create listing #977, external/connected agents members section #978); README Discord community badge; dependabot Python deps tracked via the uv ecosystem. All landed on dev gated by CI + CodeRabbit + Gitar + Kilo.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation