feat(skald): sidecar manifest parser + available_models protocol#1
Closed
hognek wants to merge 2 commits into
Closed
feat(skald): sidecar manifest parser + available_models protocol#1hognek wants to merge 2 commits into
hognek wants to merge 2 commits into
Conversation
added 2 commits
July 6, 2026 16:00
…ls protocol - tests/test_skald_manifest.py: 15 tests for load_manifest(), SOFTWARE_TO_BACKEND_TYPE mapping, env var override, missing file, invalid JSON, and edge cases - tests/test_cluster_worker_protocol.py: 9 tests for WorkerInfo available_models field defaults/serialisation/roundtrip, plus 4 tests for ClusterManager.heartbeat() available_models derivation from backends (dedup, additive, empty backends) All 36 new tests pass, 0 regressions in 16 existing cluster tests. Refs: t_634725d4
- tinyagentos/worker/skald_manifest.py: load_manifest() parses the sidecar Taos.md sidecar into a Backend dict; SOFTWARE_TO_BACKEND_TYPE map - tinyagentos/cluster/worker_protocol.py: WorkerInfo gains available_models field (list[str]) - tinyagentos/cluster/manager.py: heartbeat() derives available_models from worker backends (dedup, additive, empty-backends safe) - tinyagentos/worker/agent.py: SkaldProactiveAgent loads manifest on init, exposes available_models for ClusterManager heartbeat Refs: t_634725d4
📝 WalkthroughWalkthroughAdds a new ChangesSkald manifest integration for worker available models
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant WorkerAgent
participant SkaldManifest
participant ClusterManager
participant WorkerInfo
WorkerAgent->>WorkerAgent: probe backends and models
WorkerAgent->>SkaldManifest: load_manifest()
SkaldManifest-->>WorkerAgent: manifest models
WorkerAgent->>WorkerAgent: mark models loaded/available, attach available_models
WorkerAgent->>ClusterManager: heartbeat(backends)
ClusterManager->>ClusterManager: flatten & dedupe available_models by model_id
ClusterManager->>WorkerInfo: update available_models
Suggested labels: enhancement, worker, cluster Suggested reviewers: hognek 🐰 A manifest whispers which models can run, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7d58e6e. Configure here.
| "status": status, | ||
| }) | ||
| if available: | ||
| backend["available_models"] = available |
There was a problem hiding this comment.
Manifest ignores backend port
Medium Severity
Sidecar entries are attached to every probed backend whose type matches SOFTWARE_TO_BACKEND_TYPE, without comparing manifest port/health_url to the backend URL. Multiple llama-cpp instances or Skald services on non-default ports can get the wrong models, ports, and health URLs on the cluster view.
Reviewed by Cursor Bugbot for commit 7d58e6e. Configure here.
| for m in manifest["models"]: | ||
| if SOFTWARE_TO_BACKEND_TYPE.get(m.get("software", "")) != backend_type: | ||
| continue | ||
| model_id = m["model_id"] |
There was a problem hiding this comment.
Missing model_id crashes probe
Medium Severity
Manifest enrichment uses m["model_id"] without a fallback. A valid JSON manifest with a model object missing model_id raises KeyError inside detect_backends, breaking registration and heartbeats until the file is fixed.
Reviewed by Cursor Bugbot for commit 7d58e6e. Configure here.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tinyagentos/worker/agent.py (1)
26-31: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
kokoro/whispermanifest entries are currently unreachableSOFTWARE_TO_BACKEND_TYPEmaps them intinyagentos/worker/skald_manifest.py, butWorkerAgent.detect_backends()never probes those backend types, and they also aren’t accepted as valid backend types elsewhere. Either add the corresponding backend support or drop these manifest mappings until they can attach to a real backend.🤖 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 26 - 31, The `kokoro` and `whisper` manifest mappings are unreachable because `WorkerAgent.detect_backends()` does not probe those backend types and the backend validation path still rejects them. Update the backend detection/acceptance flow in `WorkerAgent.detect_backends()` and the related backend-type validation so these symbols are recognized end-to-end, or remove the `SOFTWARE_TO_BACKEND_TYPE` entries in `skald_manifest.py` until real support exists.
🧹 Nitpick comments (2)
tinyagentos/worker/agent.py (2)
138-138: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueManifest is re-read from disk and re-parsed on every heartbeat tick.
detect_backends()runs every ~5s viaheartbeat()and callsload_manifest()each time, doing a disk stat + read + JSON parse even though the sidecar manifest rarely changes. Consider caching the parsed result keyed on the file's mtime to avoid unnecessary I/O on this hot 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/worker/agent.py` at line 138, The heartbeat path in detect_backends() is reloading the manifest from disk on every tick, which is unnecessary overhead. Update load_manifest() in agent.py to cache the parsed manifest using the file’s mtime and reuse it when unchanged, and have detect_backends() continue calling that cached loader so heartbeat() avoids repeated stat/read/JSON parse work.
133-162: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo test coverage for the manifest-enrichment branch of
detect_backends().Manifest parsing (
skald_manifest.py) and cluster propagation (manager.py/worker_protocol.py) both have dedicated tests, but the mapping/loaded-vs-available logic added here (lines 133-162) isn't covered by any provided test file. Given the branching (backend-type matching, loaded-vs-available status derivation, empty-manifest handling), this seems worth a dedicated unit 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 `@tinyagentos/worker/agent.py` around lines 133 - 162, The manifest-enrichment branch in detect_backends() is missing unit coverage, especially the backend-type filtering and loaded-vs-available status logic. Add a focused test for tinyagentos.worker.agent.detect_backends that mocks load_manifest() and backends to verify matching SOFTWARE_TO_BACKEND_TYPE entries populate available_models with the correct fields and status values. Include cases for a loaded model, an available-but-not-loaded model, and an empty or missing manifest.models path to confirm no enrichment happens when appropriate.
🤖 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/worker/agent.py`:
- Around line 133-162: The manifest enrichment in detect_backends() can throw
and break register() when sidecar data is malformed or mid-write. Add local
exception handling around load_manifest() and the per-model access in this
block, and make missing or invalid manifest entries fail soft by skipping
enrichment instead of propagating. Keep the existing backend loop and
available_models shaping intact, but ensure register() and the run()
registration path stay resilient even if manifest parsing or model_id lookup
fails.
---
Outside diff comments:
In `@tinyagentos/worker/agent.py`:
- Around line 26-31: The `kokoro` and `whisper` manifest mappings are
unreachable because `WorkerAgent.detect_backends()` does not probe those backend
types and the backend validation path still rejects them. Update the backend
detection/acceptance flow in `WorkerAgent.detect_backends()` and the related
backend-type validation so these symbols are recognized end-to-end, or remove
the `SOFTWARE_TO_BACKEND_TYPE` entries in `skald_manifest.py` until real support
exists.
---
Nitpick comments:
In `@tinyagentos/worker/agent.py`:
- Line 138: The heartbeat path in detect_backends() is reloading the manifest
from disk on every tick, which is unnecessary overhead. Update load_manifest()
in agent.py to cache the parsed manifest using the file’s mtime and reuse it
when unchanged, and have detect_backends() continue calling that cached loader
so heartbeat() avoids repeated stat/read/JSON parse work.
- Around line 133-162: The manifest-enrichment branch in detect_backends() is
missing unit coverage, especially the backend-type filtering and
loaded-vs-available status logic. Add a focused test for
tinyagentos.worker.agent.detect_backends that mocks load_manifest() and backends
to verify matching SOFTWARE_TO_BACKEND_TYPE entries populate available_models
with the correct fields and status values. Include cases for a loaded model, an
available-but-not-loaded model, and an empty or missing manifest.models path to
confirm no enrichment happens when appropriate.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d426cc5d-bdff-43d1-a62a-670d0073d45c
📒 Files selected for processing (6)
tests/test_cluster_worker_protocol.pytests/test_skald_manifest.pytinyagentos/cluster/manager.pytinyagentos/cluster/worker_protocol.pytinyagentos/worker/agent.pytinyagentos/worker/skald_manifest.py
Comment on lines
+133
to
+162
| from tinyagentos.worker.skald_manifest import ( | ||
| load_manifest, | ||
| SOFTWARE_TO_BACKEND_TYPE, | ||
| ) | ||
|
|
||
| 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 SOFTWARE_TO_BACKEND_TYPE.get(m.get("software", "")) != backend_type: | ||
| continue | ||
| model_id = m["model_id"] | ||
| status = "loaded" if model_id in probed_names else "available" | ||
| available.append({ | ||
| "model_id": model_id, | ||
| "capability": m.get("capability", ""), | ||
| "software": m.get("software", ""), | ||
| "port": m.get("port", 0), | ||
| "vram_required_gb": m.get("vram_required_gb", 0.0), | ||
| "health_url": m.get("health_url", ""), | ||
| "status": status, | ||
| }) | ||
| if available: | ||
| backend["available_models"] = available | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Manifest enrichment can crash register() on malformed/racy sidecar data.
This block has no exception handling, and both indexing (m["model_id"]) and file parsing failures propagate uncaught. heartbeat() (line 434) wraps its entire backend probe in try/except and safely returns 0 on failure, but register() (line 378) calls detect_backends() directly with no protection, and run()'s registration branch (lines 474-479) doesn't guard it either. A malformed or transiently-rewritten manifest file (e.g. JSONDecodeError, a missing model_id key, or a non-dict JSON root) would propagate an unhandled exception out of register(), potentially breaking the worker's registration loop entirely.
🛡️ Proposed fix: contain manifest failures locally
- from tinyagentos.worker.skald_manifest import (
- load_manifest,
- SOFTWARE_TO_BACKEND_TYPE,
- )
-
- manifest = load_manifest()
- if manifest.get("models"):
+ from tinyagentos.worker.skald_manifest import (
+ load_manifest,
+ SOFTWARE_TO_BACKEND_TYPE,
+ )
+
+ try:
+ manifest = load_manifest()
+ except Exception:
+ manifest = {}
+
+ if isinstance(manifest, dict) and 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 SOFTWARE_TO_BACKEND_TYPE.get(m.get("software", "")) != backend_type:
continue
- model_id = m["model_id"]
+ model_id = m.get("model_id")
+ if not model_id:
+ continue
status = "loaded" if model_id in probed_names else "available"📝 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
| from tinyagentos.worker.skald_manifest import ( | |
| load_manifest, | |
| SOFTWARE_TO_BACKEND_TYPE, | |
| ) | |
| 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 SOFTWARE_TO_BACKEND_TYPE.get(m.get("software", "")) != backend_type: | |
| continue | |
| model_id = m["model_id"] | |
| status = "loaded" if model_id in probed_names else "available" | |
| available.append({ | |
| "model_id": model_id, | |
| "capability": m.get("capability", ""), | |
| "software": m.get("software", ""), | |
| "port": m.get("port", 0), | |
| "vram_required_gb": m.get("vram_required_gb", 0.0), | |
| "health_url": m.get("health_url", ""), | |
| "status": status, | |
| }) | |
| if available: | |
| backend["available_models"] = available | |
| from tinyagentos.worker.skald_manifest import ( | |
| load_manifest, | |
| SOFTWARE_TO_BACKEND_TYPE, | |
| ) | |
| try: | |
| manifest = load_manifest() | |
| except Exception: | |
| manifest = {} | |
| if isinstance(manifest, dict) and 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 SOFTWARE_TO_BACKEND_TYPE.get(m.get("software", "")) != backend_type: | |
| continue | |
| model_id = m.get("model_id") | |
| if not model_id: | |
| continue | |
| status = "loaded" if model_id in probed_names else "available" | |
| available.append({ | |
| "model_id": model_id, | |
| "capability": m.get("capability", ""), | |
| "software": m.get("software", ""), | |
| "port": m.get("port", 0), | |
| "vram_required_gb": m.get("vram_required_gb", 0.0), | |
| "health_url": m.get("health_url", ""), | |
| "status": status, | |
| }) | |
| if available: | |
| backend["available_models"] = 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 133 - 162, The manifest enrichment
in detect_backends() can throw and break register() when sidecar data is
malformed or mid-write. Add local exception handling around load_manifest() and
the per-model access in this block, and make missing or invalid manifest entries
fail soft by skipping enrichment instead of propagating. Keep the existing
backend loop and available_models shaping intact, but ensure register() and the
run() registration path stay resilient even if manifest parsing or model_id
lookup fails.
hognek
added a commit
that referenced
this pull request
Jul 18, 2026
…cel arbiter on stale-update-available, gate online event - worker/agent.py: store lifecycle_status/lifecycle_reason in WorkerAgent, resend on periodic heartbeats so controller restart doesn't revert draining/updating workers to 'online' (CodeRabbit MAJOR #1) - manager.py: cancel GPU arbiter tasks on stale-update-available worker timeout, matching the cancellation already done for stale-drain and stale-update paths (CodeRabbit MAJOR #2) - manager.py: gate recovery 'worker.online' event on worker.status == 'online' to suppress false events for workers recovering into draining/updating state (CodeRabbit MINOR jaylfc#3) - tests/test_worker.py: accept **kwargs in fake_heartbeat mocks to match expanded heartbeat() signature
hognek
added a commit
that referenced
this pull request
Jul 18, 2026
…aylfc#890 C2) (jaylfc#1903) * feat(cluster): worker-initiated graceful pause + drain protocol (jaylfc#890 C2) Add worker-initiated status transitions via heartbeat: - heartbeat() accepts optional status and drain_reason fields - Workers can self-report 'update-available', 'draining', or 'updating' - Status 'update-available' workers remain routable (still serving) - Status 'draining' triggers same behavior as controller-initiated drain - Monitor loop handles stale update-available workers Controller side (ClusterManager): - _worker_for_resource, get_workers_for_capability, aggregate_catalog now include 'update-available' alongside 'online' - Worker-initiated drain emits notifications to activity feed Worker side (WorkerAgent): - report_update_available(), initiate_self_drain(), notify_drain_complete() - Convenience methods wrapping heartbeat() with appropriate status Tests: 11 new unit tests covering state transitions, routability, catalog inclusion, lease claims, monitor loop staleness, and the full online->update-available->draining pipeline. State machine: online -> update-available -> draining -> updating -> (restart) -> online * fix(cluster): rebase onto dev + address Kilo findings (1C/1W/2S) for worker drain Rebased feat/worker-self-drain onto origin/dev, resolving conflicts in manager.py, routes/cluster.py, and worker/agent.py from recently-landed registration-drift refresh fields (jaylfc#1538). Kilo fixes: - CRITICAL: Protect 'update-available' from status-less heartbeat reversion. The periodic ~15s heartbeat with no status was silently flipping update-available workers back to online, defeating the update signal. Now 'update-available' is protected alongside 'draining' — only explicit status transitions are honoured. - WARNING: Handle 'updating' in _monitor_loop. A worker stuck/crashed in 'updating' state was never re-onlined or offlined, permanently excluded from routing/catalog. Now they are force-offlined after HEARTBEAT_TIMEOUT. - SUGGESTION: Validate status against allow-list and sanitize drain_reason in notification block to prevent operator-facing UI injection. - SUGGESTION: notify_drain_complete() now sends status='updating' heartbeat so the controller transitions from 'draining' to 'updating'. Test update: test_heartbeat_without_status_reonlines_update_available renamed to test_heartbeat_without_status_preserves_update_available and now asserts the corrected protection behavior. 72/72 targeted tests pass. * fix(cluster): protect updating status from heartbeat reversion and cancel arbiter tasks on stale-update Add 'updating' to the protected status tuple in heartbeat so a periodic status-less heartbeat doesn't revert an updating worker to 'online', which would re-enter it into routing/catalog/lease-claims mid-update and prevent the monitor loop's stale-updating branch from firing. Mirror the stale-draining branch's GPU arbiter task cancellation in the stale-updating path so that in-flight arbiter work for force-released leases isn't orphaned when an updating worker goes stale. Fixes: PR jaylfc#1903 Kilo review findings (WARNING manager.py:239, SUGGESTION manager.py:835) * fix(cluster): hoist status allow-list to module frozenset + guard against invalid status heartbeat bypass Kilo found two issues post-rebase on jaylfc#1903: 1. The drain/update status allow-list was duplicated in three places (guard condition, assignment check, notification block). Hoist to module-level _VALID_STATUSES frozenset and reference in all spots. 2. An invalid status value via heartbeat ('online', 'offline', garbage) could revert a protected (draining/update-available/updating) worker back to 'online', defeating drain protection. Tighten the guard to require status in _VALID_STATUSES when the worker is protected. Closes jaylfc#1903. * fix(cluster): persist lifecycle status across controller restart, cancel arbiter on stale-update-available, gate online event - worker/agent.py: store lifecycle_status/lifecycle_reason in WorkerAgent, resend on periodic heartbeats so controller restart doesn't revert draining/updating workers to 'online' (CodeRabbit MAJOR #1) - manager.py: cancel GPU arbiter tasks on stale-update-available worker timeout, matching the cancellation already done for stale-drain and stale-update paths (CodeRabbit MAJOR #2) - manager.py: gate recovery 'worker.online' event on worker.status == 'online' to suppress false events for workers recovering into draining/updating state (CodeRabbit MINOR jaylfc#3) - tests/test_worker.py: accept **kwargs in fake_heartbeat mocks to match expanded heartbeat() signature --------- Co-authored-by: Hogne <227774406+hognek@users.noreply.github.com>
hognek
added a commit
that referenced
this pull request
Jul 18, 2026
) * docs(design): cross-user collaboration spec (contacts, human collaborators, agent delegation, human DM) Flagship epic: add another taOS user as a contact (hub friend edge), invite them as a human project collaborator, receive their delegated agents (normal external-selfjoin identities tagged with a sponsor contact), and chat human-to-human over an instance-to-instance peer channel. Rides the shipped invite/consent/registry rails; rejects registry-abuse and bus-federation. Phased plan with a pilot-first path (hogne) and lanes for lead/hognek/fleet. Awaiting owner decisions (section 10). * docs(design): add Milestone F - collaborator community view (stats, leaderboard, live kanban, community chat) When a user collaborates on a remote project, their Projects app gets a rich read-mostly community view (a mini social-GitHub for the shared project) so they see their contribution/involvement and feel part of the team. Introduces a scoped read-only project projection served over the peer channel (never direct API access, never sensitive fields) - the read counterpart to the write-delegation in Milestone D. Refines Decision 2: rich read surface, still zero board/file write for the human. Member-scoped in v1; public view a later toggle on the same machinery. * docs(design): address 1 Kilo + 5 CodeRabbit security findings on cross-user-collab spec (jaylfc#2024) CR #1 — Add replay protection, canonical serialization, audience binding, freshness window, and persistent replay cache to envelope handshake (line 124). CR #2 — Add peer-token vs. project-level authorization: contact identity authenticates the route family, but per-operation project membership, policy, and channel membership are independently enforced (line 131). CR jaylfc#3 — Add fail-closed runtime checks on every delegated request: verify current sponsor-contact status and project membership at call time, not just at token issuance; purge outbox and unclaim in-flight tasks on revoke (line 255). CR jaylfc#4 — Require non-null, owner-validated, immutable project_id claim in minted JWTs for project_tasks and canvas_read scopes (line 251). CR jaylfc#5 — Require atomic uniqueness constraint on (contact_id, channel_id, remote_msg_id) in chat_messages, insert-before-ack, treat conflicts as successful duplicates (line 328). Kilo #1 — Clarify project.visibility as a future schema addition (projects.visibility TEXT DEFAULT 'private'), not an existing column on hub posts (line 524). Co-authored-by: Hogne <227774406+hognek@users.noreply.github.com> --------- Co-authored-by: hognek <hognek@gmail.com> Co-authored-by: Hogne <227774406+hognek@users.noreply.github.com>
hognek
added a commit
that referenced
this pull request
Jul 19, 2026
…rate-limit LRU, prune commit, token docs Fix #1 (WARNING): Document outbound_token plaintext threat model in contacts_store schema comment — token must be presented on outbound requests; at-rest encryption deferred to post-MVP. Fix #2 (WARNING): Centralized /api/peer/ authentication via router-level _peer_auth_dep dependency. Previously EXEMPT_PREFIXES bypassed all auth middleware and correctness depended on every route calling _authenticate_peer. Now every route under /api/peer/ gets bearer-auth automatically; route handlers read contact_id from request.state. Fix jaylfc#3 (SUGGESTION): Commit the opportunistic nonce prune in its own transaction so a replay (IntegrityError) rollback does not undo it. Fix jaylfc#4 (SUGGESTION): Add record_nonce(…, kind='ack') to /api/peer/ack for replay protection. A replayed ack now returns 409 Conflict, matching the /inbox and /chat contract. Fix jaylfc#5 (SUGGESTION): Add LRU fallback eviction to the rate limiter. When all 2000+ entries have active windows (no expired entries to sweep), the oldest entry is evicted to prevent unbounded dict growth under sustained load.
hognek
added a commit
that referenced
this pull request
Jul 19, 2026
…verters for names with /
- Add Depends(get_current_user) to GET /api/secrets/agent/{agent_name}/github
so unauthenticated callers receive 401 instead of leaking installation IDs
and repo names (jaylfc MUST-FIX #1, Kilo WARNING)
- Change {name} to {name:path} on GET/PUT/DELETE /api/secrets/{name}
so secrets whose names contain / (e.g. GitHub repo full names like
owner/repo) are correctly routed
- Add test_github_grants_requires_auth (401 for unauthenticated caller)
- Fix test_update_secret_agents (uses literal / path now that {name:path}
handles multi-segment names)
- Remove useless path-traversal guard on agent_name (Kilo: irrelevant
since agent_name is not a filesystem path)
hognek
added a commit
that referenced
this pull request
Jul 20, 2026
…orage (jaylfc#2036) * feat(secrets): per-agent GitHub token grant UI + short-lived token storage - Add github-installation secret category to SecretsStore - Create tinyagentos/github_token.py: lifetime-aware token minting cache with mint_installation_token() and get_agent_github_token() - Add SecretsStore.get_agent_github_installations() for per-agent queries - Add GET /api/secrets/agent/{name}/github endpoint - Update SecretsApp.tsx with github-installation category support - Add per-agent access toggles for each GitHub App repo in GitHubConnect.tsx - Tests: 7/7 github_token tests, 6/7 secrets tests pass * fix(secrets): address 6 Kilo findings on GitHub token grants Fix 4 WARNING + 2 SUGGESTION findings from Kilo review on PR jaylfc#2036: WARNING fixes: - routes/secrets.py: Validate agent_name path parameter (reject empty, path separators, traversal) with docstring noting CSRF auth middleware - github_token.py: Iterate all granted installations instead of only installations[0]; document per-repo vs per-installation token scope - GitHubConnect.tsx + github_oauth.py + github.ts: Pass real permissions from GitHub App installation instead of hardcoded [] - test_secrets.py: Add TestSecretsEncryption class restoring XOR/Fernet round-trip coverage (encrypt/decrypt, list masking, update re-encrypt) SUGGESTION fixes: - github_token.py: LRU eviction by oldest expiry timestamp - GitHubConnect.tsx: Per-repo savingGrants Set<string> state * fix(secrets): add auth guard to GitHub grants endpoint, fix route converters for names with / - Add Depends(get_current_user) to GET /api/secrets/agent/{agent_name}/github so unauthenticated callers receive 401 instead of leaking installation IDs and repo names (jaylfc MUST-FIX #1, Kilo WARNING) - Change {name} to {name:path} on GET/PUT/DELETE /api/secrets/{name} so secrets whose names contain / (e.g. GitHub repo full names like owner/repo) are correctly routed - Add test_github_grants_requires_auth (401 for unauthenticated caller) - Fix test_update_secret_agents (uses literal / path now that {name:path} handles multi-segment names) - Remove useless path-traversal guard on agent_name (Kilo: irrelevant since agent_name is not a filesystem path) * fix(github): normalize permissions to list form across the stack The GitHub API returns permissions as a dict {contents: read} but the rest of the pipeline (secrets.py, tests) expects a list [contents:read]. Convert at the API-response boundary in github_oauth.py and align TypeScript types (Record<string,string> → string[]). Fixes Kilo WARNING: permissions type mismatch across the stack. * fix(github-grants): add owner/admin auth check, guard non-dict JSON, surface save errors - routes/secrets.py: verify caller owns agent (or is admin) before returning GitHub installation grants (CodeRabbit Major) - secrets.py: guard against json.loads returning non-dict payload in get_agent_github_installations (CodeRabbit Minor) - GitHubConnect.tsx: surface save-failure as transient inline error message instead of silently ignoring (CodeRabbit Minor) --------- Co-authored-by: Hogne <227774406+hognek@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Task: t_634725d4 / t_974e39db. Adds Skald sidecar manifest (Taos.md) parser and wires available_models through the cluster protocol.
Changes
Refs
Closes jaylfc#361 (load-on-demand infrastructure — inert until available_models is populated).
Note
Low Risk
Additive cluster/worker metadata and optional file read on backend detection; no auth or load-on-demand behavior yet.
Overview
Workers can now advertise catalog models they can run (not only what is probed as loaded) by reading Skald’s JSON sidecar via new
load_manifest()(TAOS_SKALD_MANIFEST/ default path), withSOFTWARE_TO_BACKEND_TYPEtying manifest entries to TAOS backend types.WorkerAgent.detect_backends()attaches per-backendavailable_models(metadata +loadedvsavailablefrom probe names).WorkerInfogainsavailable_models, andClusterManager.heartbeat()flattens/dedupes that list from heartbeat backends when present; heartbeats without backends leave existing values unchanged.Coverage is added in
test_skald_manifest.pyandtest_cluster_worker_protocol.py.Reviewed by Cursor Bugbot for commit 7d58e6e. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes