Skip to content

feat(cluster): expose real-time free/used VRAM in /api/cluster/workers#1673

Closed
hognek wants to merge 2 commits into
jaylfc:masterfrom
hognek:feat/vram-realtime-worker-fields
Closed

feat(cluster): expose real-time free/used VRAM in /api/cluster/workers#1673
hognek wants to merge 2 commits into
jaylfc:masterfrom
hognek:feat/vram-realtime-worker-fields

Conversation

@hognek

@hognek hognek commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

taOS #894 slice — additive VRAM fields in worker payload

What: Adds real-time free_vram_mb and used_vram_mb to each worker in GET /api/cluster/workers. Workers report these on every heartbeat; the controller stores and surfaces them.

Why: Skald's TaosDispatcher._route_taos currently sorts candidates by hardware.gpu.vram_mb (total capacity). With real free/used VRAM, it can route to workers that actually have headroom instead of the one with the biggest total card.

What this is NOT: The full VRAM-accounted admission/eviction scheduler from #894. This is purely additive telemetry — no scheduler behavior changes.

Changes (6 files, +120 -1)

File Change
tinyagentos/cluster/worker_protocol.py +2 fields: free_vram_mb, used_vram_mb (default 0)
tinyagentos/cluster/worker_capacity.py + gpu_vram_snapshot() wraps system_stats.read_nvidia_vram()
tinyagentos/worker/agent.py heartbeat calls gpu_vram_snapshot(), sends VRAM in JSON
tinyagentos/routes/cluster.py HeartbeatBody accepts optional VRAM fields; endpoint forwards them
tinyagentos/cluster/manager.py heartbeat() stores VRAM when provided, skips when absent (legacy compat)
tests/test_routes_cluster.py +3 tests: default-zero, heartbeat-updates, legacy-survive

Backwards compatibility

  • Legacy workers that don't send VRAM fields: stored values remain unchanged (optional with None sentinel)
  • Workers registered without VRAM: default 0
  • No API breaking changes — purely additive payload fields

Tests

  • test_worker_registration_defaults_vram_to_zero — no VRAM sent → 0
  • test_heartbeat_updates_vram — heartbeat with VRAM → stored + surfaced
  • test_vram_fields_survive_heartbeat_without_vram — legacy sparse heartbeat preserves prior values
  • All 21 existing tests in test_routes_cluster.py unaffected

Verification

  • Import smoke test: WorkerInfo dataclass, gpu_vram_snapshot, asdict serialization all pass
  • Ruff: 0 new lint errors (pre-existing E402/F401 in other files unchanged)
  • gpu_vram_snapshot gracefully returns None when nvidia-smi unavailable

Summary by CodeRabbit

  • New Features

    • Added GPU VRAM reporting to worker heartbeats and cluster status updates.
    • Added a cluster endpoint to manually scan for and promote archived models.
    • Archived models can now be automatically promoted when compatible workers join.
  • Bug Fixes

    • Legacy heartbeats can omit VRAM fields without clearing previously stored values.
    • Promotion now safely skips already-installed models while cleaning up archive metadata.
    • More archived model cases are handled gracefully, including invalid entries and missing directories.

Summary by Gitar

  • New features:
    • Added an archived model promotion engine to scan for and promote compatible models when a worker joins.
    • Introduced /api/cluster/promote-archived as a manual admin endpoint to trigger promotion across all online workers.
  • Refactorings:
    • Added tinyagentos/cluster/model_archive.py to handle archive scanning, hardware requirement validation, and model file promotion.

This will update automatically on new commits.

Add model archive promotion engine: when a new worker registers with
hardware that can run previously-archived models, automatically promote
them from ~/taos/archive/models/ to the active models tree.

- model_archive.py: archive scan, hardware compatibility check,
  model promotion (move + manifest cleanup), notifications
- ClusterManager.register_worker: auto-trigger promotion on worker join
- GET /api/cluster/promote-archived: manual promotion scan endpoint
- 29 unit tests covering compatibility checks, archive I/O,
  promotable filtering, and file movement
@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown

👋 Thanks for the PR! This one targets master, which is our
stable branch (it's what live installs track). Please retarget it to
dev — click Edit next to the PR title and change the base
branch dropdown from master to dev. Your commits and any review
carry over, nothing is lost.

See CONTRIBUTING.md for the branch model.

@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds a model archive promotion system that scans a manifest-based archive, checks worker hardware compatibility, and moves promotable model files into the active tree during worker registration or via a new admin endpoint. It also adds GPU VRAM tracking fields propagated through worker heartbeats.

Changes

Model archive promotion and VRAM reporting

Layer / File(s) Summary
Model archive module
tinyagentos/cluster/model_archive.py
Implements archive root resolution, manifest listing, worker compatibility checks, promotable filtering, file-move promotion, and async batch promotion with optional notifications.
Worker registration wiring and tests
tinyagentos/cluster/manager.py, tests/test_model_archive.py
register_worker triggers promote_compatible_models with exceptions caught and logged; new tests cover compatibility checks, manifest listing, promotable filtering, promotion filesystem behavior, and archive root resolution.
Admin promote-archived route
tinyagentos/routes/cluster.py
Adds GET /api/cluster/promote-archived scanning online workers, promoting compatible archived models, emitting notifications, and returning a summary.
VRAM fields in WorkerInfo and heartbeat handling
tinyagentos/cluster/worker_protocol.py, tinyagentos/cluster/manager.py, tinyagentos/routes/cluster.py, tests/test_cluster_worker_protocol.py
Adds free_vram_mb/used_vram_mb fields to WorkerInfo, extends ClusterManager.heartbeat and HeartbeatBody/route to accept and apply them, with tests for defaults, serialization, roundtrip, and persistence across heartbeats.
Worker-side GPU VRAM snapshot capture
tinyagentos/cluster/worker_capacity.py, tinyagentos/worker/agent.py
Adds gpu_vram_snapshot() probing NVIDIA VRAM and computing free/used values; WorkerAgent.heartbeat captures and includes the snapshot in its payload.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant WorkerAgent
  participant ClusterRoute
  participant ClusterManager
  participant ModelArchive
  participant FileSystem
  participant Notifications

  WorkerAgent->>ClusterRoute: POST heartbeat(free_vram_mb, used_vram_mb)
  ClusterRoute->>ClusterManager: heartbeat(..., free_vram_mb, used_vram_mb)
  ClusterManager->>ClusterManager: store VRAM values on worker

  WorkerAgent->>ClusterManager: register_worker(info)
  ClusterManager->>ModelArchive: promote_compatible_models(hardware, name)
  ModelArchive->>FileSystem: list_archived_models()
  ModelArchive->>ModelArchive: _worker_can_run() check per manifest
  ModelArchive->>FileSystem: promote_model() moves files, removes manifest
  ModelArchive->>Notifications: emit_event("model.promoted")
  ModelArchive-->>ClusterManager: promoted model ids
Loading

Possibly related PRs

  • jaylfc/taOS#675: Both PRs modify the same archived-model promotion logic in tinyagentos/cluster/model_archive.py and the /api/cluster/promote-archived route.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main additive change: exposing real-time VRAM data in cluster worker responses.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

req_npu = requirements.get("npu_type")
if req_npu:
worker_npu_type = hw_npu.get("type", "none") or "none"
if worker_npu_type != req_npu and worker_npu_type not in (req_npu,):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: Dead/redundant clause in NPU check.

if worker_npu_type != req_npu and worker_npu_type not in (req_npu,):

worker_npu_type not in (req_npu,) is logically identical to worker_npu_type != req_npu (a 1-tuple membership test equals equality on its single element). It is dead code that obscures intent and should be removed.

Suggested change
if worker_npu_type != req_npu and worker_npu_type not in (req_npu,):
if worker_npu_type != req_npu:
return False

Reply with @kilocode-bot fix it to have Kilo Code address this issue.

level="info",
)

# Promote any archived models this worker can now run

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: promote_compatible_models is invoked from an async def but runs blocking filesystem I/O (Path.is_dir, Path.glob, json.loads, shutil.move) on the event loop.

register_worker is async def and promote_compatible_models is async, but its body delegates to find_promotable -> list_archived_models (glob + read every manifest) and promote_model (shutil.move + unlink) — all synchronous, all blocking. With a non-trivial archive and large model files this will stall the controller's event loop and delay heartbeats, other registrations, and request handling.

Recommend wrapping the scan and move in asyncio.to_thread(...) (or loop.run_in_executor) and keeping only the notifications.emit_event awaited.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

promoted_by_worker: dict[str, list[str]] = {}
total = 0

for w in online:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: find_promotable / promote_model run synchronously inside an async def route, performing O(online_workers * archived_models) filesystem reads plus shutil.move of model files.

promote_archived_models loops over every online worker, and each iteration calls find_promotable (which re-reads every manifest) followed by promote_model (shutil.move of model files) inline in the async handler. A single request can therefore block the event loop for the duration of multi-GB copies and time out under realistic loads.

Recommend offloading archive scan + movement to await asyncio.to_thread(...) and deduping list_archived_models across workers (e.g. read the archive once, then per-worker filter).


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

return JSONResponse({"error": str(exc)}, status_code=502)


@router.get("/api/cluster/promote-archived")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SUGGESTION: The new /api/cluster/promote-archived endpoint appears to be unauthenticated, while it performs filesystem-level shutil.move operations and emits notifications.

Other endpoints in this file are gated by a shared/worker token; adding an un-authenticated route that can move files into the active models tree and emit admin-visible notifications expands the cluster's external attack surface. Consider gating this route behind the same auth scheme used by sibling endpoints (or at minimum an admin token), even though it's "intended" for the admin UI.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

"""
promotable: list[dict] = []
for model in list_archived_models(archive_dir):
reqs = model.get("requirements") or {}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: find_promotable mutates the manifest dict in place by assigning model["worker_name"] = worker_name.

Today this is "safe" only because list_archived_models rebuilds each dict from JSON every call. The instant anyone caches list_archived_models results (an obvious perf win given the per-call I/O cost), the same dict reference gets worker_name re-stamped across calls and promote_model will silently mis-attribute the promotion. Make ownership unambiguous with a copy:

Suggested change
reqs = model.get("requirements") or {}
promotable.append({**model, "worker_name": worker_name})

Reply with @kilocode-bot fix it to have Kilo Code address this issue.


if not model_files_dir.is_dir():
logger.warning(
"model_archive: model files directory %s not found — "

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: promote_model deletes the archive manifest before confirming the archive files are moved, and is not crash-safe.

When target_dir.exists(), the code unlink()s the manifest while leaving the archive files directory completely untouched. If target_dir is a stale empty stub from a previous failure, the model files are silently orphaned in the archive forever (no manifest, no move). More generally, shutil.move then manifest_path.unlink() is not atomic across crashes — a power loss between the two leaves files in archive with no manifest.

Recommend: (1) log a warning when the target pre-existed so the operator knows files were left behind, (2) move manifest deletion into the same try block that already wraps shutil.move, and re-create the manifest on failure, or (3) move into a temp dir, fsync, then atomically rename into place + delete the manifest.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

caps = self.detect_capabilities(backends)
kv_quant = self.detect_kv_quant_support(backends)
snap = capacity_snapshot()
vram = gpu_vram_snapshot()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: Heartbeat now blocks on read_nvidia_vram() (typically a shell-out to nvidia-smi) with no timeout.

gpu_vram_snapshot() is invoked inline in heartbeat(). If the NVIDIA driver is faulted or NVML hangs, the probe stalls until the OS-level subprocess times out (or never). Meanwhile the controller will mark this worker offline and trigger a re-register storm.

The docstring promises "a missing nvidia-smi or a probe timeout is not an error" but no timeout is actually enforced in this PR. Wrap the probe with asyncio.wait_for / subprocess timeout, or run it in asyncio.to_thread with a watchdog.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

available or the probe fails.

Called from the worker heartbeat loop alongside capacity_snapshot().
Best-effort: a missing nvidia-smi or a probe timeout is not an error.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: gpu_vram_snapshot exposes VRAM for "the first GPU" only — multi-GPU hosts are silently mis-reported.

read_nvidia_vram() returns (used_mb, total_mb) for GPU 0. The agent heartbeat then publishes this as the worker's total free/used VRAM. On a host where GPU 0 is saturated but GPU 1 is idle, Skald's TaosDispatcher will route to this worker under the false impression it has headroom, and ignore workers that actually do.

Either aggregate across all GPUs (sum free, sum used) and expose the totals, or document the limit and surface free_vram_mb as max across GPUs so the router picks "does this worker have at least one usable card?"


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

@hognek hognek force-pushed the feat/vram-realtime-worker-fields branch from 64d530b to 893ed7b Compare July 6, 2026 15:08
taOS jaylfc#894 slice — additive data, no scheduler behavior change.

Worker side (worker/agent.py, worker_capacity.py):
- Add gpu_vram_snapshot() to worker_capacity that reads real-time
  free/used VRAM via system_stats.read_nvidia_vram()
- Worker heartbeat now samples VRAM and includes free_vram_mb +
  used_vram_mb in the heartbeat JSON payload

Controller side (routes/cluster.py, manager.py, worker_protocol.py):
- WorkerInfo gains free_vram_mb and used_vram_mb fields (default 0)
- HeartbeatBody accepts optional free_vram_mb / used_vram_mb
- ClusterManager.heartbeat() stores VRAM when provided; absent
  values leave existing stored values unchanged (legacy compat)
- GET /api/cluster/workers surfaces both fields via asdict()

Tests (test_routes_cluster.py):
- 3 new tests: default-zero, heartbeat updates, legacy-survive

Skald's TaosDispatcher._route_taos can now sort candidates by
actual free VRAM instead of hardware.gpu.vram_mb (total capacity).
@hognek hognek force-pushed the feat/vram-realtime-worker-fields branch from 893ed7b to 86d0012 Compare July 6, 2026 15:08
@gitar-bot

gitar-bot Bot commented Jul 6, 2026

Copy link
Copy Markdown

Gitar is working

Gitar

return JSONResponse({"error": str(exc)}, status_code=502)


@router.get("/api/cluster/promote-archived")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: GET with mutating side effects — violates HTTP semantics and risks duplicate execution.

@router.get("/api/cluster/promote-archived") triggers shutil.move of multi-GB model files and emits notifications. GET requests are expected to be safe and idempotent; intermediaries (CDNs, browsers, proxies, link previews) can cache or prefetch GET responses, potentially re-triggering the promotion and double-emitting model.promoted notifications. Should be @router.post(...) (or PATCH) so the verb signals mutating intent and is not cached/refetched.

model_id,
)

return {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: Endpoint silently reports success on partial failure — no error visibility for callers.

promote_model() swallows OSError/shutil.Error and returns False, but the route only counts successful promotions and discards failures. A caller cannot tell whether promoted: 0 means "nothing was promotable" vs "everything failed". Consider returning per-worker failure counts (or a failures array with model_id + reason) so the admin UI / CLI can surface actionable errors when a worker should have been able to run a model but the move failed (permissions, disk full, cross-device, etc).

@kilo-code-bot

kilo-code-bot Bot commented Jul 6, 2026

Copy link
Copy Markdown

Code Review Summary

Status: 2 New Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 2
SUGGESTION 0

The 8 issues from the previous review pass (NPU dead clause, blocking I/O on the event loop in register_worker and /api/cluster/promote-archived, manifest mutation in find_promotable, non-crash-safe promote_model, unauthenticated admin endpoint, read_nvidia_vram blocking the heartbeat, multi-GPU reporting only GPU 0) remain unaddressed and continue to apply to current HEAD 86d0012a. Two additional findings below complement them.

Issue Details (click to expand)

WARNING

File Line Issue
tinyagentos/routes/cluster.py 669 GET /api/cluster/promote-archived performs mutating shutil.move of multi-GB model files and emits model.promoted notifications. GET is expected to be safe/idempotent; intermediaries may cache or prefetch and re-trigger the operation. Should be POST (or PATCH).
tinyagentos/routes/cluster.py 716 Endpoint returns {"promoted": N, ...} even when every promote_model() call silently failed. No failures array or per-worker error count is surfaced, so the admin UI cannot distinguish "nothing promotable" from "everything failed" (permissions, disk full, cross-device move, etc).
Files Reviewed (8 files)
  • tests/test_cluster_worker_protocol.py - 0 new issues
  • tests/test_model_archive.py - 0 new issues
  • tinyagentos/cluster/manager.py - 0 new issues (existing blocking-I/O + auto-promotion concerns already flagged)
  • tinyagentos/cluster/model_archive.py - 0 new issues (NPU clause, manifest mutation, non-crash-safe promote_model already flagged)
  • tinyagentos/cluster/worker_capacity.py - 0 new issues (first-GPU-only already flagged)
  • tinyagentos/cluster/worker_protocol.py - 0 new issues
  • tinyagentos/routes/cluster.py - 2 new issues
  • tinyagentos/worker/agent.py - 0 new issues (heartbeat blocking on read_nvidia_vram already flagged)

Fix these issues in Kilo Cloud


Reviewed by minimax-m3 · Input: 40.4K · Output: 5.5K · Cached: 245.2K

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (4)
tinyagentos/worker/agent.py (1)

396-404: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Blocking subprocess call on the event loop.

gpu_vram_snapshot() (via read_nvidia_vram()) uses subprocess.run(..., timeout=3), a blocking call invoked directly in the async heartbeat() coroutine. This can stall the event loop for up to 3s per heartbeat cycle (every 5s), delaying any other coroutines sharing this loop (e.g. a local worker HTTP server on worker_port).

Offload it to a thread to keep the loop responsive:

♻️ Proposed fix
-            snap = capacity_snapshot()
-            vram = gpu_vram_snapshot()
+            snap = capacity_snapshot()
+            vram = await asyncio.to_thread(gpu_vram_snapshot)
🤖 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 396 - 404, The heartbeat coroutine
is calling gpu_vram_snapshot() synchronously, which can block the event loop via
read_nvidia_vram() and subprocess.run. Update
tinyagentos.worker.agent.Agent.heartbeat to offload the gpu_vram_snapshot() work
to a thread (for example alongside the existing capacity_snapshot flow) so the
async loop stays responsive; keep the change localized to the load/capacity
collection path and preserve the existing backend/capability detection behavior.
tinyagentos/cluster/model_archive.py (2)

199-213: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Non-empty archive files dir is left behind when target already exists.

When target_dir exists you unlink the manifest and only rmdir the files dir if it's empty; a populated model_files_dir is silently orphaned in the archive with no manifest referencing it. Consider shutil.rmtree (or logging) so stale archive files don't accumulate.

🤖 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/cluster/model_archive.py` around lines 199 - 213, The
target-exists branch in model_archive handling leaves populated archive files
behind because it only calls model_files_dir.rmdir() when the directory is
empty. Update the existing target_dir.exists() path in model_archive to remove
the archive contents more robustly, using shutil.rmtree on model_files_dir or
equivalent cleanup, and keep the manifest unlink behavior so stale files do not
accumulate.

128-132: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Redundant NPU condition.

worker_npu_type not in (req_npu,) is equivalent to worker_npu_type != req_npu, so the and clause is a tautology.

♻️ Simplify
     if req_npu:
         worker_npu_type = hw_npu.get("type", "none") or "none"
-        if worker_npu_type != req_npu and worker_npu_type not in (req_npu,):
+        if worker_npu_type != req_npu:
             return False
🤖 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/cluster/model_archive.py` around lines 128 - 132, The NPU
compatibility check in model_archive.py is redundant because `worker_npu_type
not in (req_npu,)` duplicates the `worker_npu_type != req_npu` comparison.
Simplify the `req_npu` branch in `model_archive.py` by keeping a single
inequality check in the `worker_npu_type` comparison, preserving the same
behavior while removing the tautological `and` condition.
tests/test_model_archive.py (1)

140-147: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Apple-silicon test doesn't exercise the unified-memory branch.

With vram_mb=16384 already ≥ min_vram_mb=8192, the check passes regardless of the max(worker_vram, hw_ram) logic. To actually cover the unified-memory path, set vram_mb below the requirement and rely on ram_mb to satisfy it (e.g. vram_mb=0, ram_mb=16384).

🤖 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_model_archive.py` around lines 140 - 147, The Apple-silicon test
currently passes without exercising the unified-memory logic in _worker_can_run.
Update test_apple_silicon_unified_memory so the worker GPU vram_mb is below the
min_vram_mb requirement and the assertion depends on ram_mb satisfying it via
the unified-memory path. Keep the test focused on the aarch64 Apple hardware
case and verify that the max(worker_vram, hw_ram) behavior is what makes the
result True.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tinyagentos/cluster/model_archive.py`:
- Line 215: Offload the synchronous model promotion work in promote_model() so
it does not block the event loop when called from ClusterManager.register_worker
or GET /api/cluster/promote-archived. Move the filesystem-heavy logic around
shutil.move in model_archive.py into a thread using asyncio.to_thread() or
run_in_executor(), keeping the async callers responsive during large archive
promotions.

In `@tinyagentos/routes/cluster.py`:
- Around line 669-670: The promote_archived_models endpoint is mutating state
but is currently exposed as a GET route, which should be safe and non-mutating.
Update the route decorator on promote_archived_models to use POST instead of
GET, and keep the existing file-moving/manifests-deleting behavior behind that
method so it is not triggerable via browser prefetch or CSRF-prone GET requests.

---

Nitpick comments:
In `@tests/test_model_archive.py`:
- Around line 140-147: The Apple-silicon test currently passes without
exercising the unified-memory logic in _worker_can_run. Update
test_apple_silicon_unified_memory so the worker GPU vram_mb is below the
min_vram_mb requirement and the assertion depends on ram_mb satisfying it via
the unified-memory path. Keep the test focused on the aarch64 Apple hardware
case and verify that the max(worker_vram, hw_ram) behavior is what makes the
result True.

In `@tinyagentos/cluster/model_archive.py`:
- Around line 199-213: The target-exists branch in model_archive handling leaves
populated archive files behind because it only calls model_files_dir.rmdir()
when the directory is empty. Update the existing target_dir.exists() path in
model_archive to remove the archive contents more robustly, using shutil.rmtree
on model_files_dir or equivalent cleanup, and keep the manifest unlink behavior
so stale files do not accumulate.
- Around line 128-132: The NPU compatibility check in model_archive.py is
redundant because `worker_npu_type not in (req_npu,)` duplicates the
`worker_npu_type != req_npu` comparison. Simplify the `req_npu` branch in
`model_archive.py` by keeping a single inequality check in the `worker_npu_type`
comparison, preserving the same behavior while removing the tautological `and`
condition.

In `@tinyagentos/worker/agent.py`:
- Around line 396-404: The heartbeat coroutine is calling gpu_vram_snapshot()
synchronously, which can block the event loop via read_nvidia_vram() and
subprocess.run. Update tinyagentos.worker.agent.Agent.heartbeat to offload the
gpu_vram_snapshot() work to a thread (for example alongside the existing
capacity_snapshot flow) so the async loop stays responsive; keep the change
localized to the load/capacity collection path and preserve the existing
backend/capability detection 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: 0c40014f-3a7b-45ea-a11d-dee2dcd23696

📥 Commits

Reviewing files that changed from the base of the PR and between 7695783 and 64d530b.

📒 Files selected for processing (8)
  • tests/test_model_archive.py
  • tests/test_routes_cluster.py
  • tinyagentos/cluster/manager.py
  • tinyagentos/cluster/model_archive.py
  • tinyagentos/cluster/worker_capacity.py
  • tinyagentos/cluster/worker_protocol.py
  • tinyagentos/routes/cluster.py
  • tinyagentos/worker/agent.py

pass
return True

shutil.move(str(model_files_dir), str(target_dir))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== model_archive.py outline ==\n'
ast-grep outline tinyagentos/cluster/model_archive.py --view expanded || true

printf '\n== register_worker / promote-archived references ==\n'
rg -n "promote_model|register_worker|promote-archived|shutil\.move|to_thread|run_in_executor" tinyagentos -S

printf '\n== model_archive.py relevant region ==\n'
sed -n '150,260p' tinyagentos/cluster/model_archive.py | cat -n

printf '\n== cluster manager relevant region ==\n'
sed -n '1,260p' tinyagentos/cluster/cluster_manager.py | cat -n

printf '\n== routes relevant region ==\n'
sed -n '1,260p' tinyagentos/cluster/routes.py | cat -n

Repository: jaylfc/taOS

Length of output: 10161


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== cluster-related files ==\n'
git ls-files 'tinyagentos/cluster/*' 'tinyagentos/routes/cluster.py' | sort

printf '\n== manager.py outline ==\n'
ast-grep outline tinyagentos/cluster/manager.py --view expanded || true

printf '\n== cluster.py outline ==\n'
ast-grep outline tinyagentos/routes/cluster.py --view expanded || true

printf '\n== manager.py register_worker region ==\n'
sed -n '1,180p' tinyagentos/cluster/manager.py | cat -n

printf '\n== cluster route promote/register regions ==\n'
sed -n '100,220p' tinyagentos/routes/cluster.py | cat -n
sed -n '660,725p' tinyagentos/routes/cluster.py | cat -n

Repository: jaylfc/taOS

Length of output: 22709


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== model_archive.py top ==\n'
sed -n '1,120p' tinyagentos/cluster/model_archive.py | cat -n

printf '\n== root helpers references ==\n'
rg -n "_archive_root|_active_models_root|TAOS|models|archive" tinyagentos/cluster/model_archive.py tinyagentos -S

printf '\n== any custom model root config ==\n'
rg -n "active models|archive root|models root|archive_dir|TAOS_.*MODEL|MODEL.*ROOT|ARCHIVE.*ROOT" tinyagentos -S

Repository: jaylfc/taOS

Length of output: 50368


Offload model promotion off the event loop. promote_model() does synchronous filesystem work and is awaited from ClusterManager.register_worker and GET /api/cluster/promote-archived; large archive moves can block worker registration and heartbeats, especially when shutil.move() falls back to copying across filesystems. Wrap the promotion work in asyncio.to_thread()/run_in_executor() from the async path.

🤖 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/cluster/model_archive.py` at line 215, Offload the synchronous
model promotion work in promote_model() so it does not block the event loop when
called from ClusterManager.register_worker or GET /api/cluster/promote-archived.
Move the filesystem-heavy logic around shutil.move in model_archive.py into a
thread using asyncio.to_thread() or run_in_executor(), keeping the async callers
responsive during large archive promotions.

Comment on lines +669 to +670
@router.get("/api/cluster/promote-archived")
async def promote_archived_models(request: Request):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

State-mutating operation exposed as GET.

promote-archived moves files and deletes manifests, but GET is expected to be safe/idempotent. Exposing it as GET makes it triggerable by browser prefetch, crawlers, or cross-site requests (CSRF) without a request body. Use @router.post instead.

🔒 Suggested change
-@router.get("/api/cluster/promote-archived")
+@router.post("/api/cluster/promote-archived")
 async def promote_archived_models(request: Request):
📝 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
@router.get("/api/cluster/promote-archived")
async def promote_archived_models(request: Request):
`@router.post`("/api/cluster/promote-archived")
async def promote_archived_models(request: Request):
🤖 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/cluster.py` around lines 669 - 670, The
promote_archived_models endpoint is mutating state but is currently exposed as a
GET route, which should be safe and non-mutating. Update the route decorator on
promote_archived_models to use POST instead of GET, and keep the existing
file-moving/manifests-deleting behavior behind that method so it is not
triggerable via browser prefetch or CSRF-prone GET requests.

@jaylfc

jaylfc commented Jul 6, 2026

Copy link
Copy Markdown
Owner

Thanks Hogne — this landed on dev already: I folded your VRAM-fields commit into #1680 (the GPU-lease PR) so your authorship is preserved in the history. Closing this as merged-via-#1680. The free_vram_mb/used_vram_mb heartbeat fields are live on dev now, which #1683 builds on.

@jaylfc jaylfc closed this Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants