feat(userspace): first-party reference .taosapp + boot-seeding (#89 P4a)#973
Conversation
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.
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? |
|
Warning Review limit reached
More reviews will be available in 1 minute and 56 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds ChangesUserspace boot-seeding
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 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}
| except (PackageError, Exception): | ||
| logger.warning("failed to seed bundled app in %s", app_dir, exc_info=True) |
There was a problem hiding this comment.
💡 Quality: Redundant exception tuple catches everything via Exception
except (PackageError, Exception): is redundant — PackageError subclasses Exception, so the tuple is equivalent to except Exception:. Listing PackageError first is misleading (it suggests differentiated handling that doesn't exist) and linters (e.g. flake8/pylint) flag the unreachable-by-subset clause. The broad catch-all is intentional here (per-app isolation so one bad seed only warns), so simplify to a plain except Exception: to make the intent explicit.
Drop the redundant PackageError from the tuple.:
except Exception:
logger.warning("failed to seed bundled app in %s", app_dir, exc_info=True)
Was this helpful? React with 👍 / 👎
| zip_bytes = _build_zip_from_dir(app_dir) | ||
| extract_package(zip_bytes, apps_root) |
There was a problem hiding this comment.
💡 Edge Case: Version-bump re-seed leaves stale files from old version
On a version bump, extract_package is called again for the same app and uses app_dir.mkdir(parents=True, exist_ok=True) then writes the new zip's files on top of the existing directory. Files that existed in the previous bundled version but were removed in the new version are never deleted, so stale assets accumulate in the app directory across version bumps. For the current single-file welcome app this is harmless, but as reference apps grow this can serve dead/obsolete files. Consider clearing the destination directory before extracting a new version (or extracting to a fresh temp dir and swapping) so the on-disk bundle exactly matches the new package.
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 👍 Approved with suggestions 0 resolved / 2 findingsImplements bundled app seeding and the reference .taosapp package structure to validate first-party installation flows. Simplify the redundant catch-all exception tuple and ensure stale files are cleaned up during version-bump re-seeding. 💡 Quality: Redundant exception tuple catches everything via Exception📄 tinyagentos/userspace/seed.py:78-79
Drop the redundant PackageError from the tuple.💡 Edge Case: Version-bump re-seed leaves stale files from old version📄 tinyagentos/userspace/seed.py:65-66 On a version bump, 🤖 Prompt for agentsOptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/userspace/test_seed.py (1)
69-94: ⚡ Quick winAdd a regression test for same-version trust upgrade.
Current coverage checks same-version idempotency, but not the case where an app already exists at that version with non-
first-partytrust. Add a case to ensure seeding upgrades trust instead of skipping.Suggested test addition
+@pytest.mark.asyncio +async def test_seed_upgrades_same_version_to_first_party(tmp_path): + seed_dir = tmp_path / "seed" + apps_root = tmp_path / "apps" + _write_app(seed_dir, "my-app", version="1.0.0") + store = await _make_store(tmp_path) + + await store.install( + app_id="my-app", + name="Test App", + version="1.0.0", + app_type="web", + entry="index.html", + icon="", + permissions_requested=["app.kv"], + trust="community", + ) + + await seed_bundled_apps(store, apps_root, seed_dir) + row = await store.get("my-app") + assert row is not None + assert row["trust"] == "first-party" + await store.close()🤖 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 69 - 94, Add a new async test function to verify that seeding an app at the same version upgrades its trust level instead of skipping the update. Create a test that first seeds an app with a non-first-party trust value (such as "third-party"), then calls seed_bundled_apps again with the same version, and verifies that the trust is upgraded to "first-party" while the installed_at timestamp remains unchanged. This test should follow the same pattern as test_seed_idempotent_same_version but specifically validate the trust upgrade behavior by checking the trust value before and after the second seed operation.
🤖 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 `@tinyagentos/userspace/seed.py`:
- Around line 61-63: The idempotency check on line 61 only validates that the
version matches but does not verify the trust level of the existing app. Modify
the condition that checks "if existing is not None and existing.get("version")
== version" to also require that the existing entry has first-party trust by
adding an additional check for the trust attribute/field. This ensures that if
an app exists at the same version but with non-first-party trust, the seeding
will not be skipped and the trust will be corrected to the trusted path.
---
Nitpick comments:
In `@tests/userspace/test_seed.py`:
- Around line 69-94: Add a new async test function to verify that seeding an app
at the same version upgrades its trust level instead of skipping the update.
Create a test that first seeds an app with a non-first-party trust value (such
as "third-party"), then calls seed_bundled_apps again with the same version, and
verifies that the trust is upgraded to "first-party" while the installed_at
timestamp remains unchanged. This test should follow the same pattern as
test_seed_idempotent_same_version but specifically validate the trust upgrade
behavior by checking the trust value before and after the second seed operation.
🪄 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: 0899cc43-eeab-4827-8291-4a198479cbd4
📒 Files selected for processing (5)
tests/userspace/test_seed.pytinyagentos/app.pytinyagentos/userspace/seed.pytinyagentos/userspace/seed/welcome/index.htmltinyagentos/userspace/seed/welcome/manifest.yaml
…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
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| # 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) |
There was a problem hiding this comment.
CRITICAL: Validate app_id before deleting the target bundle directory
app_id comes from the seed manifest and is used directly in shutil.rmtree(apps_root / app_id). A manifest declaring an absolute or traversal id can delete outside apps_root before extract_package has a chance to reject the unsafe path. This also deletes the existing bundle before extraction succeeds, so a bad package can leave the app broken.
Validate/resolve the target path before deletion, and prefer extracting to a temporary directory and swapping only after success.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
…st+package+seeding all on dev, holding for Jay
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
Other Observations (not in diff)Issues found in unchanged code that cannot receive inline comments:
Files Reviewed (5 files)
Fix these issues in Kilo Cloud Reviewed by nex-n2-pro:free · 1,592,032 tokens |
1 participant
Phase P4a (backend) of the app system (#89): a minimal first-party reference .taosapp plus boot-seeding, proving the package + trusted-install loop end to end. Additive and backend-only.
What lands
Scope boundary
Backend only. Surfacing userspace apps in the desktop launcher (the high-blast-radius UI merge) is a separate, design-sensitive step handled by the lead. This PR proves the package format + the first-party seed/runtime path; the welcome app installs first-party at boot and is served with the first-party CSP.
Tests
8 new (tests/userspace/test_seed.py): first-party seed, idempotency, version-bump re-seed, missing-dir tolerance, first-party CSP on the served bundle, and the real bundled welcome app. Full userspace suite 68 pass; create_app OK.
Part of #89 (P4).
Summary by CodeRabbit
New Features
Tests