Conversation
…aged systemd service (#1755) Audit of how backends run on the Pi found the production rkllama was a hand-started bare process (PPID 1, no unit), never adopted by systemd, so it had no boot persistence and the Activity 'Restart AI Services' recovery (routes/system.py) could not restart it. Root cause: the store wrapper short-circuited with exit 0 the moment any process answered /api/tags on 7833/8080, before delegating to install-rknpu.sh, so a hand-start permanently blocked the systemd unit from ever being created. taOS manages backends as systemd services (boot persistence + recovery + cluster placement/migration all depend on it), so the idempotent short-circuit now also requires rkllama.service to be enabled. If it is not, we fall through to install-rknpu.sh, whose install_systemd_unit adopts the running instance under systemd (ExecStartPre reaps the orphan so the unit can bind the port). Also corrects the routes/system.py comment: rkllama is a system unit (per install-rknpu.sh), not a user unit.
* docs(design): cluster-aware backend service management spec Phase 1 (build now): mandatory managed-service manifest contract + CI lint, node-local backend service manager in the worker agent (ensure/self-heal/ start/stop/restart/health), #1743 rewired onto it, and a Backend Services surface in the Cluster app. Phase 2 (spec'd, deferred): controller cross-node reconciler + adaptive host-backend migration. Grounded in the backend-lifecycle audit (qmd managed, rkllama hand-started; #1755 store-wrapper trap fix). * docs(design): correct contract to extend existing lifecycle block (not a new managed block)
…hase 1.1) (#1756) * feat(catalog): managed-backend-service manifest contract + CI lint Phase 1, unit 1 of the cluster-aware backend service management design (docs/design/cluster-backend-service-management.md). A service manifest whose lifecycle.auto_manage is true must now declare lifecycle.unit (systemd unit), lifecycle.scope (system|user), and lifecycle.health (url + expect), so taOS can manage the backend as a systemd service uniformly across cluster nodes. Enforced by a new scripts/check_manifests.py managed-lint wired into the doc-gate workflow (the existing audit-manifests.py is a one-off migration checker, not in CI). Normalizes rkllama (the only auto_manage:true manifest today) to declare rkllama.service / system / api-tags health. auto_manage:false backends (rk-llama-cpp, llama-cpp, mlc-llm, openllm) are grandfathered until a later slice flips them on with proper fields. 8 lint tests incl. the real catalog. * fix(manifest-lint): harden auto_manage/YAML checks (fold Kilo review on #1756) - auto_manage truthy-but-not-bool (quoted "true", 1) is now flagged instead of silently bypassing the contract via an identity check. - an unparseable or non-mapping manifest.yaml is now an error, not a silent skip that read as clean. - validate lifecycle.unit ends in .service and lifecycle.health.expect is a string (documented as a substring match on the health body). 4 new tests; 12 total, real catalog clean.
…1758) Additive module tinyagentos/cluster/backend_services.py for the cluster-aware backend service management design. Reusable, node-agnostic helpers: - load_managed_backends(catalog_root): the managed backends declared in manifests (lifecycle.auto_manage + unit/scope/health). - resolve_scope / unit_state: user-vs-system scope + installed/enabled/active. - service_action(unit, verb): fail-soft systemctl start/stop/restart, promoted and generalised from routes/system.py::_restart_ai_unit (kill+reap on timeout, 'not installed' when no unit). - health_probe(url, expect): httpx GET, substring assertion on the 200 body. No existing behaviour changes; the #1743 recovery endpoint (unit 3) and the worker-agent backend endpoints (unit 2b) will build on these. 12 tests (mocked systemctl + httpx).
…ontract (Phase 1.3) (#1760) Rewire the AI-stack recovery endpoint (POST /api/system/ai-stack/restart) onto the node-local backend service manager (cluster/backend_services.py, #1758) instead of the hardcoded AI_STACK_UNITS list and the private _restart_ai_unit/_systemctl_rc helpers it duplicated. The restart target set is now derived per node: core-managed units that have no store manifest (qmd, installed by install-server.sh) unioned with the store-declared managed backends (lifecycle.auto_manage in their catalog manifest, e.g. rkllama), then narrowed to units whose systemd file actually exists on this node. A backend that has migrated to another cluster node is skipped rather than reported as a spurious failure, and a newly installed managed backend joins recovery with no code change. Membership uses systemctl cat (via unit_state) so a dead/crashed unit still counts. A node with no managed backend installed now returns status "noop" instead of a misleading "failed". A broken catalog still yields the core units, so qmd never drops out of recovery. _restart_ai_unit / _systemctl_rc scope-resolution + kill-reap logic already moved to backend_services in #1758 and is tested there; the endpoint tests now stub the derivation + service_action, and a new TestManagedAiUnits covers the core+manifest union and the installed-on-this-node filter. Docs-Reviewed: no route added or removed; endpoint path, method, auth gate, and response shape are unchanged (internal target-derivation refactor only).
… fix, #1706) (#1725) * feat(models): atomic VRAM check-and-reserve before model load (TOCTOU fix) Adds VramReservationManager — an asyncio.Lock-protected VRAM reservation system that atomically checks free VRAM (via nvidia-smi) and reserves the required amount before a model load begins. Concurrent callers see the in-flight reservation and back off, preventing the TOCTOU race where two model pulls both see enough free VRAM and both try to load, causing OOM. Integration points: - POST /api/models/pull — reserve before ollama pull, release after - POST /api/models/download (rkllama path) — reserve before installer, release in the background task's finally block - GET /api/models/vram-reservations — stats endpoint for monitoring New files: - tinyagentos/vram_reservation.py — atomic reserve/release manager - tests/test_vram_reservation.py — 15 tests including concurrent safety Wired as app.state.vram_reservation in the FastAPI lifespan alongside the cluster manager and resource scheduler. Issue: #1706 * docs: add TODO for per-model VRAM estimate replacement jaylfc review: min_ram_mb heuristic is safe for v1 (over-reserves, fails closed), but should be replaced with a real per-model VRAM estimate on CUDA GGUF to avoid false 503s in multi-model setups. --------- Co-authored-by: Hogne <227774406+hognek@users.noreply.github.com>
…#1762) * feat(cluster): advertise available models from Skald sidecar manifest Add skald_manifest.py — parses /etc/taos/skald-models.json (env-configurable) to enumerate which models a worker CAN load per the GPU catalog. - WorkerInfo gains available_models list[dict] field (defaults to []) - ClusterManager.heartbeat() derives available_models from backend entries, deduplicating model_ids across backends - WorkerAgent.detect_backends() enriches each backend with available models from the sidecar manifest, tagging each as 'loaded' or 'available' based on live probe results Tests: 42 pass — 14 parser tests, 5 field serialization tests, 5 heartbeat derivation tests, 18 existing protocol tests (0 regressions). Also verified zero regressions against 32 existing worker tests. Completes end-to-end wiring for Skald PR #361 — the controller's cluster view now shows both loaded and available models per worker. * de-skald worker manifest: rename skald_manifest → worker_manifest - Rename tinyagentos/worker/skald_manifest.py → worker_manifest.py - Default manifest path: /etc/taos/skald-models.json → /etc/taos/worker-models.json - Env var: TAOS_SKALD_MANIFEST → TAOS_WORKER_MANIFEST - Remove all Skald-specific language from docstrings, comments, and field labels - Update import in agent.py - Rename test file, update all references, 14/14 pass - Keep available_models field on WorkerInfo as generic (unchanged) Core is now fork-agnostic: any external platform can write the manifest. Skald-specific logic belongs in Skald's codebase. --------- Co-authored-by: Hogne <227774406+hognek@users.noreply.github.com>
…ches (#1764) Re-pin TAOS_RKLLAMA_REF from 58038c95 (rknn-llm 1.2.3) to dadea413 (feat/rknpu-1.3.0: upstream 1.3.0 base + the 6 taOS fork patches + the rerank-ABI adaptation for the 1.3.0 options element). This ref was deployed and smoke-verified on the RK3588 production Pi on 2026-07-09, so fresh installs now get 1.3.0 for latest-model support instead of 1.2.3. Also add a static patch-presence guard right after checkout (issue #1730): a past ref bump silently dropped --preload and broke the preloaded embed/rerank/expand models. The installer now refuses to build against any ref whose source is missing --preload or the context-overflow payload, and fails fast before the venv install and service start. Docs-Reviewed: internal dependency-ref bump plus a defensive guard; no change to documented install steps, flags, or user-facing behaviour.
…real min_ram_mb key (#1767) Fixes the urgent findings from the full code audit of the recent merges (follow-ups #1765/#1766 track the deeper redesign parts): - worker_manifest.load_manifest: a malformed or non-object manifest file is external input and now degrades to the empty manifest with a logged warning instead of raising. Previously one typo'd comma in /etc/taos/worker-models.json crash-looped the whole worker under systemd. - worker/agent.py enrichment: skip entries without model_id (logged) and wrap the whole enrichment in a fail-soft try/except so no manifest problem can take detect_backends down. Heartbeat send failures are now logged before being swallowed so a payload-build bug no longer masquerades as controller unreachability. - vram_reservation: the probe now returns None (cannot know) distinct from (0, 0) (known-full), and reserve() fails OPEN with bookkeeping-only admission when no probe exists, matching cluster claim_lease semantics. Previously every reservation was denied 503 on AMD/Apple/Rockchip, i.e. on exactly the rkllama hardware the guard was written for. The probe also moved to asyncio.to_thread so nvidia-smi no longer blocks the event loop while holding the reservation lock. stats() gains probe_available. - routes/models.py: the rkllama pull gate now reads the real backend-level requirement (max of variant.requires.backends[].min_ram_mb, falling back to the variant key). Every rkllm variant has variant-level min_ram_mb 0, so the previous read made the TOCTOU gate dead code against the entire catalog. - cluster/backend_services.load_managed_backends: skipped manifests are now logged so a device-side catalog edit cannot silently drop a backend out of #1743 recovery. - install-rknpu.sh: replace the stale copy-of-truth comment (old port 8080 and pre-consolidation models path) with an accurate one. Tests: the two tests that blessed the crash now assert the degrade behaviour, plus new coverage for non-object manifests, no-probe fail-open, probe-unavailable stats, and a regression guard that a real probe showing insufficiency still denies. Docs-Reviewed: behaviour hardening within existing features; no documented install step, endpoint path, or response shape changed (installer diff is a comment correction only).
…o spoofing) (#1768) * feat(a2a): authenticated bus send proxy so agents post as themselves The controller's A2A bus proxy was read-only; the only send path was the raw :7900 bus, which is unauthenticated on the LAN and trusts its `from` field. So a non-owner agent (e.g. @taOS-website-dev) had to ride the owner's account /SSH to post, and any caller could spoof any identity. Add POST /api/a2a/bus/send: - Authorized senders: an admin session/local token (may set an explicit `from`), or an agent registry JWT holding the `a2a_send` scope. - For an agent caller the bus `from` is DERIVED from the agent's own registry handle; a client-supplied `from` is ignored, so one agent cannot post as another. - Fails 502 on bus error (a send must know it did not land), unlike the read proxies which degrade to an empty 200. - Allowlisted in the auth middleware as a write path (own set), preserving the no-skeleton-key guard. Tests: 7 new cases (from-derivation/anti-spoof, scope required, no-auth 401, admin explicit-from, validation 400, bus-error 502). Docs: agent-coordination.md gains the authenticated-send path; module docstring updated. doc-gate clean. * fold Kilo review on #1768: sanitize admin from, trim body, validate reply_to - Admin-supplied "from" is stripped of non-printable chars and capped at 64 so it cannot inject newlines/control chars into the bus record or logs. - The message body is trimmed before forwarding, consistent with thread. - reply_to, when present, must be a positive message id (rejects 0/negative). - 4 new tests cover the three folds. * bound reply_to to the 64-bit id space (Kilo suggestion on #1768)
…1769) * test(designstudio): add coverage for the untested Design Studio app The designstudio/ directory shipped with zero tests. Add 105 vitest cases across 11 files covering the pure logic and the view components, matching the music/game studio testing conventions (fetch stubbing, controlled-host rendering, react-konva mocked to inert passthroughs). - types: hasFill/hasStroke type guards, isValidDesignContent validation, constants - elementFactory: element creation, centering, zIndex stacking, id uniqueness, duplicate - designs-api: list/get/create/update/rename/delete fetch wiring and error messages - useElementHistory: commit/undo/redo, redo-stack clearing, empty-stack no-ops, history cap - useHtmlImage: load/error lifecycle, crossOrigin, unmount cancellation - LayersPanel: ordering, labels, select/reorder/visibility/delete, disabled edges - PropertiesPanel: per-type controls, position display, duplicate/delete, disabled chips - TemplatesView: grid, filter pills, selection dimensions - MagicView: prompt/generate/style chips, needs-model and error states, results - LibraryView: empty/error states, open, backend-confirmed delete, rename - DesignView: toolbar add/undo, auto-select, brand-swatch gating, zoom controls * fold Kilo suggestion on #1769: verify the history-cap eviction branch The undo-cap test only asserted the most-recent snapshot after undo; now it also walks undo to exhaustion and asserts the original (v0) state was evicted and the undo depth is bounded by the 100-entry cap, exercising the eviction branch. (The DesignView 'latest' state-isolation suggestion is already handled by the existing afterEach reset.)
…in (Slice 1) (#1770) * docs(design): taOSgo mesh-join foundation + Game Studio asset-generation specs Two draft design specs for Jay review: - taOSgo mesh-join foundation: unblocks Web Studio publish (#167) and off-LAN remote access. Controller has no mesh-join today; specs the credential store + poll-intercept persistence, headless Headscale join (system tailscale vs tsnet decision), and the publish caller + static-site server, in 3 shippable slices. - Game Studio asset-generation (#55): tier-aware textures/audio/(stretch)3D reusing the existing image + MusicGen backends, spec -> build -> harden. * feat(taosgo): persist per-host mesh service credentials at cluster-join (Slice 1) The controller had no writer for the get_controller_token() seam: it read a placeholder env var, so the taOSnet passkey fetch (and the future Web Studio publish caller) had no credential once a host actually joined an account mesh. Add tinyagentos/taosnet/mesh_credentials.py: a host-local 0600 store for the per-host service tokens taos.my returns in the cluster-join ready payload (controller_token = taosnet:passkey, sites_token = sites:publish). The single-use headscale preauth key is intentionally NOT persisted. get_controller_ token()/get_sites_token() read it, with the TAOS_CONTROLLER_TOKEN/TAOS_SITES_ TOKEN env overrides still winning for dev/tests. Wire it via a poll-intercept: routes/account_proxy.py::cluster_join_poll now persists the ready-payload tokens server-side and strips them from the browser- facing body, so a bearer credential never reaches browser JavaScript. Fail-soft: non-200/non-JSON/unparseable bodies and save failures leave the response untouched. passkey_client.get_controller_token() delegates to the new store. This is Slice 1 of docs/design/taosgo-mesh-join-foundation.md: credentials only, no mesh-join yet (Slice 2). It already makes the headless taOSnet passkey fetch real once a host joins. Tests: store round-trip, 0600 perms, preauth-key not persisted, idempotent re-poll, env override, and the intercept persist+strip + fail-soft matrix (13). Existing account-proxy/taosnet suites stay green. * fold Kilo review on #1770: decouple token-strip from save, harden loads - account_proxy: the service tokens are now ALWAYS stripped from the browser body once seen in a 200 JSON response, whether or not the save succeeds. The previous fail-soft returned the unstripped body on a save error, leaking the bearer token (e.g. a payload with controller_token but no host_id raised in save). Parsing no longer depends on Content-Type, so a JSON body served without one cannot bypass stripping either. - mesh_credentials._load(): a corrupt/externally-edited file that parses to a non-object (list/string) is treated as absent, so the getters can never raise AttributeError and break the headless passkey fetch. - .gitignore: data/mesh_credentials.json (never commit a bearer-token file). - 3 regression tests: strip-even-when-save-fails, corrupt-non-dict-file, json-without-content-type. 16 pass.
…ts (#1694) * fix(catalog): fix 6 broken install-script references in agent manifests Three agents switched from broken script references to pip install (agent-zero, hermes, openclaw — all available on PyPI). Three agents (deer-flow, moltis, picoclaw) had missing install scripts created. Also renamed deer-flow's reference from the generic 'scripts/install.sh' to 'scripts/install-deer-flow.sh' to avoid namespace collision. Fixes #888 * docs: document agent framework install scripts (fix doc-drift gate) Documents the three new agent framework install scripts added in the catalog-broken-install-scripts fix: - scripts/install-deer-flow.sh (clone + uv sync, Python 3.12) - scripts/install-moltis.sh (cargo install from crates.io) - scripts/install-picoclaw.sh (clone + cmake build) Also notes that agent-zero, Hermes Agent, and OpenClaw use pip installs via their catalog manifests, reflecting the manifest changes that switched them from missing/broken script references to method: pip. This satisfies the doc-gate 'installers' rule which requires README.md edits when install scripts are added or removed. * fix(catalog): revert agent-zero and openclaw to script-based install, fix picoclaw make install, correct deer-flow note agent-zero and openclaw were switched to method: pip in the prior catalog fix, but both PyPI names resolve to unrelated packages: - agent-zero on PyPI = voice-agent framework with VAD/WebSocket routes, not frdel/agent-zero (autonomous AI agent) - openclaw on PyPI = no matching package exists (CMDOP plugin) Both reverted to clone-and-run via install-agent-zero.sh and install-openclaw.sh. hermes-agent on PyPI is confirmed correct (Nous Research — kept as pip). Also fixed: - scripts/install-picoclaw.sh: added make install step so the binary lands on PATH (was only building, never installing) - app-catalog/agents/deer-flow/manifest.yaml: removed false claim that the script writes config.yaml+systemd (it doesn't) - README.md: updated agent framework table and description --------- Co-authored-by: Hogne <227774406+hognek@users.noreply.github.com>
…e 2) (#1772) * feat(taosgo): headless Headscale mesh-join via system tailscale (Slice 2) Slice 2 of docs/design/taosgo-mesh-join-foundation.md. Jay chose system tailscale (real kernel networking, boot-persistent) over userspace tsnet. - tinyagentos/taosnet/mesh.py: async, fail-soft wrappers over the tailscale CLI: mesh_up (tailscale up --login-server hs.taos.my --authkey <preauth> --hostname), mesh_status (parse tailscale status --json -> joined/tailnet/node_ip), is_joined, mesh_down, is_tailscale_installed. A host without the binary degrades to a structured "not available" result; nothing raises into the join flow. Login server overridable via TAOS_HEADSCALE_URL. - routes/account_proxy.py: the cluster-join poll-intercept now consumes the single-use headscale_preauth_key server-side to join the mesh (fire-and-forget background task, guarded by is_joined() to avoid re-join) and strips the preauth key from the browser body too. New GET /api/account/mesh/status for the Account pane. - The strip decision stays decoupled from the join: secrets are always removed from the browser body whether or not the join succeeds. Tests: 15 for mesh.py (up/status/down/installed-guard, arg shape, fail-soft matrix) + 2 intercept tests (preauth stripped + join intent surfaced); Slice-1 intercept tests updated for the new tuple return. 42 pass across mesh/creds/ account-proxy. doc-gate clean. FOLLOW-UP (Slice 2b, needs live Pi): install.sh must install tailscale + tailscaled on supported hosts, and a real join against hs.taos.my must be verified end to end. Until then this is inert on hosts without tailscale. * fold Kilo review on #1772: always-strip preauth, single-use guard, task ref - account_proxy: the intercept now fires on ANY secret in the body (service token OR preauth key), so a ready payload carrying only a preauth key is still stripped -- the "ALWAYS strip" guarantee no longer hinges on a service token. Persist runs only when a controller_token is present. - Single-use key guard: a preauth key that already had a join attempted is not re-fired on a re-delivered ready poll (in-memory _attempted_preauth), since a single-use key cannot be retried anyway. - create_task GC-safety: keep a strong reference to the in-flight join task (module set + done-callback discard) so it is not garbage-collected mid-run. - mesh.py: mesh_status docstring corrected to reflect that detail is only on the not-available/error path, not the success dict. - +1 regression test (preauth-only payload still stripped). 43 pass.
…lice 1) (#1773) * feat(gamestudio): AI texture/sprite generation via ComfyUI backend (Slice 1) Add offline texture/sprite generation to the Game Studio, driven by a ComfyUI backend and gated by hardware tier, mirroring the Images Studio tier-aware routing. Backend: - tinyagentos/asset_gen: async, fail-soft ComfyUI client (submit /prompt, poll /history, fetch /view) with a checked-in SDXL text-to-image workflow and a parameteriser (prompt/size/seed/checkpoint, tileable prompt hint). - routes/game_assets.py: POST /api/games/{id}/assets/texture. Resolves the tier (catalog ComfyUI backend, else local GPU=SDXL / NPU=SD1.5, else none), returns 200 {available:false} when no accelerator, writes the PNG into the game's file set, and returns its preview path. Fail-soft on backend errors. - games.py: preserve binary assets on save so a generated texture is not swept by the text-only full-replace. - backend_catalog: comfyui advertises image-generation. Frontend: - AssetsPanel + game-assets-api + asset-reference, wired into EditorView as a Chat/Assets tab. Tier-gated "Needs a GPU worker" state, preview, and insert. Tests: 51 Python (client/workflow/route/games) + 9 desktop (AssetsPanel, asset-reference) all green. ComfyUI HTTP is fully mocked; no live install. Docs-Reviewed: new asset route + gamestudio panel; ComfyUI is a store backend, no coordination-doc or README surface changes needed for Slice 1 * fix(gamestudio): fold Kilo review findings on the ComfyUI asset slice - game_assets: 502 error body dropped the misleading `available:true` (the client throws on non-2xx and only reads `.error`; `available` is a 200-only tier-gate signal). - game_assets: cap generated PNGs by per-image size (12MB) and per-game count (200) so a runaway request can't fill the disk; refuse at the cap with 409 before spending a GPU call. - game_assets: drop the NPU->SD1.5 tier. There is no wired NPU texture backend, so it resolved to a local ComfyUI that always 502s; return unavailable (needs-a-GPU-worker) instead of an always-failing tier. - games: preserve generated binary assets on full-replace save by known binary extension first (png/jpg/webp/glb/wav/...), then the undecodable-bytes check, so a binary payload that happens to decode as UTF-8 is never swept. - asset-reference: derive the JS const name from the filename so repeated inserts into one JS file don't emit a colliding `const assetUrl`. - workflows: cache load_template (fixed small template set; callers deep-copy before mutating) so it isn't re-read+re-parsed from disk on every generation. Tests updated/added: 502-body omits `available`, asset-count cap 409, distinct JS const names. 52 python + 10 desktop pass.
…#1774) * feat(agents): least-privilege project_tasks registry scope for kanban Add a project_tasks registry-JWT scope so an approved external agent can drive ONLY its own project's task board with its Ed25519 token, no session cookie. This unblocks the executor lane (agent claims/updates/closes tasks). Enforces five security invariants: 1. An agent token reaches task routes only for the project whose id equals the token's project_id claim; any other project is an existence-hiding 404, identical to a non-owner session. 2. project_tasks grants task read + lifecycle + comments only. Task create, member management, and project lifecycle stay session owner/admin only. 3. The verified token's canonical_id is the authoritative actor; a lifecycle body actor id (claimer_id/releaser_id/closed_by/reopened_by) that differs is rejected 403 (an agent may only act as itself). 4. Session owner/admin behavior is unchanged (regression covered). 5. A registry JWT authenticates nothing off the allowlist (no skeleton key); the task-path passthrough uses an anchored, segment-strict regex. Changes: - agent_token_auth.py: extract a shared verifier and add check_agent_scope_for_project; check_agent_scope signature/behavior kept. - auth_middleware.py: add _is_agent_task_path matcher and OR it into the registry-JWT passthrough. - routes/agent_auth_requests.py: add project_tasks to VALID_SCOPES. - routes/projects.py: dual-auth the allowed task routes via _authorize_task_actor and bind agent actor ids via _resolve_actor. * fix(agents): bind comment author to the token id (close actor-impersonation) add_comment gated task comments behind project_tasks but passed the body's author_id straight to the store, so an agent caller could post a comment authored as another identity. A comment author is an actor id like the four lifecycle fields, so run it through _resolve_actor: an agent is pinned to its canonical_id and a mismatched body author returns 403; a session owner keeps its provided author_id. Tests: agent comment as another id -> 403; as self -> stored under the token id. * fix(agents): bind comment author_id to token identity (invariant 3) add_comment gated an agent caller but then trusted the request body's author_id, so an agent could post a comment authored as another identity. Pin author_id to the verified token canonical_id via _resolve_actor exactly like the lifecycle handlers: a mismatched body author is rejected 403, and an omitted author is stored as the agent's own canonical_id. author_id becomes optional so an agent may omit it (the server pins it); a session caller that omits it is rejected 400 rather than inserting a NULL author (the column is NOT NULL). Audit of every agent-gated task route confirms each body-actor field is now bound to the token for agent callers: claim (claimer_id), release (releaser_id), close (closed_by), reopen (reopened_by), comment (author_id). The read-only routes (list/ready/get/list_comments/context) carry no actor field. update_task's assignee_id is a prospective task target, not an actor identity, and the route records no actor, so it is intentionally not pinned. Tests: agent comment with author_id != canonical_id -> 403; omitted author succeeds and is stored as the canonical_id. * fix(agents): tighten project_tasks scope per Kilo review on #1774 Two folds from the Kilo review: WARNING: PATCH update_task was on the agent allowlist, letting a project_tasks agent free-mutate title/body/assignee_id/parent (broader than the documented read + lifecycle + comments, and assignee_id had no actor binding). PATCH is now session owner/admin only and off the middleware allowlist; agents drive the board via read + claim/release/close/reopen + comments. Test relocated to assert an agent token is rejected on PATCH. SUGGESTION (defense in depth): check_agent_scope_for_project trusted only the token project_id claim. It now also requires an active grant whose own project_id matches, so a token claim and grant that diverge cannot authorize. * fix(agents): add project_tasks to registry _ALLOWED_SCOPES (sync with VALID_SCOPES) The consent-flow VALID_SCOPES gained project_tasks but the registry grant allowlist _ALLOWED_SCOPES did not, so test_allowed_scopes_matches_canonical_valid_scopes failed on CI (the two lists must not drift). Add project_tasks so a grant for it is accepted by the registry grant path, matching the consent vocabulary.
…ct-tasks scope, managed backend services) (#1775)
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. |
📝 WalkthroughWalkthroughBeta.39 adds Game Studio asset generation, taOSgo mesh joining, managed backend service controls, project-scoped agent authorization, atomic VRAM reservations, worker model advertisement, installer updates, release metadata, and broad frontend/backend test coverage. ChangesBeta.39 platform and feature updates
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant EditorView
participant AssetsPanel
participant GameAssetsAPI
participant ComfyUI
participant GameDirectory
EditorView->>AssetsPanel: Select Assets tab
AssetsPanel->>GameAssetsAPI: POST texture generation request
GameAssetsAPI->>ComfyUI: Submit workflow and poll output
ComfyUI-->>GameAssetsAPI: Return image bytes
GameAssetsAPI->>GameDirectory: Save generated PNG
GameAssetsAPI-->>AssetsPanel: Return preview and filename
AssetsPanel->>EditorView: Insert asset reference
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| """ | ||
| reservation = self._pending.pop(reservation_id, None) | ||
| if reservation is not None: | ||
| self._reserved_vram_mb -= reservation.vram_mb |
There was a problem hiding this comment.
WARNING: release() mutates _reserved_vram_mb without holding self._lock
reserve() holds the lock but await asyncio.to_thread(self._probe_vram) (line 92) yields control; release() runs without the lock and can decrement _reserved_vram_mb during that await. When reserve() resumes it reads the now-smaller reserved total (line 107) and can over-admit — reopening the exact TOCTOU race this feature was built to prevent. Acquire self._lock in release() (and make the read-only accessors lock-safe).
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| path should already have been admitted fail-open by :meth:`reserve` | ||
| (a deny message never reports these zeros as real capacity). | ||
| """ | ||
| probe = self._probe_vram() |
There was a problem hiding this comment.
WARNING: Blocking nvidia-smi probe runs synchronously on the event loop
available_vram() (and stats() at line 182) call the blocking _probe_vram() subprocess directly on the asyncio loop instead of asyncio.to_thread. available_vram() is reachable on the request hot path via the 503 branch in models.py:666, so every denied model pull blocks the entire event loop (and all concurrent requests) for the probe duration. Wrap these calls in asyncio.to_thread.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| body.required_vram_mb, caller=f"ollama-pull:{model_name}", | ||
| ) | ||
| if reservation is None: | ||
| effective_free, _total = vram_mgr.available_vram() |
There was a problem hiding this comment.
WARNING: Synchronous blocking VRAM probe on the request hot path
vram_mgr.available_vram() (used in the 503 “insufficient VRAM” branch) calls a blocking nvidia-smi subprocess directly on the event loop, stalling all concurrent requests while denied pulls wait on the probe. Make the accessor use asyncio.to_thread (see the vram_reservation.py finding).
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| const ref = buildAssetReference(path, filename); | ||
| setFiles((prev) => { | ||
| const next = { ...prev, [path]: (prev[path] ?? "") + ref }; | ||
| void flushSave(next); |
There was a problem hiding this comment.
WARNING: Side effect (flushSave) inside a setState updater
void flushSave(next) is invoked inside the setFiles((prev) => { ... }) updater. React requires updater functions to be pure; under StrictMode (dev double-invoke) this save runs twice, risking redundant/raced writes. Compute next outside the updater, then call setFiles(next) and flushSave(next) separately.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| json={"priority": 5}, | ||
| headers=_hdr(token), | ||
| ) | ||
| assert resp.status_code in (401, 403, 404) |
There was a problem hiding this comment.
WARNING: Loose status-code assertion can mask a security regression
assert resp.status_code in (401, 403, 404) — a valid pid is always supplied, so the dual-auth gate's only correct denial here is 401 (404 would mean an unauthorized agent reached project lookup). Including 404 lets a regression that does project lookup before the auth gate pass silently. Pin to == 401 (the same looseness appears at lines 309, 320, 329, 336).
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| return {"unit": unit, "ok": False, "scope": scope, "detail": f"{verb} timed out"} | ||
| except Exception as exc: # pragma: no cover - defensive | ||
| return {"unit": unit, "ok": False, "scope": scope, "detail": str(exc)[:200]} | ||
|
|
There was a problem hiding this comment.
SUGGESTION: Inconsistent access: direct subscript backend[\"type\"]
The surrounding loop uses .get(...) for fail-soft access; this direct subscript is inconsistent and would raise KeyError if a backend entry lacked type. Use backend.get(\"type\") for uniformity.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| pass | ||
|
|
||
| p = _path() | ||
| tmp = p.with_name(p.name + ".tmp") |
There was a problem hiding this comment.
SUGGESTION: Stale .tmp file left on error path
If the write fails after mesh_credentials.json.tmp is created, the temp file is never cleaned up and lingers until the next successful save. Add a try/finally to remove the temp file on failure.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| """ | ||
| errors: list[str] = [] | ||
| services_dir = root / "services" | ||
| for manifest in sorted(services_dir.glob("*/manifest.yaml")): |
There was a problem hiding this comment.
SUGGESTION: glob silently passes when the services dir is missing
If root/services (or a typo'd --root) doesn't exist, glob yields nothing and main prints “clean” (exit 0), silently passing the CI gate. Add an explicit services_dir.is_dir() check that errors out.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| log "installing agent-zero with pip" | ||
| cd "$AGENT_ZERO_HOME" | ||
| pip install -e . || die "pip install failed in ${AGENT_ZERO_HOME}" | ||
|
|
There was a problem hiding this comment.
SUGGESTION: Use python3 -m pip instead of bare pip
The prereq check only verifies python3; on hosts where pip is absent or mapped to python2 the install fails. Use python3 -m pip install -e . for consistency.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| log "installing openclaw with pip" | ||
| cd "$OPENCLAW_HOME" | ||
| pip install -e . || die "pip install failed in ${OPENCLAW_HOME}" | ||
|
|
There was a problem hiding this comment.
SUGGESTION: Use python3 -m pip instead of bare pip
Same as above — the prereq check verifies python3 but the install uses bare pip. Use python3 -m pip install -e ..
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 14 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL(none) WARNING
SUGGESTION
Files Reviewed (83 files)
Fix these issues in Kilo Cloud Reviewed by hy3-20260706:free · Input: 111.7K · Output: 21.3K · Cached: 594.3K |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (12)
scripts/install-openclaw.sh (1)
22-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffShared clone/update logic is duplicated across installer scripts.
The clone-or-update block (check
.git, fetch, checkout, pull, or mkdir + chown + clone) is identical ininstall-agent-zero.sh,install-openclaw.sh, andinstall-picoclaw.sh. Consider extracting a shared helper (e.g.,scripts/install-common.sh) to reduce maintenance burden and ensure consistent behavior.🤖 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 `@scripts/install-openclaw.sh` around lines 22 - 32, Extract the duplicated clone-or-update logic from install-agent-zero.sh, install-openclaw.sh, and install-picoclaw.sh into a shared helper such as install-common.sh, exposing a function that accepts the home directory, version, and repository (and uses the existing log behavior). Source the helper from each installer and replace their inline .git check, fetch/checkout/pull, or mkdir/chown/clone blocks with calls to the shared function while preserving current arguments and behavior.scripts/install-moltis.sh (1)
11-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
MOLTIS_HOMEis defined but never used.The variable is declared but no code references it. Either remove it or use it (e.g., as a clone target if you add a git-based fallback path).
♻️ Proposed fix
MOLTIS_VERSION="${TAOS_MOLTIS_VERSION:-latest}" MOLTIS_REPO="https://github.com/moltis-org/moltis.git" -MOLTIS_HOME="/opt/moltis"🤖 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 `@scripts/install-moltis.sh` at line 11, Remove the unused MOLTIS_HOME variable from scripts/install-moltis.sh, unless the installation flow is updated to use it as the target directory for a git-based fallback path.Source: Linters/SAST tools
tests/test_cluster_worker_protocol.py (1)
294-320: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the retained status when deduplicating IDs.
This fixture places
availablebeforeloaded, but the assertions do not verify which status survives. Assert the intended precedence—likelyloaded—so the test catches status loss.🤖 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/test_cluster_worker_protocol.py` around lines 294 - 320, Update test_heartbeat_dedupes_across_backends to assert the deduplicated model retains the intended higher-priority status, likely "loaded", by checking w.available_models[0]["status"] in addition to the existing model_id assertion.desktop/src/apps/gamestudio/EditorView.tsx (2)
272-284: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMove
flushSavecall out of thesetFilesupdater.Calling
void flushSave(next)inside thesetFilesupdater introduces a side effect in a function that should be pure. In React Strict Mode, the updater may be called twice, causing duplicate save POST requests. ComputenextfromfilesRef.currentoutside the updater and callflushSaveseparately. The same pattern exists in the pre-existinghandleApplyat line 263.♻️ Proposed refactor for handleInsertAsset
const handleInsertAsset = useCallback( (filename: string) => { const path = activePath; if (!path) return; const ref = buildAssetReference(path, filename); - setFiles((prev) => { - const next = { ...prev, [path]: (prev[path] ?? "") + ref }; - void flushSave(next); - return next; - }); + const next = { ...filesRef.current, [path]: (filesRef.current[path] ?? "") + ref }; + setFiles(next); + void flushSave(next); }, - [activePath, flushSave], + [activePath, flushSave], );🤖 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/gamestudio/EditorView.tsx` around lines 272 - 284, Move the flushSave side effect out of the setFiles updater in handleInsertAsset and the existing handleApply handler. Compute the updated files object from filesRef.current, call setFiles with the resulting state, then invoke flushSave(next) separately so the updater remains pure and Strict Mode cannot duplicate save requests.
417-444: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd
role="tabpanel"and associations for the tab panels.The tablist correctly uses
role="tab"andaria-selected, but the content panels lackrole="tabpanel"andaria-labelledby/aria-controlsassociations. Screen reader users navigating by panel role won't find the sidebar content. Addingidto each tab andaria-labelledbyon each panel completes the WAI-ARIA tab pattern.♿ Suggested accessibility improvements
<button type="button" role="tab" + id="editor-tab-chat" + aria-controls="editor-panel-chat" aria-selected={rightTab === "chat"} onClick={() => setRightTab("chat")} ... > <Sparkles size={14} className="text-accent" /> Ask the agent </button> <button type="button" role="tab" + id="editor-tab-assets" + aria-controls="editor-panel-assets" aria-selected={rightTab === "assets"} onClick={() => setRightTab("assets")} ... > <ImagePlus size={14} className="text-accent" /> Assets </button> </div> {rightTab === "assets" ? ( - <AssetsPanel gameId={gameId} activePath={activePath} onInsert={handleInsertAsset} /> + <div role="tabpanel" id="editor-panel-assets" aria-labelledby="editor-tab-assets" className="flex min-h-0 flex-1 flex-col"> + <AssetsPanel gameId={gameId} activePath={activePath} onInsert={handleInsertAsset} /> + </div> ) : ( - <> + <div role="tabpanel" id="editor-panel-chat" aria-labelledby="editor-tab-chat" className="flex min-h-0 flex-1 flex-col"> <div className="min-h-0 flex-1 overflow-auto px-3 py-2"> ... </div> ... - </> + </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/gamestudio/EditorView.tsx` around lines 417 - 444, Add complete WAI-ARIA tab associations in the sidebar: give the chat and assets buttons stable unique IDs and matching aria-controls values, then add role="tabpanel", corresponding aria-labelledby values, and matching panel IDs to the rendered content containers for each rightTab view. Update the relevant rightTab conditional panels while preserving their existing content.scripts/install-deer-flow.sh (1)
9-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPin the default
DEERFLOW_VERSIONfor reproducible installs.The default
mainis a moving target — two installs on different days can produce different code. This release includes rkllama 1.3.0 pinning, so pinning deer-flow to a specific commit or tag would be consistent. TheTAOS_DEERFLOW_VERSIONenv var already allows overrides.♻️ Proposed fix: pin to a specific ref
-DEERFLOW_VERSION="${TAOS_DEERFLOW_VERSION:-main}" +DEERFLOW_VERSION="${TAOS_DEERFLOW_VERSION:-<specific-tag-or-commit-sha>}"🤖 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 `@scripts/install-deer-flow.sh` at line 9, Change the default value assigned to DEERFLOW_VERSION from the moving main branch to a fixed release tag or commit that includes rkllama 1.3.0, while preserving the TAOS_DEERFLOW_VERSION environment-variable override.tinyagentos/app.py (1)
1144-1149: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd an eager
Nonedefault forapp.state.vram_reservation.Every other lifespan-only attribute in this file (
mcp_supervisor,orchestrator,trace_registry,bridge_sessions,vapid_keypair, etc.) gets an explicitNonein the eager-init block so attribute-existence checks work before/without the lifespan running.vram_reservationis the exception — it's only ever set insidelifespan(). Current consumers usegetattr(..., None)so this isn't a crash today, but it silently deviates from the file's established defensive pattern.♻️ Proposed fix
app.state.agent_grants = agent_grants_store app.state.app_grants = app_grants_store app.state.license_acceptances = license_acceptances_store app.state.cluster_pairing = cluster_pairing_store app.state.capability_map = capability_map_store + # vram_reservation is created by the lifespan; None here keeps + # attribute-existence checks consistent during the pre-startup window. + app.state.vram_reservation = NoneAlso applies to: 1484-1602
🤖 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/app.py` around lines 1144 - 1149, Add an explicit None initialization for app.state.vram_reservation in the eager state-initialization block, alongside mcp_supervisor, orchestrator, and other lifespan-only attributes. Keep the lifespan() assignment that replaces it with VramReservationManager, and ensure all consumers continue using the established state access pattern.tinyagentos/agent_token_auth.py (1)
168-189: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRedundant
list_grantsquery on every project-scoped auth check.
_verify_agent_scope(line 115) already fetches grants vialist_grants(canonical_id). Thencheck_agent_scope_for_project(line 181) fetches the same grants again for the defense-in-depth project-binding check. This doubles the DB queries on every project-scoped authorization.Returning the grants from
_verify_agent_scopeeliminates the redundant fetch without changing the security semantics.♻️ Proposed refactor
async def _verify_agent_scope( request: Request, required_scope: str -) -> Optional[tuple[str, dict]]: +) -> Optional[tuple[str, dict, list]]: """Shared verifier for the agent-token checks. Return ``(canonical_id, payload)`` for a valid Bearer registry JWT that holds an active *required_scope* grant, or ``None`` when no Authorization header is present. Raises the same 401/403 documented on ``check_agent_scope``. """ # ... (unchanged verification logic) ... - return canonical_id, payload + return canonical_id, payload, grants async def check_agent_scope(request: Request, required_scope: str) -> Optional[str]: # ... - canonical_id, _payload = result + canonical_id, _payload, _grants = result return canonical_id async def check_agent_scope_for_project( request: Request, required_scope: str, project_id: str ) -> Optional[str]: # ... - canonical_id, payload = result + canonical_id, payload, grants = result token_project = payload.get("project_id") if not token_project or token_project != project_id: raise HTTPException(status_code=403, detail=PROJECT_SCOPE_MISMATCH_DETAIL) - grants_store = _get_grants_store(request) now = datetime.now(timezone.utc) - grants = await grants_store.list_grants(canonical_id) grant_ok = any( g["scope"] == required_scope and g.get("project_id") == project_id and _grant_unexpired(g.get("expires_at"), now) for g in grants )🤖 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/agent_token_auth.py` around lines 168 - 189, Update _verify_agent_scope to return the fetched grants alongside canonical_id and payload, then adjust its callers to unpack the expanded result. In check_agent_scope_for_project, reuse those grants for the project_id, scope, and expiration validation and remove the second _get_grants_store/list_grants call, preserving the existing authorization behavior.tinyagentos/routes/account_proxy.py (1)
33-37: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider tuple unpacking instead of concatenation (RUF005).
_JOIN_SERVICE_TOKENS + ("headscale_preauth_key",)works but*_JOIN_SERVICE_TOKENS, "headscale_preauth_key"is the more idiomatic Python 3 form.♻️ Proposed fix
-_STRIP_KEYS = _JOIN_SERVICE_TOKENS + ("headscale_preauth_key",) +_STRIP_KEYS = (*_JOIN_SERVICE_TOKENS, "headscale_preauth_key")🤖 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/account_proxy.py` around lines 33 - 37, Update the _STRIP_KEYS definition to use tuple unpacking, replacing tuple concatenation with *_JOIN_SERVICE_TOKENS followed by "headscale_preauth_key".tinyagentos/taosnet/mesh_credentials.py (1)
106-125: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider caching
_load()result when multiple getters are called in sequence.Each getter (
get_controller_token,get_sites_token,get_host_id) independently calls_load(), re-reading and re-parsing the JSON file from disk. If a caller calls several getters together (e.g.,has_mesh_credentials()→get_controller_token()→_load()), the file is read redundantly. For the current single-call use cases this is fine, but a module-level cache (invalidated on save/clear) would eliminate repeated disk I/O if usage expands.🤖 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/taosnet/mesh_credentials.py` around lines 106 - 125, Cache the parsed credentials returned by _load() at module scope and reuse it across get_controller_token(), get_sites_token(), get_host_id(), and has_mesh_credentials() calls; invalidate or refresh that cache whenever credentials are saved or cleared so reads remain current.tests/test_mesh.py (1)
74-78: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
_STATUSis a mutable class attribute (RUF012).If a test ever mutates
_STATUS, subsequent tests would see the modified data. Marking it as a class-level constant or using a fixture would be safer.♻️ Proposed fix
class TestMeshStatus: - _STATUS = { + _STATUS: dict = { "BackendState": "Running", "CurrentTailnet": {"Name": "jason"}, "Self": {"Online": True, "HostName": "node-1", "TailscaleIPs": ["100.64.0.5"]}, }Alternatively, use a
@pytest.fixtureor copy the dict in each test.🤖 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/test_mesh.py` around lines 74 - 78, Update the _STATUS class attribute in tests/test_mesh.py to avoid mutable shared state: annotate it as a ClassVar if it is intended to remain class-level, and ensure tests do not mutate it directly; alternatively, provide the status through a pytest fixture or return a fresh copy for each test.tinyagentos/taosnet/mesh.py (1)
57-64: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider logging the exception in the timeout cleanup path.
The bare
except Exception: passon lines 62-63 silently swallows any error duringproc.kill()/proc.wait(). A debug-level log would aid troubleshooting if the cleanup ever fails on an exotic filesystem or zombie process.♻️ Proposed fix
except asyncio.TimeoutError: if proc is not None: try: proc.kill() await proc.wait() - except Exception: # pragma: no cover - defensive - pass + except Exception: # pragma: no cover - defensive + logger.debug("taosgo: cleanup after timeout failed", exc_info=True) return 124, "", f"timed out after {timeout}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 `@tinyagentos/taosnet/mesh.py` around lines 57 - 64, In the timeout cleanup path of the process execution function, replace the silent broad exception handler around proc.kill() and proc.wait() with a debug-level log that records the cleanup exception and relevant process context, while preserving the existing timeout return behavior.
🤖 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/src/apps/gamestudio/asset-reference.ts`:
- Around line 12-24: buildAssetReference interpolates filename into HTML, CSS,
JavaScript, and fallback output without validation or escaping. Validate
filename before constructing rel, rejecting unsafe characters such as quotes,
backslashes, angle brackets, and control characters, or escape it appropriately
for each target syntax; ensure assetVarName receives the validated filename and
all generated snippets remain syntactically safe.
In `@scripts/install-agent-zero.sh`:
- Line 37: Create and activate a dedicated virtual environment in
scripts/install-agent-zero.sh before the editable install, then run pip through
that environment for the existing install step. Update the setup flow around the
pip install command to use the virtualenv’s Python/pip and preserve the existing
die failure handling.
In `@scripts/install-moltis.sh`:
- Around line 36-37: Update the install command in the shell script to support
branch-based MOLTIS_VERSION values: attempt the existing --tag flow, then fall
back to --branch or --rev when it fails, while preserving the existing die
behavior if all attempts fail. Ensure the logic uses the MOLTIS_REPO and
MOLTIS_VERSION variables consistently.
In `@scripts/install-openclaw.sh`:
- Line 37: Update the installation command in install-openclaw.sh to avoid PEP
668 failures by installing into a virtual environment or using a user-scoped
installation such as pip install --user -e .; preserve the existing die error
handling and ensure the chosen approach works for the script’s runtime commands.
In `@scripts/install-picoclaw.sh`:
- Line 52: Replace the broad grep-based detection in the install-target check
with a reliable Makefile target check, preferably invoking make’s dry run for
the `install` target or matching an exact `^install:` definition in both
candidate Makefiles. Ensure the subsequent `sudo make install` runs only when a
real install target is present, while preserving the existing fallback behavior.
In `@tests/test_agent_token_auth.py`:
- Around line 68-103: The tests lack coverage for the defense-in-depth
grant/project mismatch in check_agent_scope_for_project. Add a test that creates
a token whose project_id claim is “prj-1” while its project_tasks grant is bound
to “prj-OTHER”, then assert the call raises HTTPException with status 403 and
detail PROJECT_SCOPE_MISMATCH_DETAIL; extend or bypass _mint as needed to create
these divergent values.
In `@tests/test_worker_manifest.py`:
- Around line 171-178: Update test_default_path_fallback to monkeypatch
worker_manifest.DEFAULT_MANIFEST_PATH to a nonexistent path under tmp_path
before calling load_manifest(), ensuring the test does not depend on whether the
host’s /etc/taos/worker-models.json exists; retain the assertions for the empty
manifest and the patched default path.
In `@tinyagentos/cluster/manager.py`:
- Around line 220-231: Update the available-model flattening logic in the
backend aggregation block so duplicate model IDs are merged instead of silently
keeping the first entry. Ensure a later entry with status “loaded” takes
precedence over an earlier “available” entry, while preserving the existing
deduplication behavior and assigning the merged result to
worker.available_models.
In `@tinyagentos/taosnet/mesh.py`:
- Around line 86-92: Update the tailscale invocation in `_run` to write
`preauth_key` to a securely created temporary file and pass it using
`--auth-key=file:<path>` instead of including the secret directly in the command
arguments; ensure the temporary file is cleaned up after execution, including on
errors.
In `@tinyagentos/worker/agent.py`:
- Around line 183-199: Use resident-model data rather than downloaded-model data
when computing status in the worker manifest handling loop. For backends of type
rkllama or ollama, derive probed_names from backend["loaded_models"]; retain
backend["models"] for other backends, then continue using model_id membership to
assign "loaded" versus "available".
- Around line 187-198: In the manifest-processing loop, validate that `model_id`
is a non-empty string before using it in the `probed_names` membership check.
Replace the current truthiness-only guard after `m.get("model_id")` with an
`isinstance(model_id, str)` and non-empty validation, logging and skipping
invalid values so malformed entries cannot abort enrichment for valid models.
- Around line 170-191: Make the manifest mappings in worker_manifest.py
consistent with detect_backends(): either add discovery and capability handling
for the kokoro and whisper backend types so their entries are probed and
forwarded, or remove kokoro/whisper from SOFTWARE_TO_BACKEND_TYPE and the
manifest schema. Ensure detect_backends() no longer silently drops valid
advertised entries.
In `@tinyagentos/worker/worker_manifest.py`:
- Around line 73-79: Update the manifest-loading try block to read the file
explicitly as UTF-8 and include UnicodeDecodeError in the exceptions handled by
the existing malformed-manifest warning path. Preserve the current
logger.warning message and return empty behavior in the manifest parsing
function.
---
Nitpick comments:
In `@desktop/src/apps/gamestudio/EditorView.tsx`:
- Around line 272-284: Move the flushSave side effect out of the setFiles
updater in handleInsertAsset and the existing handleApply handler. Compute the
updated files object from filesRef.current, call setFiles with the resulting
state, then invoke flushSave(next) separately so the updater remains pure and
Strict Mode cannot duplicate save requests.
- Around line 417-444: Add complete WAI-ARIA tab associations in the sidebar:
give the chat and assets buttons stable unique IDs and matching aria-controls
values, then add role="tabpanel", corresponding aria-labelledby values, and
matching panel IDs to the rendered content containers for each rightTab view.
Update the relevant rightTab conditional panels while preserving their existing
content.
In `@scripts/install-deer-flow.sh`:
- Line 9: Change the default value assigned to DEERFLOW_VERSION from the moving
main branch to a fixed release tag or commit that includes rkllama 1.3.0, while
preserving the TAOS_DEERFLOW_VERSION environment-variable override.
In `@scripts/install-moltis.sh`:
- Line 11: Remove the unused MOLTIS_HOME variable from
scripts/install-moltis.sh, unless the installation flow is updated to use it as
the target directory for a git-based fallback path.
In `@scripts/install-openclaw.sh`:
- Around line 22-32: Extract the duplicated clone-or-update logic from
install-agent-zero.sh, install-openclaw.sh, and install-picoclaw.sh into a
shared helper such as install-common.sh, exposing a function that accepts the
home directory, version, and repository (and uses the existing log behavior).
Source the helper from each installer and replace their inline .git check,
fetch/checkout/pull, or mkdir/chown/clone blocks with calls to the shared
function while preserving current arguments and behavior.
In `@tests/test_cluster_worker_protocol.py`:
- Around line 294-320: Update test_heartbeat_dedupes_across_backends to assert
the deduplicated model retains the intended higher-priority status, likely
"loaded", by checking w.available_models[0]["status"] in addition to the
existing model_id assertion.
In `@tests/test_mesh.py`:
- Around line 74-78: Update the _STATUS class attribute in tests/test_mesh.py to
avoid mutable shared state: annotate it as a ClassVar if it is intended to
remain class-level, and ensure tests do not mutate it directly; alternatively,
provide the status through a pytest fixture or return a fresh copy for each
test.
In `@tinyagentos/agent_token_auth.py`:
- Around line 168-189: Update _verify_agent_scope to return the fetched grants
alongside canonical_id and payload, then adjust its callers to unpack the
expanded result. In check_agent_scope_for_project, reuse those grants for the
project_id, scope, and expiration validation and remove the second
_get_grants_store/list_grants call, preserving the existing authorization
behavior.
In `@tinyagentos/app.py`:
- Around line 1144-1149: Add an explicit None initialization for
app.state.vram_reservation in the eager state-initialization block, alongside
mcp_supervisor, orchestrator, and other lifespan-only attributes. Keep the
lifespan() assignment that replaces it with VramReservationManager, and ensure
all consumers continue using the established state access pattern.
In `@tinyagentos/routes/account_proxy.py`:
- Around line 33-37: Update the _STRIP_KEYS definition to use tuple unpacking,
replacing tuple concatenation with *_JOIN_SERVICE_TOKENS followed by
"headscale_preauth_key".
In `@tinyagentos/taosnet/mesh_credentials.py`:
- Around line 106-125: Cache the parsed credentials returned by _load() at
module scope and reuse it across get_controller_token(), get_sites_token(),
get_host_id(), and has_mesh_credentials() calls; invalidate or refresh that
cache whenever credentials are saved or cleared so reads remain current.
In `@tinyagentos/taosnet/mesh.py`:
- Around line 57-64: In the timeout cleanup path of the process execution
function, replace the silent broad exception handler around proc.kill() and
proc.wait() with a debug-level log that records the cleanup exception and
relevant process context, while preserving the existing timeout return behavior.
🪄 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: 34c0b469-5e47-4e11-b40b-5db37d9c432a
⛔ Files ignored due to path filters (2)
desktop/package-lock.jsonis excluded by!**/package-lock.jsonuv.lockis excluded by!**/*.lock
📒 Files selected for processing (83)
.github/workflows/doc-gate.yml.gitignoreCHANGELOG.mdREADME.mdapp-catalog/agents/agent-zero/manifest.yamlapp-catalog/agents/deer-flow/manifest.yamlapp-catalog/agents/hermes/manifest.yamlapp-catalog/agents/openclaw/manifest.yamlapp-catalog/services/rkllama/manifest.yamldesktop/package.jsondesktop/src/apps/designstudio/DesignView.test.tsxdesktop/src/apps/designstudio/LayersPanel.test.tsxdesktop/src/apps/designstudio/LibraryView.test.tsxdesktop/src/apps/designstudio/MagicView.test.tsxdesktop/src/apps/designstudio/PropertiesPanel.test.tsxdesktop/src/apps/designstudio/TemplatesView.test.tsxdesktop/src/apps/designstudio/designs-api.test.tsdesktop/src/apps/designstudio/elementFactory.test.tsdesktop/src/apps/designstudio/types.test.tsdesktop/src/apps/designstudio/useElementHistory.test.tsdesktop/src/apps/designstudio/useHtmlImage.test.tsdesktop/src/apps/gamestudio/AssetsPanel.test.tsxdesktop/src/apps/gamestudio/AssetsPanel.tsxdesktop/src/apps/gamestudio/EditorView.tsxdesktop/src/apps/gamestudio/asset-reference.test.tsdesktop/src/apps/gamestudio/asset-reference.tsdesktop/src/apps/gamestudio/game-assets-api.tsdocs/agent-coordination.mddocs/design/cluster-backend-service-management.mddocs/design/game-studio-asset-generation.mddocs/design/taosgo-mesh-join-foundation.mdpyproject.tomlscripts/check_manifests.pyscripts/install-agent-zero.shscripts/install-deer-flow.shscripts/install-moltis.shscripts/install-openclaw.shscripts/install-picoclaw.shscripts/install-rkllama.shscripts/install-rknpu.shtests/test_a2a_bus_agent_auth.pytests/test_agent_token_auth.pytests/test_asset_gen_comfyui.pytests/test_backend_services.pytests/test_check_manifests.pytests/test_cluster_worker_protocol.pytests/test_mesh.pytests/test_mesh_credentials.pytests/test_routes_agent_auth_requests.pytests/test_routes_game_assets.pytests/test_routes_games.pytests/test_routes_projects_agent_tasks.pytests/test_routes_system.pytests/test_vram_reservation.pytests/test_worker_manifest.pytinyagentos/__init__.pytinyagentos/agent_token_auth.pytinyagentos/app.pytinyagentos/asset_gen/__init__.pytinyagentos/asset_gen/comfyui_client.pytinyagentos/asset_gen/workflows.pytinyagentos/asset_gen/workflows/texture_sdxl.jsontinyagentos/auth_middleware.pytinyagentos/cluster/backend_services.pytinyagentos/cluster/manager.pytinyagentos/cluster/worker_protocol.pytinyagentos/routes/__init__.pytinyagentos/routes/a2a_bus.pytinyagentos/routes/account_proxy.pytinyagentos/routes/agent_auth_requests.pytinyagentos/routes/agent_registry.pytinyagentos/routes/game_assets.pytinyagentos/routes/games.pytinyagentos/routes/models.pytinyagentos/routes/projects.pytinyagentos/routes/system.pytinyagentos/scheduler/backend_catalog.pytinyagentos/taosnet/mesh.pytinyagentos/taosnet/mesh_credentials.pytinyagentos/taosnet/passkey_client.pytinyagentos/vram_reservation.pytinyagentos/worker/agent.pytinyagentos/worker/worker_manifest.py
Comment on lines
+12
to
+24
| export function buildAssetReference(activePath: string, filename: string): string { | ||
| const ext = activePath.slice(activePath.lastIndexOf(".") + 1).toLowerCase(); | ||
| const rel = `./${filename}`; | ||
| if (ext === "html" || ext === "htm") { | ||
| return `\n<img src="${rel}" alt="" />`; | ||
| } | ||
| if (ext === "css") { | ||
| return `\n/* generated asset */\n.generated-asset { background-image: url("${rel}"); }`; | ||
| } | ||
| if (ext === "js" || ext === "mjs" || ext === "ts") { | ||
| return `\n// generated asset\nconst ${assetVarName(filename)} = "${rel}";`; | ||
| } | ||
| return `\n/* asset: ${rel} */`; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Unescaped filename in generated code snippets can break output or enable injection.
buildAssetReference interpolates filename directly into HTML attributes (src="${rel}"), CSS url("${rel}"), and JS string literals ("${rel}") without escaping. A filename containing ", \, or < will produce syntactically broken code, and in the HTML case could introduce XSS if the generated markup is rendered in a preview context.
🛡️ Proposed fix: validate filename before generating reference
export function buildAssetReference(activePath: string, filename: string): string {
+ // Reject filenames with characters that could break or inject into generated
+ // HTML/CSS/JS snippets. Asset pipelines should produce safe names; this guard
+ // prevents silent corruption if they don't.
+ if (/["'\\<>{}\s]/.test(filename)) {
+ throw new Error(`Unsafe asset filename: ${JSON.stringify(filename)}`);
+ }
const ext = activePath.slice(activePath.lastIndexOf(".") + 1).toLowerCase();
const rel = `./${filename}`;📝 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
| export function buildAssetReference(activePath: string, filename: string): string { | |
| const ext = activePath.slice(activePath.lastIndexOf(".") + 1).toLowerCase(); | |
| const rel = `./${filename}`; | |
| if (ext === "html" || ext === "htm") { | |
| return `\n<img src="${rel}" alt="" />`; | |
| } | |
| if (ext === "css") { | |
| return `\n/* generated asset */\n.generated-asset { background-image: url("${rel}"); }`; | |
| } | |
| if (ext === "js" || ext === "mjs" || ext === "ts") { | |
| return `\n// generated asset\nconst ${assetVarName(filename)} = "${rel}";`; | |
| } | |
| return `\n/* asset: ${rel} */`; | |
| export function buildAssetReference(activePath: string, filename: string): string { | |
| // Reject filenames with characters that could break or inject into generated | |
| // HTML/CSS/JS snippets. Asset pipelines should produce safe names; this guard | |
| // prevents silent corruption if they don't. | |
| if (/["'\\<>{}\s]/.test(filename)) { | |
| throw new Error(`Unsafe asset filename: ${JSON.stringify(filename)}`); | |
| } | |
| const ext = activePath.slice(activePath.lastIndexOf(".") + 1).toLowerCase(); | |
| const rel = `./${filename}`; | |
| if (ext === "html" || ext === "htm") { | |
| return `\n<img src="${rel}" alt="" />`; | |
| } | |
| if (ext === "css") { | |
| return `\n/* generated asset */\n.generated-asset { background-image: url("${rel}"); }`; | |
| } | |
| if (ext === "js" || ext === "mjs" || ext === "ts") { | |
| return `\n// generated asset\nconst ${assetVarName(filename)} = "${rel}";`; | |
| } | |
| return `\n/* asset: ${rel} */`; | |
| } |
🤖 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/gamestudio/asset-reference.ts` around lines 12 - 24,
buildAssetReference interpolates filename into HTML, CSS, JavaScript, and
fallback output without validation or escaping. Validate filename before
constructing rel, rejecting unsafe characters such as quotes, backslashes, angle
brackets, and control characters, or escape it appropriately for each target
syntax; ensure assetVarName receives the validated filename and all generated
snippets remain syntactically safe.
| # --- install with pip ------------------------------------------------------ | ||
| log "installing agent-zero with pip" | ||
| cd "$AGENT_ZERO_HOME" | ||
| pip install -e . || die "pip install failed in ${AGENT_ZERO_HOME}" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## scripts/install-agent-zero.sh (outline)\n'
ast-grep outline scripts/install-agent-zero.sh --view expanded || true
printf '\n## scripts/install-agent-zero.sh (relevant lines)\n'
nl -ba scripts/install-agent-zero.sh | sed -n '1,140p'
printf '\n## Search for virtualenv / pip context\n'
rg -n "venv|virtualenv|pip install|AGENT_ZERO_HOME|break-system-packages|--user" -S scripts README.md . || trueRepository: jaylfc/taOS
Length of output: 328
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## scripts/install-agent-zero.sh\n'
sed -n '1,160p' scripts/install-agent-zero.sh
printf '\n## Search for install-agent-zero references\n'
rg -n "install-agent-zero|AGENT_ZERO_HOME|pip install -e \\.|break-system-packages|--user|venv|virtualenv" -S .Repository: jaylfc/taOS
Length of output: 49896
🌐 Web query:
pip externally managed environment --user editable install PEP 668 pip install -e --user
💡 Result:
PEP 668 introduces a mechanism where operating systems can mark a Python environment as externally managed [1][2][3]. This is intended to prevent users from accidentally installing packages via pip that might conflict with system-level packages managed by the OS package manager (e.g., apt, pacman) [1][2]. When an environment is marked as externally managed—indicated by the presence of an EXTERNALLY-MANAGED file in the standard library directory—pip will refuse to install packages into that environment [1][2]. This restriction applies to both system-wide installations and --user installations, as the latter can also lead to dependency conflicts by overriding packages on the Python sys.path [1][4]. Regarding editable installs (PEP 660) and --user installs, if you are in an externally managed environment, these commands will typically fail with the error: externally-managed-environment [5][6]. To work around this, you have the following options: 1. Use a Virtual Environment (Recommended): Create and activate a virtual environment using python -m venv . This provides an isolated space where pip management is not restricted by the system's PEP 668 marker [1][2][6]. 2. Use Specialized Tools: For installing command-line applications, tools like pipx are recommended as they automatically manage isolated environments for each application [5][6]. 3. Override the restriction: If you are in a disposable environment (such as a container or CI pipeline) where you are certain that breaking system-level package management is acceptable, you can use the --break-system-packages flag [2][6]. pip install --user --break-system-packages Note that this flag is intended for use only when strictly necessary and carries the risk of breaking OS-level tools [2][6]. For editable installs, they should be performed within a virtual environment, which is the standard practice for modern Python development [7][6].
Citations:
- 1: https://github.com/python/peps/blob/main/peps/pep-0668.rst
- 2: https://lwn.net/Articles/867657/
- 3: https://peps.python.org/pep-0668/
- 4: "pip install --user" should not be considered "Externally Managed" pypa/pip#12085
- 5: Pip should not apply PEP668 to user local installation. pypa/pip#12103
- 6: https://pydevtools.com/handbook/how-to/how-to-fix-the-externally-managed-environment-error/
- 7: https://peps.python.org/pep-0660/
Create a virtualenv before the editable install scripts/install-agent-zero.sh:37 runs against the host Python, so pip install -e . can fail on PEP 668-managed systems; --user doesn’t solve that for editable installs.
🤖 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 `@scripts/install-agent-zero.sh` at line 37, Create and activate a dedicated
virtual environment in scripts/install-agent-zero.sh before the editable
install, then run pip through that environment for the existing install step.
Update the setup flow around the pip install command to use the virtualenv’s
Python/pip and preserve the existing die failure handling.
Comment on lines
+36
to
+37
| cargo install --git "$MOLTIS_REPO" --tag "$MOLTIS_VERSION" moltis \ | ||
| || die "cargo install moltis from git failed" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
--tag fails for branch names.
cargo install --git --tag "$MOLTIS_VERSION" only accepts git tags. If a user sets TAOS_MOLTIS_VERSION to a branch name (e.g., main), the install will fail. Consider falling back to --rev or --branch when --tag fails, or document that only tag names are supported.
🔧 Suggested fix
else
log "installing moltis from git at ${MOLTIS_VERSION}"
- cargo install --git "$MOLTIS_REPO" --tag "$MOLTIS_VERSION" moltis \
- || die "cargo install moltis from git failed"
+ cargo install --git "$MOLTIS_REPO" --tag "$MOLTIS_VERSION" moltis \
+ || cargo install --git "$MOLTIS_REPO" --branch "$MOLTIS_VERSION" moltis \
+ || die "cargo install moltis from git failed"📝 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
| cargo install --git "$MOLTIS_REPO" --tag "$MOLTIS_VERSION" moltis \ | |
| || die "cargo install moltis from git failed" | |
| cargo install --git "$MOLTIS_REPO" --tag "$MOLTIS_VERSION" moltis \ | |
| || cargo install --git "$MOLTIS_REPO" --branch "$MOLTIS_VERSION" moltis \ | |
| || die "cargo install moltis from git failed" |
🤖 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 `@scripts/install-moltis.sh` around lines 36 - 37, Update the install command
in the shell script to support branch-based MOLTIS_VERSION values: attempt the
existing --tag flow, then fall back to --branch or --rev when it fails, while
preserving the existing die behavior if all attempts fail. Ensure the logic uses
the MOLTIS_REPO and MOLTIS_VERSION variables consistently.
| # --- install with pip ------------------------------------------------------ | ||
| log "installing openclaw with pip" | ||
| cd "$OPENCLAW_HOME" | ||
| pip install -e . || die "pip install failed in ${OPENCLAW_HOME}" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
pip install -e . may fail on PEP 668 systems.
Same concern as install-agent-zero.sh: on modern Linux distributions with PEP 668 enforcement, system-wide pip install will fail. Consider adding --user or using a virtual environment.
🔧 Suggested fix
- pip install -e . || die "pip install failed in ${OPENCLAW_HOME}"
+ pip install --user -e . || die "pip install failed in ${OPENCLAW_HOME}"🤖 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 `@scripts/install-openclaw.sh` at line 37, Update the installation command in
install-openclaw.sh to avoid PEP 668 failures by installing into a virtual
environment or using a user-scoped installation such as pip install --user -e .;
preserve the existing die error handling and ensure the chosen approach works
for the script’s runtime commands.
| if [[ -d build ]]; then | ||
| cd "$PICOCLAW_HOME/build" | ||
| fi | ||
| if grep -q 'install' Makefile 2>/dev/null || grep -q 'install' ../Makefile 2>/dev/null; then |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fragile install target detection.
grep -q 'install' Makefile matches any line containing "install" — including comments, variable assignments, or unrelated targets like install-docs. This could attempt sudo make install when no real install target exists, or conversely miss a target named differently. Consider using make -n install (dry-run) or matching the target definition pattern ^install: instead.
🔧 Suggested fix
- if grep -q 'install' Makefile 2>/dev/null || grep -q 'install' ../Makefile 2>/dev/null; then
+ if make -n install >/dev/null 2>&1; then
sudo make install || log "make install failed — binary may need manual PATH setup"📝 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
| if grep -q 'install' Makefile 2>/dev/null || grep -q 'install' ../Makefile 2>/dev/null; then | |
| if make -n install >/dev/null 2>&1; then |
🤖 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 `@scripts/install-picoclaw.sh` at line 52, Replace the broad grep-based
detection in the install-target check with a reliable Makefile target check,
preferably invoking make’s dry run for the `install` target or matching an exact
`^install:` definition in both candidate Makefiles. Ensure the subsequent `sudo
make install` runs only when a real install target is present, while preserving
the existing fallback behavior.
Comment on lines
+86
to
+92
| rc, _out, err = await _run( | ||
| [ | ||
| "tailscale", "up", | ||
| "--login-server", server, | ||
| "--authkey", preauth_key, | ||
| "--hostname", hostname, | ||
| ], |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does the tailscale CLI support passing the authkey via stdin, environment variable, or file instead of a command-line argument to avoid exposing it in the process list?
💡 Result:
The Tailscale CLI does not natively support reading the authkey from stdin for the tailscale up command. However, it does provide a secure mechanism to load the authkey from a file by using a specific prefix, which avoids exposing the key in the process list [1]. If you pass the --auth-key flag with a value starting with "file:", the Tailscale CLI treats the remainder of the string as a file path and reads the authkey from that file [1]. Example: tailscale up --auth-key=file:/path/to/authkey This method allows you to store the authkey in a secure location (such as a mounted secret or a restricted-permission file) and reference it without placing the sensitive credential directly on the command line where it could be visible in process monitoring tools [1]. Regarding other methods: - Environment Variables: The tailscale CLI itself does not automatically read an authkey from an environment variable like TS_AUTHKEY for the up command; this is a pattern specifically implemented by the containerboot wrapper used in official Tailscale container images [2][3][4]. - Stdin: The CLI does not support passing the authkey via stdin [1]. For users deploying in container environments, the containerboot wrapper (used in official Docker images) does support the TS_AUTHKEY environment variable, which can also be set to file:/path/to/authkey to achieve a similar security outcome [2][4].
Citations:
- 1: https://github.com/tailscale/tailscale/blob/main/cmd/tailscale/cli/up.go
- 2: https://github.com/tailscale/tailscale/blob/main/cmd/containerboot/main.go
- 3: cmd/containerboot: load auth key from file if TS_AUTHKEY_FILE is set tailscale/tailscale#16091
- 4: FR: Enable docker secrets by allowing
*_FILEenvironment variables tailscale/tailscale#14210
Pass the auth key via file: instead of the command line tailscale up supports --auth-key=file:/path/to/key, which avoids exposing the secret in ps or /proc/<pid>/cmdline.
🤖 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/taosnet/mesh.py` around lines 86 - 92, Update the tailscale
invocation in `_run` to write `preauth_key` to a securely created temporary file
and pass it using `--auth-key=file:<path>` instead of including the secret
directly in the command arguments; ensure the temporary file is cleaned up after
execution, including on errors.
Comment on lines
+170
to
+191
| from tinyagentos.worker.worker_manifest import ( | ||
| load_manifest, | ||
| SOFTWARE_TO_BACKEND_TYPE, | ||
| ) | ||
|
|
||
| # The manifest is external input: a malformed file or entry must | ||
| # degrade to "no manifest" (logged), never crash detect_backends() | ||
| # and take the whole worker down with it. | ||
| try: | ||
| manifest = load_manifest() | ||
| if manifest.get("models"): | ||
| for backend in backends: | ||
| backend_type = backend["type"] | ||
| probed_names = { | ||
| m.get("name", "") for m in backend.get("models", []) | ||
| } | ||
| available = [] | ||
| for m in manifest["models"]: | ||
| if not isinstance(m, dict): | ||
| continue | ||
| if SOFTWARE_TO_BACKEND_TYPE.get(m.get("software", "")) != backend_type: | ||
| continue |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== tinyagentos/worker/agent.py ==\n'
ast-grep outline tinyagentos/worker/agent.py --view expanded || true
printf '\n== tinyagentos/worker/worker_manifest.py ==\n'
ast-grep outline tinyagentos/worker/worker_manifest.py --view expanded || true
printf '\n== search for kokoro/whisper/backend type support ==\n'
rg -n --hidden -S 'kokoro|whisper|SOFTWARE_TO_BACKEND_TYPE|backend_type|type"\s*:\s*"(kokoro|whisper)' tinyagentos || true
printf '\n== detect_backends references ==\n'
rg -n --hidden -S 'detect_backends\(' tinyagentos || trueRepository: jaylfc/taOS
Length of output: 14118
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== tinyagentos/worker/agent.py: detect_backends body ==\n'
sed -n '112,210p' tinyagentos/worker/agent.py
printf '\n== tinyagentos/worker/worker_manifest.py ==\n'
sed -n '1,120p' tinyagentos/worker/worker_manifest.py
printf '\n== backend capability definitions and related backend types ==\n'
rg -n --hidden -S 'BACKEND_CAPABILITIES|VALID_BACKEND_TYPES|kokoro|whisper|sd-cpp|llama-cpp|ollama|rkllama' tinyagentos/worker tinyagentos/scheduler tinyagentos/routes tinyagentos/config.pyRepository: jaylfc/taOS
Length of output: 29635
Make Kokoro/Whisper manifest entries reachable.
tinyagentos/worker/worker_manifest.py advertises kokoro and whisper, but detect_backends() never probes those backend types, so those rows are always dropped and never reach the controller. Either add worker discovery/capability support for them or remove them from the manifest mapping/schema.
🤖 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/worker/agent.py` around lines 170 - 191, Make the manifest
mappings in worker_manifest.py consistent with detect_backends(): either add
discovery and capability handling for the kokoro and whisper backend types so
their entries are probed and forwarded, or remove kokoro/whisper from
SOFTWARE_TO_BACKEND_TYPE and the manifest schema. Ensure detect_backends() no
longer silently drops valid advertised entries.
Comment on lines
+183
to
+199
| probed_names = { | ||
| m.get("name", "") for m in backend.get("models", []) | ||
| } | ||
| available = [] | ||
| for m in manifest["models"]: | ||
| if not isinstance(m, dict): | ||
| continue | ||
| if SOFTWARE_TO_BACKEND_TYPE.get(m.get("software", "")) != backend_type: | ||
| continue | ||
| model_id = m.get("model_id") | ||
| if not model_id: | ||
| logger.warning( | ||
| "skipping worker-manifest entry without model_id: %r", m | ||
| ) | ||
| continue | ||
| status = "loaded" if model_id in probed_names else "available" | ||
| available.append({ |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use resident models to determine loaded status.
For rkllama and ollama, backend["models"] includes downloaded models, not only resident models. The code should use backend["loaded_models"] for those backends; otherwise available capacity is incorrectly reported as already loaded.
Proposed fix
- probed_names = {
- m.get("name", "") for m in backend.get("models", [])
- }
+ residency_models = (
+ backend.get("loaded_models", [])
+ if backend_type in ("rkllama", "ollama")
+ else backend.get("models", [])
+ )
+ probed_names = {
+ m.get("name", "") for m in residency_models
+ }📝 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
| probed_names = { | |
| m.get("name", "") for m in backend.get("models", []) | |
| } | |
| available = [] | |
| for m in manifest["models"]: | |
| if not isinstance(m, dict): | |
| continue | |
| if SOFTWARE_TO_BACKEND_TYPE.get(m.get("software", "")) != backend_type: | |
| continue | |
| model_id = m.get("model_id") | |
| if not model_id: | |
| logger.warning( | |
| "skipping worker-manifest entry without model_id: %r", m | |
| ) | |
| continue | |
| status = "loaded" if model_id in probed_names else "available" | |
| available.append({ | |
| residency_models = ( | |
| backend.get("loaded_models", []) | |
| if backend_type in ("rkllama", "ollama") | |
| else backend.get("models", []) | |
| ) | |
| probed_names = { | |
| m.get("name", "") for m in residency_models | |
| } | |
| available = [] | |
| for m in manifest["models"]: | |
| if not isinstance(m, dict): | |
| continue | |
| if SOFTWARE_TO_BACKEND_TYPE.get(m.get("software", "")) != backend_type: | |
| continue | |
| model_id = m.get("model_id") | |
| if not model_id: | |
| logger.warning( | |
| "skipping worker-manifest entry without model_id: %r", m | |
| ) | |
| continue | |
| status = "loaded" if model_id in probed_names else "available" | |
| available.append({ |
🤖 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/worker/agent.py` around lines 183 - 199, Use resident-model data
rather than downloaded-model data when computing status in the worker manifest
handling loop. For backends of type rkllama or ollama, derive probed_names from
backend["loaded_models"]; retain backend["models"] for other backends, then
continue using model_id membership to assign "loaded" versus "available".
Comment on lines
+187
to
+198
| for m in manifest["models"]: | ||
| if not isinstance(m, dict): | ||
| continue | ||
| if SOFTWARE_TO_BACKEND_TYPE.get(m.get("software", "")) != backend_type: | ||
| continue | ||
| model_id = m.get("model_id") | ||
| if not model_id: | ||
| logger.warning( | ||
| "skipping worker-manifest entry without model_id: %r", m | ||
| ) | ||
| continue | ||
| status = "loaded" if model_id in probed_names else "available" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Validate model_id is a non-empty string.
A truthy list or dictionary passes Line 193, then raises TypeError during membership testing at Line 198. The outer catch suppresses the error but drops all enrichment, including valid entries.
- if not model_id:
+ if not isinstance(model_id, str) or not model_id:📝 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
| for m in manifest["models"]: | |
| if not isinstance(m, dict): | |
| continue | |
| if SOFTWARE_TO_BACKEND_TYPE.get(m.get("software", "")) != backend_type: | |
| continue | |
| model_id = m.get("model_id") | |
| if not model_id: | |
| logger.warning( | |
| "skipping worker-manifest entry without model_id: %r", m | |
| ) | |
| continue | |
| status = "loaded" if model_id in probed_names else "available" | |
| for m in manifest["models"]: | |
| if not isinstance(m, dict): | |
| continue | |
| if SOFTWARE_TO_BACKEND_TYPE.get(m.get("software", "")) != backend_type: | |
| continue | |
| model_id = m.get("model_id") | |
| if not isinstance(model_id, str) or not model_id: | |
| logger.warning( | |
| "skipping worker-manifest entry without model_id: %r", m | |
| ) | |
| continue | |
| status = "loaded" if model_id in probed_names else "available" |
🤖 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/worker/agent.py` around lines 187 - 198, In the
manifest-processing loop, validate that `model_id` is a non-empty string before
using it in the `probed_names` membership check. Replace the current
truthiness-only guard after `m.get("model_id")` with an `isinstance(model_id,
str)` and non-empty validation, logging and skipping invalid values so malformed
entries cannot abort enrichment for valid models.
Comment on lines
+73
to
+79
| try: | ||
| data = json.loads(manifest_path.read_text()) | ||
| except (OSError, json.JSONDecodeError) as exc: | ||
| logger.warning( | ||
| "ignoring malformed worker manifest %s: %s", manifest_path, exc | ||
| ) | ||
| return empty |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- worker_manifest.py ---\n'
nl -ba tinyagentos/worker/worker_manifest.py | sed -n '1,220p'
printf '\n--- search manifest-related uses ---\n'
rg -n "load_manifest|read_text\\(|UnicodeDecodeError|json\\.loads\\(" tinyagentos -SRepository: jaylfc/taOS
Length of output: 240
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- tinyagentos/worker/worker_manifest.py ---'
nl -ba tinyagentos/worker/worker_manifest.py | sed -n '1,220p'
echo
echo '--- search manifest-related uses ---'
rg -n "load_manifest|read_text\(|UnicodeDecodeError|json\.loads\(" tinyagentos -SRepository: jaylfc/taOS
Length of output: 235
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- tinyagentos/worker/worker_manifest.py ---'
cat -n tinyagentos/worker/worker_manifest.py | sed -n '1,220p'
echo
echo '--- search manifest-related uses ---'
rg -n "load_manifest|read_text\(|UnicodeDecodeError|json\.loads\(" tinyagentos -SRepository: jaylfc/taOS
Length of output: 27133
Handle invalid manifest encodings as malformed input.
manifest_path.read_text() can raise UnicodeDecodeError before json.loads() runs, so malformed external files bypass the fail-soft path. Read as UTF-8 and catch decode failures here too.
🤖 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/worker/worker_manifest.py` around lines 73 - 79, Update the
manifest-loading try block to read the file explicitly as UTF-8 and include
UnicodeDecodeError in the exceptions handled by the existing malformed-manifest
warning path. Preserve the current logger.warning message and return empty
behavior in the manifest parsing function.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Promotion of dev to master for the 1.0.0-beta.39 release.
Includes the 16 PRs merged since beta.38: Game Studio AI textures/sprites (#1773), taOSgo Headscale mesh-join + persisted service credentials (#1770, #1772), authenticated A2A bus send (#1768), managed node-local backend services + model advertisement (#1756/1758/1760/1762), atomic VRAM check-and-reserve (#1725), audit hotfixes (#1767), rkllama 1.3.0 pin (#1764), catalog install-script fixes (#1694), and the least-privilege project-tasks agent scope (#1774). Full notes in CHANGELOG under [1.0.0-beta.39].
Summary by CodeRabbit
New Features
Bug Fixes
Documentation