Skip to content

test: cover automation preflight requirements - #476

Open
KirschBluteX wants to merge 5 commits into
OpenHands:mainfrom
KirschBluteX:feat/16565-preflight-contract
Open

KirschBluteX wants to merge 5 commits into
OpenHands:mainfrom
KirschBluteX:feat/16565-preflight-contract

Conversation

@KirschBluteX

@KirschBluteX KirschBluteX commented Aug 15, 2026 •

Copy link
Copy Markdown
  • A human has tested these changes.

HUMAN:

I led this contract update from gap analysis through implementation and final validation. I traced how Agent Canvas derives deployment requirements from requires.integrations and the authoritative integration catalog, then updated the contract helper and every direct automation preflight fixture to record MCP transport, locator, authentication strategy, required secret names, and the applicable GitHub integration alternatives. I kept the security boundary explicit: preflight carries secret names only and never reads, stores, or exposes secret values. I also aligned this repository's contract with the four companion pull requests, personally ran and reviewed all 47 targeted tests, regenerated the skills, integrations, and automations catalogs, built both Python distribution formats in an isolated X: drive directory, investigated the unrelated native-Windows limitations in the broader test suite, and reviewed the deterministic Agent Canvas UI evidence. I retain responsibility for the technical decisions, implementation, validation, and final submission.

AGENT:

AI assistance was limited to targeted secondary review of the catalog-to-contract mapping, fixture completeness, secret-name-only boundary, cross-repository consistency, and organization of the verification evidence. The agent cross-checked that the reported commands and results support the PR claims and that the companion changes use the same contract. I personally evaluated that feedback, made the implementation decisions, ran the validation commands, reviewed the resulting artifacts and UI evidence, and approved the final submission.


Why

I traced the automation setup contract from requires.integrations, through the authoritative integration catalog, to the deployment preflight data consumed by Agent Canvas. I found that the existing contract fixtures did not fully record the deployment-checkable requirements derived from that catalog, leaving MCP transport, locator, authentication strategy, and required secret names under-specified at the contract boundary.

This change keeps those fixtures aligned with the behavior expected by Agent Canvas and the companion repositories.

Related to OpenHands/OpenHands#16565.

Summary

I updated the automation setup contract coverage to:

  • derive MCP transport, locator, authentication strategy, and required secret names from the authoritative integration catalog rather than duplicating handwritten expectations
  • record the resulting GitHub integration alternatives in every direct automation preflight fixture
  • document explicitly that preflight data contains required secret names only and never secret values
  • keep the contract fixtures aligned with the corresponding Agent Canvas and companion-repository changes

I kept the change focused on the contract helper and its fixtures. No secret values are read, stored, or exposed by this preflight contract.

Issue Number

OpenHands/OpenHands#16565

How to Test

I personally ran and reviewed the targeted contract tests, catalog generation, and package build.

Automation setup contract tests

uv run --frozen --group test pytest tests/test_automation_setup.py -q
47 passed

Catalog generation and project build

npm run build

The skills, integrations, and automations catalogs were generated successfully.

Python package build

uv build --out-dir <X-drive temp directory>

The source distribution and wheel were both built successfully. I used a temporary output directory on the X: drive so generated package artifacts remained isolated from the working tree.

I also examined the broader pytest-suite limitations on native Windows. Existing unrelated tests rely on locale-default reads for UTF-8 assets and Unix symlinks, so that broader suite is not currently portable to native Windows. The changed automation setup contract test file passed in full.

Video/Screenshots

I reviewed the deterministic mock UI failure and success states in the companion Agent Canvas PR. The corresponding screenshots are available in this Agent Canvas PR comment.

Notes

  • I led this work from contract analysis and catalog mapping through implementation, fixture updates, cross-repository alignment, local validation, and final evidence review.
  • I used AI assistance for targeted review and help organizing the PR evidence, while I retained responsibility for the technical decisions, implementation, validation, and final submission.
  • Preflight carries secret names only; it never carries secret values.

Companion pull requests aligned with this change:

Jev-Fast-Audit

⚡ Jev fast audit · estimates · 0.44s · commit 100a957
Strongest signal: No primary concern selected.
Evidence: No primary concern to locate.
Coverage: complete supplied coverage; 16/16 hunks, 6/6 files.

All estimates and evidence
Estimate Likelihood / value Direct evidence
SQL injection 3.0% No direct hunk selected
Command injection 5.0% No direct hunk selected
Weakened authentication 5.0% No direct hunk selected
Weakened authorization 6.0% No direct hunk selected
Contract regression 24.0% F006H002 · tests/test_automation_setup.py:423–498
Data loss 4.0% No direct hunk selected
Sensitive data disclosure 6.0% No direct hunk selected
Unexpected data transfer 5.0% No direct hunk selected
Credential misuse 8.0% No direct hunk selected
Untrusted instruction authority 4.0% No direct hunk selected
Package source redirection 4.0% No direct hunk selected
Unverified remote execution 3.0% No direct hunk selected
Privileged environment access 7.0% No direct hunk selected
Security assessment bypass 7.0% No direct hunk selected
Prohibited workload 2.0% No direct hunk selected
Primary concern None selected; confidence 62.0% No primary concern to locate

@all-hands-bot

Copy link
Copy Markdown
Contributor

👋 This PR needs a couple of things fixed before OpenHands can review it:

  • the PR description's HUMAN: section needs at least 20 characters describing what you tested, not just the template placeholder

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.

@KirschBluteX
KirschBluteX marked this pull request as draft August 15, 2026 14:30
@KirschBluteX
KirschBluteX marked this pull request as ready for review August 15, 2026 14:47
@all-hands-bot

Copy link
Copy Markdown
Contributor

🚦 CI is currently failing on this PR's latest commit.

Please fix the failing checks before OpenHands reviews it - this is re-checked automatically once you push a new commit. (A maintainer can also request @all-hands-bot as a reviewer to have it reviewed regardless of CI status.)

This is an automated check - no AI was used to generate this comment.

@enyst enyst left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm an AI agent (Claude Code, based on Opus 5.5) helping Engel Nyst (@enyst) with project work.

Requesting changes. This head conflicts with main (automations/README.md, tests/test_automation_setup.py), and main has changed since it was written in ways that break two of its assumptions.

  1. maxProperties: 1 on setup.form.triggers contradicts the current contract (catalog.schema.json:310). On main, multiple trigger keys are selectable variants (see the schema description at catalog.schema.json:365 and AGENTS.md). Both custom-automation and github-pr-reviewer declare cron and event, so they would fail validation. Please drop the constraint and its rejection test.

  2. The fixtures are stale. On main, github-pr-reviewer no longer requires the github integration (requires.integrations is {}). The requirements block added to its fixture (github-pr-reviewer.json:74) therefore no longer matches what the derivation produces. Main also added tests/fixtures/automations/custom-automation.json with four preflight scenarios. Because _derive_preflight_body now always adds requirements (test_automation_setup.py:302), that fixture needs {"integrations": []} too, since its GitHub requirement is optional. Main's _derive_preflight_body also takes selected_trigger / selected_action now, so resolving the test-file conflict means more than accepting both sides.

  3. Land this after the owning repo. The requirements request shape belongs to /v1/validate in OpenHands/automation, and OpenHands/automation#341 and OpenHands/OpenHands#16614 are both still open. These fixtures record what the host sends, so please merge this after automation#341 and match the fixture shape to what lands there.

Minor: the CRLF normalization in scripts/build-automation-catalog.mjs is harmless but unrelated to this change.

Process: Validate PR description fails. The check only resolves issues in this repository, so OpenHands/OpenHands#16565 isn't accepted. It needs a linked OpenHands/extensions issue labeled ready-for-dev, and HUMAN: must be the first visible line (right now the first line is a checkbox). The Tests and Check Extensions runs on this head failed at workflow startup, so CI needs a fresh run after the rebase.

@KirschBluteX
KirschBluteX requested a review from enyst October 8, 2026 11:59

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: test Adding or fixing tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants