fix(openhands-automation): warn about webhook limitations on local deployments, recommend polling#274
Conversation
…ployments and recommend polling Webhooks require an internet-accessible URL. When AGENT_SERVER_URL or <HOST> is a local address (localhost, 127.0.0.1, etc.), external services cannot deliver events to the automation service. Changes: - Add CRITICAL rule 5: check for local URL before suggesting event triggers; recommend cron-based polling and reference the github-repo-monitor skill - Flag Event trigger type in the Trigger Types table as requiring a public URL - Add a callout box to the Trigger Types section linking to the new alternative - Add a new 'Polling as a Webhook Alternative' section with a GitHub example, a polling-vs-webhooks comparison table, and a tip to the github-repo-monitor skill - Add a public-URL-required warning at the top of the Event-Triggered Automations section Co-authored-by: openhands <openhands@all-hands.dev>
… github-repo-monitor refs Replace all locality-check references to AGENT_SERVER_URL / <HOST> with RUNTIME_URL — the variable that is actually present in production K8s pods. AGENT_SERVER_URL and HOST are absent there. Also remove the two mentions of the github-repo-monitor skill: the openhands-automation skill should stand alone without cross-referencing other skills. Changes: - Rule 5: check RUNTIME_URL (with explicit echo command), remove github-repo-monitor bullet - Trigger Types warning: RUNTIME_URL - Polling as a Webhook Alternative intro: RUNTIME_URL - Polling Tip blockquote: removed (referenced github-repo-monitor) - Event-Triggered Automations warning: RUNTIME_URL Co-authored-by: openhands <openhands@all-hands.dev>
Removed GitHub polling example and related details from SKILL.md.
- RUNTIME_URL locality check: was stated in Rule 5, Trigger Types warning, Polling section intro, and Event-Triggered warning. Keep only Rule 5; remove the three restatements. - 'How It Works' implementation-detail blocks: removed from both Prompt Preset and Plugin Preset sections — not actionable for the agent. - 'Choosing the Right Preset' prose paragraphs: removed the three paragraphs that restated Rules 0 and 3; the decision table is sufficient. Net: -27 lines, no information lost. Co-authored-by: openhands <openhands@all-hands.dev>
Co-authored-by: openhands <openhands@all-hands.dev>
Co-authored-by: openhands <openhands@all-hands.dev>
Added guidance on handling deterministic patterns and custom automation options.
Removed redundant sections about handling deterministic patterns and custom automation options, and added them back in a more concise manner.
|
✅ Review complete. This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here. |
all-hands-bot
left a comment
all-hands-bot
left a comment
There was a problem hiding this comment.
🟡 Acceptable — the intent is right and the table addition is clean, but there is one factual error in the guidance and the implementation is incomplete relative to the PR description.
Critical Issues
Undefined environment variable RUNTIME_URL
Rule 5 instructs agents to "check the RUNTIME_URL environment variable" — but RUNTIME_URL is not defined or documented anywhere in this skill, the automation reference docs, or the broader OpenHands system as described in this file. The document already defines:
AGENT_SERVER_URL— the agent server base URLOPENHANDS_HOST— the automation service API host (e.g.,https://app.all-hands.dev)<HOST>— the system-prompt value agents are told to check for API calls
An agent following rule 5 in production would not know what RUNTIME_URL refers to or where to find it, likely defaulting to suggesting webhooks anyway — defeating the fix. The correct variable to probe appears to be OPENHANDS_HOST: if it resolves to localhost or 127.0.0.1, webhook delivery from external services isn't possible. This should be made consistent with the document's established terminology.
Improvement Opportunities
Incomplete implementation vs. PR description
The PR description promises four distinct changes; the diff delivers roughly two and a half:
- ✅ New rule #5 (with the undefined-variable bug above)
- ✅ Trigger Types table annotation
⚠️ Polling section: comparison table present, but "a concrete GitHub polling example" and "a tip pointing to thegithub-repo-monitorskill" are absent- ❌ "Prominent callout at the top of the Event-Triggered Automations section" — entirely missing from the diff
The sparse polling section is particularly problematic: it tells agents why to use polling but provides no concrete how. Without an example cron automation or a link to the github-repo-monitor skill, agents must improvise from scratch.
Unrelated change in openhands-sdk/SKILL.md
Adding a link to 51_agent_hooks has no connection to the webhook/polling limitation being fixed. Should be a separate commit or PR to keep history clean and reviewable.
Risk Assessment
[RISK ASSESSMENT]
- [Overall PR]
⚠️ Risk Assessment: 🟡 MEDIUM
Documentation-only change — no production code at risk. However, theRUNTIME_URLfactual error could actively mislead agents following this skill. An agent checking an undefined variable gets no signal and may fall back to recommending webhooks regardless, defeating the purpose of the fix. The incomplete polling section also means the intended UX improvement isn't fully realized.
VERDICT:
❌ Needs rework: Fix the RUNTIME_URL undefined variable (use OPENHANDS_HOST or AGENT_SERVER_URL to match existing terminology), and add the missing concrete polling example and github-repo-monitor tip before merging.
KEY INSIGHT:
The idea is correct — local deployments can't receive inbound webhooks and agents need to know this — but the implementation introduces an undefined env var and omits the concrete polling example that would make the guidance actionable.
Improve this review? If any feedback above seems incorrect or irrelevant to this repository, you can teach the reviewer to do better:
- Add a
.agents/skills/custom-codereview-guide.mdfile to your branch (or edit it if one already exists) with the/codereviewtrigger and the context the reviewer is missing. See the customization docs for the required frontmatter format.- Re-request a review — the reviewer reads guidelines from the PR branch, so your changes take effect immediately.
- When your PR is merged, the guideline file goes through normal code review by repository maintainers.
Was this review helpful? React with 👍 or 👎 to give feedback.
This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation
all-hands-bot
left a comment
all-hands-bot
left a comment
There was a problem hiding this comment.
🟡 Acceptable — the intent is right and the table addition is clean, but there is one factual error in the guidance and the implementation is incomplete relative to the PR description.
Critical Issues
Undefined environment variable RUNTIME_URL
Rule 5 instructs agents to "check the RUNTIME_URL environment variable" — but RUNTIME_URL is not defined or documented anywhere in this skill, the automation reference docs, or the broader OpenHands system as described in this file. The document already defines:
AGENT_SERVER_URL— the agent server base URLOPENHANDS_HOST— the automation service API host (e.g.,https://app.all-hands.dev)<HOST>— the system-prompt value agents are told to check for API calls
An agent following rule 5 in production would not know what RUNTIME_URL refers to or where to find it, likely defaulting to suggesting webhooks anyway — defeating the fix. The correct variable to probe appears to be OPENHANDS_HOST: if it resolves to localhost or 127.0.0.1, webhook delivery from external services is not possible. This needs to be consistent with the document's established terminology.
Improvement Opportunities
Incomplete implementation vs. PR description
The PR description promises four distinct changes; the diff delivers roughly two and a half:
- ✅ New rule #5 (with the undefined-variable bug above)
- ✅ Trigger Types table annotation
⚠️ Polling section: comparison table present, but "a concrete GitHub polling example" and "a tip pointing to thegithub-repo-monitorskill" are absent- ❌ "Prominent callout at the top of the Event-Triggered Automations section" — entirely missing from the diff
The sparse polling section is particularly problematic: it tells agents why to use polling but provides no concrete how. Without an example cron automation or a link to the github-repo-monitor skill, agents must improvise from scratch.
Unrelated change in openhands-sdk/SKILL.md
Adding a link to 51_agent_hooks has no connection to the webhook/polling limitation being fixed. Should be a separate commit or PR to keep history clean and reviewable.
[RISK ASSESSMENT]
- [Overall PR]
⚠️ Risk Assessment: 🟡 MEDIUM
Documentation-only change — no production code at risk. However, theRUNTIME_URLfactual error could actively mislead agents following this skill. An agent checking an undefined variable gets no signal and may fall back to recommending webhooks regardless, defeating the purpose of the fix. The incomplete polling section also means the intended UX improvement is not fully realized.
VERDICT:
❌ Needs rework: Fix the RUNTIME_URL undefined variable (use OPENHANDS_HOST or AGENT_SERVER_URL to match existing terminology), and add the missing concrete polling example and github-repo-monitor tip before merging.
KEY INSIGHT:
The idea is correct — local deployments can't receive inbound webhooks and agents need to know this — but the implementation introduces an undefined env var and omits the concrete polling example that would make the guidance actionable.
Improve this review? If any feedback above seems incorrect or irrelevant to this repository, you can teach the reviewer to do better:
- Add a
.agents/skills/custom-codereview-guide.mdfile to your branch (or edit it if one already exists) with the/codereviewtrigger and the context the reviewer is missing. See the customization docs for the required frontmatter format.- Re-request a review — the reviewer reads guidelines from the PR branch, so your changes take effect immediately.
- When your PR is merged, the guideline file goes through normal code review by repository maintainers.
Was this review helpful? React with 👍 or 👎 to give feedback.
This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation
…L vs AGENT_SERVER_URL) The review bot flagged RUNTIME_URL as undefined. This commit makes the environment variable landscape unambiguous: - RUNTIME_URL: ambient in cloud environments; the public-facing URL of the sandbox (both agent server and automation service). Used as the API host for all automation calls and to determine whether external webhook delivery is possible. - AGENT_SERVER_URL: NOT an ambient env var. Injected by the automation service into scripts at run time only. Only accessible inside a running script. - OPENHANDS_HOST: not a real environment variable. Used throughout examples as a shell-variable convention; now documented explicitly with the one-liner to resolve it from RUNTIME_URL. Changes: - Architecture section: add key env vars table; update component descriptions to match reality. - Rule 5: fix capitalisation (check → Check). - Determining the API Host: replace '<HOST> system prompt' instruction with RUNTIME_URL and the OPENHANDS_HOST shell-variable pattern. Co-authored-by: openhands <openhands@all-hands.dev>
…only RUNTIME_URL is the public-facing URL of the agent server sandbox, not the automation service. The automation service may run at a separate URL. - Architecture: restore OPENHANDS_HOST convention in Automation Service description (RUNTIME_URL was incorrect there) - Env vars table: clarify RUNTIME_URL is the agent server URL; note the automation service may be at a separate URL - Determining the API Host: remove the incorrect OPENHANDS_HOST=RUNTIME_URL one-liner; restore the <HOST> system-prompt lookup with app.all-hands.dev as default Co-authored-by: openhands <openhands@all-hands.dev>
Corrected formatting for API URL in documentation.
| > - **Custom script** — full control over code, with or without LLM; point them to `references/custom-automation.md` | ||
| > - Let the user choose which approach to use. | ||
| > 4. **Only create custom scripts after the user agrees to that path.** Refer to `references/custom-automation.md` for the full reference. | ||
| > 5. **Before suggesting event-triggered (webhook) automations, check whether the deployment is publicly reachable.** Check `RUNTIME_URL`. Webhooks require an internet-accessible URL so that external services (GitHub, Slack, Linear, etc.) can deliver events to the automation service. If `RUNTIME_URL` is unset, empty, or a local address (`localhost`, `127.0.0.1`, `0.0.0.0`, etc.), the service cannot receive inbound webhook traffic from the public internet. In that case: |
There was a problem hiding this comment.
This is normally fair. It’s also possible to make a small receiver, deployed publicly and use a tunnel to localhost.
This is a tiny reason why I feel something is not totally clear about automation service architecture and resulting UX. Maybe we could conceive the automation part as “the receiver”, and that’s what needs to be accessible from the internet, strictly speaking.
| **Before making API calls, determine the correct host:** | ||
|
|
||
| Look for a `<HOST>` value in the system prompt. If present, use that URL. Otherwise, default to `https://app.all-hands.dev`. | ||
| The automation service may run at a different URL from the agent server. In the examples throughout this skill, `${OPENHANDS_HOST}` is a shell-variable convention for the automation service base URL — it is **not** a real environment variable. Set it from context before running any curl command: |
There was a problem hiding this comment.
Why isn’t this a real env var? Maybe it can be?
There was a problem hiding this comment.
It is not a real env var in local because it is not needed locally. In the cloud / staging environment, it is not yet defined - but we may change that.
There was a problem hiding this comment.
I think maybe I should clarify: this reads a bit confusing. Why have a var which is set to the value of HOST env var, when we can just do that directly?
| | Variable | Availability | Description | | ||
| |---|---|---| | ||
| | `RUNTIME_URL` | Ambient in cloud environments | Public-facing URL of the **agent server** sandbox. Use this to determine whether external webhook delivery is possible — if unset or local, webhooks cannot be received. The automation service may run at a separate URL (see Determining the API Host). | | ||
| | `AGENT_SERVER_URL` | Injected into scripts at run time only | Internal URL of the agent server. Available inside script execution context; **not** an ambient environment variable outside of a running script. | |
There was a problem hiding this comment.
Is this accurate? Is it cloud-only and if so, could we check that it’s clear that it’s cloud-only?
There was a problem hiding this comment.
It is accurate as of earlier this morning. I think in the long run we should unify these variables across projects, but that's a separate PR.
There was a problem hiding this comment.
Sorry, I guess I should clarify: the text says
RUNTIME_URL
...
if unset or local
|AGENT_SERVER_URL
...
at run time only | Internal URL of the agent server. Available inside script execution context; not an ambient environment variable outside of a running script
The text for the first var talks about local.
The text for the second var doesn't specify anything, and it's in reality cloud-only. (?)
This reads very confusing. People will read that the second var as being for local too.
|
✅ Review complete. This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here. |
all-hands-bot
left a comment
all-hands-bot
left a comment
There was a problem hiding this comment.
Review Summary
The core intent is right and the execution is mostly solid. Fixing the silent-failure scenario where agents recommend webhooks on local deployments is a real improvement, and the three-part approach — rule 5 in the CRITICAL block, the env vars table, and the trigger type caveat — is well-targeted. The author’s responses to earlier review threads are reasonable, and the design choices (single-source rule 5, no cross-skill coupling, no redundant callout) are defensible.
Two new observations worth addressing, plus one suggestion:
This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation
Co-authored-by: OpenHands Bot <contact@all-hands.dev>
Co-authored-by: OpenHands Bot <contact@all-hands.dev>
Co-authored-by: OpenHands Bot <contact@all-hands.dev>
Removed duplicate section header for clarity.
|
✅ Review complete. This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here. |
all-hands-bot
left a comment
all-hands-bot
left a comment
There was a problem hiding this comment.
Code Review
✅ Approve — Ready to merge
This PR achieves its stated goal cleanly: agents will no longer silently recommend webhook automations on local deployments. The three-layer approach — env vars table, CRITICAL rule 5, and trigger-type annotation — is well-targeted and proportionate.
Key concerns from prior review rounds are addressed in the current revision
OPENHANDS_HOST in the env vars table — The third table row is present in the current diff, making the three-variable picture complete: RUNTIME_URL (public-facing sandbox URL), AGENT_SERVER_URL (internal script-only URL), and OPENHANDS_HOST (shell convention for the automation service API). This fully documents the landscape agents need to reason about.
RFC 1918 private ranges already covered — Rule 5 already enumerates 10.x.x.x, 192.168.x.x, and 172.16–31.x.x alongside localhost / 127.0.0.1 / 0.0.0.0. The unreachability check is comprehensive.
Intro prose before the polling table — The section opens with a clear prose sentence ("When the deployment cannot receive inbound webhook traffic…") before the comparison table. The ### Polling vs. Webhooks at a Glance sub-heading then gives the table appropriate context.
RUNTIME_URL rationale is convincing — RUNTIME_URL is the public-facing agent-server sandbox URL, distinct from OPENHANDS_HOST (automation service). This separation is clarified in the table and in the "Determining the API Host" prose. The author's explanation that OPENHANDS_HOST is set manually (not injected) while RUNTIME_URL is ambient in cloud environments is now clearly documented. Rule 5 correctly points agents at the variable that actually reflects public reachability.
Design choices for intentional omissions are sound — removing the cross-skill callout (no coupling to github-repo-monitor), keeping the polling section minimal rather than adding a full example, and dropping the redundant webhooks-section callout in favour of a single authoritative CRITICAL rule are all reasonable trade-offs. The current structure avoids duplication and keeps the skill self-contained.
Non-blocking observation
The skills/openhands-sdk/SKILL.md line addition (linking 51_agent_hooks) is unrelated to the webhook/local-URL fix. The author has explained it predates this branch and removing it breaks the CI diff checks — acceptable context. It is a one-line addition with no correctness impact and not worth blocking on.
This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation
4 participants
Problem
The
openhands-automationskill presented event-triggered (webhook) automations without any caveat about deployment requirements. Webhooks only work when the automation service is publicly reachable — on a local deployment external services cannot POST events to it, so an agent following the skill would recommend an approach that silently fails.The skill also had no documentation on the environment variables that control the deployment, leaving agents to guess or use incorrect variables (
AGENT_SERVER_URL,<HOST>) when deciding whether webhooks are viable.Changes (
skills/openhands-automation/SKILL.md)Key environment variables table (Architecture section)
New table documenting the three variables agents need to understand:
RUNTIME_URLOPENHANDS_HOST<HOST>in the system prompt, or default tohttps://app.all-hands.dev.CRITICAL rule 5 — check
RUNTIME_URLbefore suggesting webhooksBefore recommending event-triggered automations, the agent checks
RUNTIME_URL. If it is unset, empty, or a local/private address, the deployment cannot receive inbound webhook trafBefore recommending event-triggered automations, tautBefore recommending event-triggered automations, the agee user.Determining the API Host (expanded)
Single line replaced with a clear explanation: the automation service may run aSingle line replaced with a clear explanation: the automation service may run aSingle line replaced with a clear explanation: the automation service may run aSingle line replaced with a clear explanation: the automation service may run aSingle line replaced with a clear explanation: the automation service may run aSinn
ingle line replaced with a clear explanation: the automation service may run aSingle line replaced with a clear explanation: the automation service may run aSingle line replaced with a clear explanation: thng ingle line replaced with a clear explanation: the automation servicedded a link to the new
51_agent_hooksexample in the examples list.Screenshots
I create an automation from a conversation...
This succeeds...
And creates a review bot...
I create a PR...
And the automation finds it...
And writes a review...
This PR was created and updated by an AI agent (OpenHands) on behalf of the repository owner.