feat: add agent-led OpenHands Enterprise Replicated install skill - #458
feat: add agent-led OpenHands Enterprise Replicated install skill#458rajshah4 wants to merge 3 commits into
Conversation
Co-authored-by: openhands <openhands@all-hands.dev>
Co-authored-by: openhands <openhands@all-hands.dev>
Co-authored-by: openhands <openhands@all-hands.dev>
|
👋 This PR needs a couple of things fixed before OpenHands can review it:
Push an update once this is addressed and this check re-runs automatically. This is an automated check - no AI was used to generate this comment. |
|
🤖 OpenHands is reviewing this PR. Head commit: This comment was posted by an AI agent (OpenHands). |
all-hands-bot
left a comment
There was a problem hiding this comment.
This review was created by an AI agent (OpenHands) on behalf of the repository maintainers.
Summary
This PR adds install-openhands-replicated, a well-structured skill for guiding OpenHands Enterprise VM installations through Replicated Embedded Cluster. The skill covers preflight checks, infrastructure provisioning, installer execution, Admin Console configuration, validation, and operator handoff.
The safety contract is strong: all mutating operations require explicit approval, secrets are treated as sensitive throughout, and the skill explicitly does not claim headless installation. The progressive disclosure structure (SKILL.md entry point + references/ + scripts/) follows the AgentSkills standard. The marketplace registration, vendor symlinks, and catalog updates are consistent with the repo conventions.
Risk Assessment: LOW
The skill is documentation and read-only preflight scripts. The main correctness issue (missing curl availability check) affects a preflight script but does not modify state. All mutating operations are gated behind explicit approval. No secrets are hardcoded.
Findings
Correctness
check_outbound.shdoes not verifycurlis installed (line 35): Ifcurlis missing,codeis an empty string,[[ "${code}" == "000" ]]is false, and every URL reportsOK- a false negative that could allow proceeding with installation without verifying outbound connectivity. The other preflight scripts guard their dependencies (check_tls_files.shchecks foropensslon line 42,summarize_terraform_outputs.shchecks forterraformon line 16). Consider addingcommand -v curlat the top and exiting with a clear error if missing. Additionally, even when curl is installed but fails before producing a status code, the empty string passes the check - consider also failing on emptycode.
Minor
-
apply_kots_config.shpreview description is slightly misleading (line 8): The usage text says "Previews a KOTS ConfigValues merge by default" but the preview (lines 159-169) only prints the command that would be executed; it does not show the resulting merged configuration. An agent following the skill might assume the preview shows a diff. Consider clarifying the usage text to say the preview prints the command to be run. -
check_dns.shnslookup fallback has inconsistent output (line 55): Thegetentanddigbranches capture output and re-print it withprintf, but thenslookupbranch runs the command directly, producing verbose unstructured output to stdout. This is a UX inconsistency, not a correctness issue (the exit-code check correctly detects resolution failures).
Positive observations
- Explicit approval gates for all mutating operations (installer, ConfigValues, deployments, provider setup, restores)
- Private key hashes are unset after comparison in
check_tls_files.sh(line 84) - Temp files in
check_tls_files.share cleaned up viatrap cleanup EXIT - Storage guard correctly detects emptyDir/hostPath volumes that could lose state
- No em-dashes in new content (follows repo punctuation convention)
- Correct
environmentcategory and valid marketplace entry - Good separation of concerns: read-only preflights vs. mutating helpers
- Blue/green reinstall reference explicitly requires separate approval
|
|
||
| failed=0 | ||
| for url in "${urls[@]}"; do | ||
| code="$(curl -sSIL --max-time 15 -o /dev/null -w "%{http_code}" "${url}" || true)" |
There was a problem hiding this comment.
Missing curl availability check. If curl is not installed, code is an empty string, [[ "${code}" == "000" ]] is false, and every URL reports OK - a false negative in a safety-critical preflight. The sibling scripts guard their binary dependencies (check_tls_files.sh line 42 checks for openssl, summarize_terraform_outputs.sh line 16 checks for terraform). Consider adding command -v curl >/dev/null 2>&1 at the top and exiting with an error if missing. Also, even when curl is installed, a pre-request failure can produce an empty string - consider failing when code is empty in addition to "000".
| cat >&2 <<'EOF' | ||
| usage: apply_kots_config.sh --appslug <slug> --config-file <config-values.yaml> (--current | --sequence <number>) [options] | ||
|
|
||
| Previews a KOTS ConfigValues merge by default. Add --execute only after the |
There was a problem hiding this comment.
The usage text says "Previews a KOTS ConfigValues merge by default" but the preview (lines 159-169) only prints the command that would be executed - it does not show the merged configuration result. Consider clarifying the wording to "Prints the command to be run" to avoid an agent assuming the preview shows a diff of the merge.
| printf '%s\n' "${output}" | ||
| elif command -v dig >/dev/null 2>&1 && output="$(dig +short "${host}" 2>/dev/null)" && [[ -n "${output}" ]]; then | ||
| printf '%s\n' "${output}" | ||
| elif command -v nslookup >/dev/null 2>&1 && nslookup "${host}"; then |
There was a problem hiding this comment.
The nslookup fallback runs the command directly, producing verbose unstructured output to stdout, while the getent and dig branches capture and re-print output with printf. This is a UX inconsistency. The exit-code check correctly detects resolution failures, so this is not a correctness issue, but consider capturing and formatting the nslookup output for consistency.
HUMAN: This is a skill i have reviewed / run to help users install the replicated install. The idea is to provide a bit more scaffolding around the install process.
Summary
Adds
install-openhands-replicated, a customer-neutral skill for guiding supported OpenHands Enterprise VM installations delivered with Replicated Embedded Cluster.This implements the first milestone of internal tracker PRD-137:
Safety boundary
The skill does not claim a fully headless installation. Headless automation remains conditional on a documented, release-specific installer schema and supported secret-input mechanism. The current workflow uses guided installer/Admin Console ClickOps and preview-first documented KOTS configuration.
Validation
uv run python scripts/sync_extensions.py --checkuv run pytest -q: 540 passed, 6 skippedRegistry updates
environmentcategoryskills/index.jsThis draft pull request was created by an AI agent (OpenHands) on behalf of Rajiv Shah.