feat: Add preset_metadata JSON field to Automation model#89
Conversation
- Add preset_metadata JSONB column to Automation model (nullable) - Add preset_metadata to AutomationResponse schema - Populate preset_metadata in prompt preset endpoint with: - preset_type: 'prompt' - prompt: the user's prompt - repos: (optional) repository configuration - Populate preset_metadata in plugin preset endpoint with: - preset_type: 'plugin' - prompt: the user's prompt - plugins: list of plugin sources - repos: (optional) repository configuration - Add Alembic migration 005 for the new column - Add comprehensive tests for preset_metadata feature This allows the UI to display preset-specific configuration without needing to inspect tarball contents. Closes #56 Co-authored-by: openhands <openhands@all-hands.dev>
|
🚀 Deploy Preview PR Created/Updated A deploy preview has been created/updated for this PR. Deploy PR: https://github.com/OpenHands/deploy/pull/3996 Once the deploy PR's CI passes, the automation service will be deployed to the feature environment. |
Change preset_metadata column from PostgreSQL-specific JSONB to generic sa.JSON to support potential future SQLite usage. Co-authored-by: openhands <openhands@all-hands.dev>
Co-authored-by: openhands <openhands@all-hands.dev>
all-hands-bot
left a comment
There was a problem hiding this comment.
🟡 Acceptable with fixes - Good structure and comprehensive tests, but needs one critical fix for type consistency.
Verdict: ❌ Needs rework - Fix the JSON→JSONB type inconsistency before merging.
Key Insight: Using JSON instead of JSONB contradicts the PR description, breaks consistency with existing fields, and sacrifices PostgreSQL-specific performance benefits.
|
[RISK ASSESSMENT]
The PR is additive and backward-compatible (nullable field), with good test coverage. However, the JSON vs JSONB type inconsistency could cause issues:
Once the type is corrected to JSONB, this becomes LOW risk.
|
|
Addressed review feedback — the use of |
all-hands-bot
left a comment
There was a problem hiding this comment.
🟢 Good taste - Clean, pragmatic implementation that solves a real problem.
The preset_metadata field provides the UI with structured configuration data without requiring tarball inspection. Implementation is straightforward: nullable JSONB column, populated by preset endpoints, comprehensive tests covering all scenarios.
The intentional use of sa.JSON (vs JSONB for older fields) maintains SQLite compatibility per maintainer guidance from resolved review threads.
[RISK ASSESSMENT]
- [Overall PR]
⚠️ Risk Assessment: 🟢 LOW
Adds a nullable column with no breaking changes. Only affects preset endpoints. Comprehensive test coverage verifies metadata population and database persistence. Safe migration strategy (nullable column compatible with existing records).
VERDICT:
✅ Worth merging - Simple, well-tested addition that enables UI functionality.
KEY INSIGHT:
Storing preset metadata alongside the automation eliminates the need for UI to parse tarballs, enabling instant configuration display—a pragmatic separation of runtime artifacts from UI state.
Summary
Adds a generic
preset_metadataJSON column to the Automation model that stores preset-specific configuration for UI consumption. This follows up on the repository support work and allows the UI to display automation configuration without needing to inspect tarball contents.Closes #56
Changes
Model Changes
automation/models.py: Addedpreset_metadata: Mapped[dict | None]JSON column (nullable)Schema Changes
automation/schemas.py: Addedpreset_metadata: dict | NonetoAutomationResponseEndpoint Changes
automation/preset_router.py:preset_metadatawith:preset_type: "prompt"prompt: the user's prompt textrepos: (optional) repository configuration when repos are specifiedpreset_metadatawith:preset_type: "plugin"prompt: the user's prompt textplugins: list of plugin source configurationsrepos: (optional) repository configuration when repos are specifiedMigration
migrations/versions/005_add_preset_metadata_column.py: Adds thepreset_metadataJSON columnTests
tests/test_preset_router.py: AddedTestPresetMetadataclass with 5 comprehensive tests:test_prompt_preset_sets_preset_metadatatest_prompt_preset_with_repos_includes_repos_in_metadatatest_plugin_preset_sets_preset_metadatatest_plugin_preset_with_repos_includes_repos_in_metadatatest_preset_metadata_stored_in_databaseMetadata Schema
{ "preset_type": "prompt" | "plugin", "prompt": "...", "plugins": [{"source": "github:owner/repo", "ref": "v1.0.0"}], "repos": [{"url": "OpenHands/repo", "ref": "main"}] }preset_type,prompt,repos(when specified)preset_type,prompt,plugins,repos(when specified)preset_metadata = NULLDesign Decision:
sa.JSONvsJSONBThis field uses
sa.JSONinstead of PostgreSQL-specificJSONBto support potential future SQLite compatibility. Existing fields (trigger,event_payload) still useJSONBas they were created before this requirement.Testing
All unit tests pass. Integration tests require Docker (skipped in this environment but should pass in CI).
This PR was created by an AI agent (OpenHands) on behalf of the user.