Repository navigation
Conversation
Co-authored-by: openhands <openhands@all-hands.dev>
Co-authored-by: openhands <openhands@all-hands.dev>
enyst
left a comment
There was a problem hiding this comment.
I realize this is just documenting for the skill, I'm a bit confused though: why A/B testing, and maybe why is it part of this skill.
First, running the reviewer agent on GitHub with a different LLM profile seem to me like stretching the term "A/B testing".
(1) Idk, what the user (PR author, PR reviewer) wants, is for the reviewer agent to work. It's not really testing for user preferences or whatnot.
(2) It would be nice to take into account writing the LLM profile into the comment: Slack proposal
(3) For different LLMs, it happens that we can intuit what kind of problems they're likely to see and what they are less likely to see. This intuition, and the problems posed by bypassing it, is maybe not as strong in this case, as it is in regular conversations... but, still 🤔 . I'm not sure, I feel it would be more useful if we can control what LLM gets used. But I don't know how, I'll make an issue on that...
|
@OpenHands Read the entire skill, and clone yourself automation/ repo from same org to see how exactly it works:
Respond as a comment here. Be CONCISE. |
|
I'm on it! enyst can track my progress at all-hands.dev |
|
Read the full skill + A/B testing is currently plugin preset only: Each variant has: Service design: validates exactly one of One doc mismatch I found: runtime conversation tags are This comment was created by an AI agent (OpenHands) on behalf of the user. |
|
Posted the concise PR comment: #378 (comment) |
|
@malhotra5 is this maybe something to fix?
|
|
Hi @enyst , thanks for the question! We're doing A/B testing because we're interested in building systems where we can try different design decisions (e.g. LLMs or prompts) and be able to monitor their downstream effects, and choose the most effective, or cost effective alternative. This is a first experiment, but we hope to do others in the future. |
Thank you @neubig . I would like to request full transparency. Please see Reviewer agent and QA Agent: add LLM, trajectories. I rewrote it in human language 😅 , for clarity. The TL;DR is simply that the automations runs, here on OpenHands repos, contain:
Edited to add: FWIW I intend to follow up with more reasoning than we see in the linked issues, by ~Monday |
|
Friendly bump from an Enterprise user: this documentation closes a real usability gap. In Enterprise 0.24.0 with automation service 1.1.5, the I have validated these fields against a live Replicated deployment and am happy to help with another verification pass. It looks like the PR now needs a rebase/conflict resolution and the noted |
|
🤖 OpenHands is reviewing this PR. Head commit: This comment was posted by an AI agent (OpenHands). |
|
This comment was posted by an AI agent (OpenHands). |
|
👋 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. |
all-hands-bot
left a comment
There was a problem hiding this comment.
This review was posted by an AI agent (OpenHands).
Scope
This is a documentation-only PR in the correct repository: skills/openhands-automation is the public registry skill for the Automations API, plugins/openhands/skills/openhands-automation is a symlink to it (single source of truth), and skills/index.js is the generated catalog artifact. No product/architecture decision is needed.
Verification performed
python scripts/sync_extensions.py --checkpasses (only the pre-existing non-blockingplugins/issue-duplicate-checkercoverage warning); regeneratingskills/index.jswithnode scripts/build-skills-catalog.mjsproduced no diff.uv run --group test pytest tests/test_skills_catalog.py tests/test_sync_extensions.py tests/test_skills_have_readme.py tests/test_catalogs.py tests/test_catalog_schema.py tests/test_skill_plugin_loading.py -q-> 210 passed.- GitHub check runs on the head commit are all
success(sync-extensions,test,validate-claude-code,sync-sdk-skill,pr-title). - Field names and semantics were cross-checked against the upstream service (
OpenHands/automation):CreateAutomationRequest.model/timeout/keep_alive,UpdateAutomationRequest,utils/timeout.py(default_run_duration = 600,max_run_duration = 1800,validate_automation_timeoutraising -> 422), andpreset_router.py(variants+experiment_idmutual exclusivity, variantmodelfallback). The newmodel,timeout,keep_alive,variants,experiment_id, and event-trigger-for-custom-automation claims all match the upstream schemas.
Findings
Two accuracy gaps, both in text this PR adds or directly extends. Neither is a functional or security defect.
-
The new update-field list recommends the deprecated
enabledfield while omittingstate, which upstream marks as its replacement (deprecated=True, "Deprecated: use state instead"). An agent following this skill would steer users to a field scheduled for removal. See inline comment. -
The new variant-level
modelfield means an A/B run tags its conversation withmodelprofilein addition to the two tags listed. The Observability table in the same file still names the tagexperiment_id, while the runtime key emitted bypresets/plugin/sdk_main.pyisexperimentid. This is the mismatch already raised in the PR discussion and not yet addressed. See inline comment.
Non-blocking context
keep_alive: false is not universally valid: for event triggers with destination: continue_conversation the service forces keep_alive = true at creation and rejects a later PATCH that sets it to false. The skill does not document destination/continue_conversation at all, so an agent is unlikely to reach that path, but the new blanket "keep_alive ... false or null lets the automation service explicitly clean it up" wording is not true in that one case. Worth a one-line caveat whenever continue_conversation gets documented.
Also note the PR is currently not mergeable against main (mergeable_state: dirty): skills/index.js conflicts, and main has since added references/security.md and references/windows.md to this same skill plus its own SKILL.md edits. A rebase will be needed, and the timeout/model wording should be reconciled with those newer additions.
🔄 CHANGES REQUESTED
| -H "Authorization: Bearer ${OPENHANDS_API_KEY}" | ||
|
|
||
| # Update (fields: name, trigger, enabled, timeout) | ||
| # Update fields: name, prompt, model, trigger, tarball_path, setup_script_path, entrypoint, enabled, timeout, keep_alive |
There was a problem hiding this comment.
This newly added update-field list names enabled, which the service marks deprecated (deprecated=True, "Deprecated: use state instead") and omits state, its replacement, plus agent_profile_id - all three are accepted by UpdateAutomationRequest. Since the goal of this PR is to surface the current update surface, please list state (ACTIVE/INACTIVE) and note that enabled is deprecated, rather than presenting enabled as the field to patch.
| | `name` | Yes | string | Unique variant name (1–100 chars) | | ||
| | `weight` | Yes | integer | Relative selection weight (> 0) | | ||
| | `plugins` | Yes | array | Plugin source(s) for this variant (at least one) | | ||
| | `model` | No | string | Model profile name for this variant; defaults to the automation-level model when omitted | |
There was a problem hiding this comment.
Adding variant-level model also changes what a run tags its conversation with: presets/plugin/sdk_main.py emits modelprofile alongside experimentid and variant whenever a variant model resolves. The Observability table in this file (around lines 172-179) still lists only experiment_id and variant, and uses experiment_id as the tag name while the actual runtime tag key is experimentid. Please fold modelprofile into that table and correct the tag keys so the tags shown match what the service actually writes.
There was a problem hiding this comment.
I'm an AI agent (Claude Code, based on Opus 5.5) helping Engel Nyst (@enyst) with project work.
This is still useful: references/custom-automation.md on main still says timeout must be 1-600 s, while the automation service now accepts up to 1800 s, and main's skill still doesn't document keep_alive or the automation-level model.
all-hands-bot's 2026-09-23 points (update-field list, A/B tag keys, the keep_alive caveat for continue_conversation, the rebase) still apply to this head. The rebase is more than the skills/index.js conflict: SKILL.md conflicts too, and since June the skill has also gained local-first host selection (#535), Cloud/Enterprise secrets and template paths (#709, #712) and reworked preset/custom guidance (#686), so the new Run Configuration Options section has to be refitted into that structure.
Overlap: #455 changes the timeout guidance in the same files, so whichever lands second should reconcile on the 1800 s max. If @malhotra5 doesn't have time for this, a maintainer could move the run-config table into a fresh PR on current main, as @rajshah4 suggested.
Why
The automation service recently added clearer support for configurable run options, including execution timeouts and sandbox cleanup behavior. The OpenHands automation skill should surface those options so agents can create and update automations with the current API.
Summary
model,timeout,keep_alive, andrepossupport.keep_alivesandbox cleanup behavior.Issue Number
N/A
How to Test
uv run --with pyyaml python scripts/sync_extensions.py --checkplugins/issue-duplicate-checker.git diff --checkVideo/Screenshots
N/A - documentation-only change.
Notes
This PR was created by an AI agent (OpenHands) on behalf of the user.
@malhotra5 can click here to continue refining the PR