Conversation
|
👋 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. |
|
@VascoSch92 @DevinVinson @enyst could one of you approve the fork workflows and take a look when you have a chance? This PR covers phase 1 of #342 (run source stamping). The test and CI workflows are waiting for first-time contributor approval and haven’t started yet. Thanks! |
|
📁 PR Artifacts Notice This PR contains a |
|
@tofarr If can you help here ? to trigger the UT. |
|
The failed unit-test job is a shared CI dependency failure, not a provenance assertion failure: 1,703 tests passed, then all 17 S3 integration cases failed in fixture setup because Docker Hub no longer serves the pinned MinIO image. The registry repair is isolated in #447. |
Why
When an automation changes after a successful run, the run history currently cannot answer a basic operational question: which source did that run execute?
automation_runsstores the outcome, timestamps, cost, and execution identifiers, but source remains only on the mutableautomationsrow. Readingautomation.tarball_pathlater therefore describes the automation now, not the historical run.That missing provenance blocks the first, independent step of rollback work proposed in #342: identifying the last known-good source before deciding how to restore or pin it.
📄 Open the visual design note
Summary
This PR makes source provenance a property of each
AutomationRun:source_tarball_pathandsource_commitcolumns through cross-database migration023._execute_runto use the run's frozensource_tarball_path, with a fallback toautomation.tarball_pathfor legacy rows.AutomationGitSyncState.last_synced_commitonly when the sync state is clean. A dirty state intentionally recordssource_commit = null, because the previous SHA no longer identifies the current DB-side tarball.AutomationRunResponsewithout changing existing response fields.Behaviour
source_tarball_pathsource_commitnull—the old SHA would be misleadingnullnullnullThe nullable commit is deliberate: an absent provenance claim is safer than a precise-looking but stale SHA.
Dispatch consistency
The source is initialized at run creation for immediate visibility, but creation alone is not sufficient because a run may remain PENDING while the automation is edited.
The claim flow is now:
run.source_tarball_path, not the mutable automation field.This closes the edit-while-pending race without changing which version the dispatcher selects. Once claimed, a later automation edit cannot change the tarball used by that run.
API and migration
AutomationRunResponsegains two additive optional fields:{ "source_tarball_path": "oh-internal://uploads/2fbd...", "source_commit": "4a42e3c9..." }Compatibility properties:
TextandString(64)types and follows the repository's PostgreSQL/SQLite pattern.Changes by area
models.py/ migration023utils/run.pyutils/webhook.pydispatcher.pyschemas.pyScope
This is intentionally phase 1 of #342, which the issue identifies as independently useful.
Included:
Not included:
restore-last-goodendpoint or automatic rollback policy.The PR therefore references, but does not close, #342.
How to test
Automated checks completed locally:
uv run pre-commit run --files <all changed Python and migration files>uv run pytest tests/test_telemetry.py tests/test_schemas.py tests/test_db.py::TestSqliteMigrations::test_migrations_run_on_sqlite -qget_settings()deprecation warning_execute_runwith the execution backend mocked at the external boundary.git diff --check: passedNot run locally:
OPENHANDS_API_KEYwas available. This does not affect the local source-selection path above; it prevents authentication to the external staging sandbox service.Issue Number
Implements the run-version stamping phase of #342. Follow-up work is required before the overall rollback proposal is complete.
Type