fix(git-sync): re-export every automation when the encryption key changes

[enyst]

I'm an AI agent (Claude Code) helping Engel Nyst (@enyst) with project work.

Issue Number

Fixes #551

Why

When you save a Git Sync encryption key, automations that were already synced stay as plaintext at the repository's HEAD. They only become ciphertext one by one, as each automation is edited. So someone who turns encryption on to protect the repo still has every existing prompt and script readable in it.

There were two gaps:

1. Changing the key through PUT /v1/git-sync/config marked no automation dirty, and the sync cycle only exports dirty automations.
2. Even a forced re-export would have skipped the write. _exported_content_is_current compares decrypted plaintext, and decrypt_file_tree passes non-Fernet content through unchanged, so plaintext on disk looked current under the new key.

Summary

• apply_git_sync_config_override now marks all of the org's AutomationGitSyncState rows dirty when the effective encryption key changes (set, rotated or cleared). It compares the key merged over the env base, so saving the same key again does nothing. This sits next to the existing backoff reset on field changes. Dirty slugs are skipped by the import, so files left under an old key are not read with the new one.
• _exported_content_is_current now treats a plaintext generated file as stale while a key is set, using a new small helper serializer.is_encrypted (which decrypt_file_tree also uses now).
• The AGENTS.md Git Sync encryption bullet documents the behaviour.

Result: the next cycle rewrites every automation in one commit, and later cycles with unchanged content push nothing. Clearing the key re-exports plaintext in one commit the same way. Git history is not rewritten; only HEAD is in scope.

Known gaps, left for a follow-up:

• A cycle that already resolved its settings before the key was saved (the periodic loop resolves settings for all due orgs at the start of a tick; a manual sync resolves them before the pull) can use up the new dirty flags under the old key. HEAD then stays plaintext until an automation is edited or the key is saved again. A path change does not have this problem because it is detected inside the cycle. The robust fix is a persisted "last synced encryption" marker checked inside the cycle, like last_synced_path.
• In local mode, changing only AUTOMATION_GIT_SYNC_ENCRYPTION_KEY in the environment and restarting still does not trigger a re-export, because no PUT happens. The same in-cycle marker would cover this.
• As with a path change, marking everything dirty means a git-side edit that has not been imported yet is overwritten by the service's copy in that cycle.

Testing

New tests:

• tests/test_git_sync.py::TestEncryptionKeyChangeReExports: setting a key turns plaintext into ciphertext in one commit (nothing left dirty, deleted_in_db is 0, all automations still enabled, the next cycle pushes nothing); clearing the key goes back to plaintext; rotating makes the files decrypt with the new key; saving the same key again does not re-export.
• tests/test_git_sync_router.py::TestGitSyncConfig::test_changing_the_encryption_key_marks_the_orgs_automations_dirty (Postgres): dirty_count in the PUT response is 0 on a branch change, 1 on set, rotate and clear, 0 on a same-key save, and the other org's rows stay clean.

On unchanged code, 3 of the 4 cycle tests fail with pushed_commit is None and the router test fails with assert 0 == 1 (.pr/tests-before.txt). With the fix all 5 pass (.pr/tests-after.txt). Reverting either half on its own makes tests fail, so both are guarded.

Commands run on this branch after rebasing onto current main:

uv run python -m pytest tests/test_git_sync.py tests/test_git_sync_serializer.py -q
# 96 passed
uv run python -m pytest tests/test_git_sync_router.py -q
# 58 passed
uv run pre-commit run --files openhands/automation/git_sync/config_override.py openhands/automation/git_sync/loop.py openhands/automation/git_sync/serializer.py tests/test_git_sync.py tests/test_git_sync_router.py AGENTS.md
# ruff format, ruff lint, pycodestyle, pyright: Passed

Before the rebase, the wider set tests/test_git_sync.py tests/test_git_sync_router.py tests/test_git_sync_serializer.py tests/test_git_sync_state.py tests/test_git_sync_backoff.py tests/test_git_sync_client.py passed (223 tests).

Evidence

End-to-end run of the real service through the issue's API recipe with .pr/e2e_key_change.py: it starts uvicorn openhands.automation.app:app in local mode (SQLite, local file store, a bare git repo as the remote), creates a prompt automation, sets the repo and syncs, saves encryption_key and syncs, syncs again with nothing changed, then clears the key and syncs, inspecting the branch HEAD with git show after each step. Full logs are in .pr/e2e-before.txt and .pr/e2e-after.txt.

Before (on main): the PUT reports dirty_count=0, HEAD stays on the first commit, and automation.yaml and tarball/prompt.txt are still plaintext.

After: the PUT reports dirty_count=1, the sync makes a new commit where both files start with gAAAAA (ciphertext), the idle sync makes no commit, and clearing the key reports dirty_count=1 and commits plaintext again (3 commits in total).

Both images are rendered captures of terminal output, not UI screenshots. This change is backend-only, inside the automation service; no UI is involved. The .pr/ directory is removed automatically when the PR is approved.

🤖 Generated with Claude Code

https://claude.ai/code/session_01HmGfBq3KDFiJTDraPJFGTT

---

Generated by Claude Code

[github-actions]

  ✅ **PR Artifacts Cleaned Up**

  The `.pr/` directory is no longer present.

[github-actions]

Warning

Your comment is too long (maximum is 65536 characters), so the coverage report was not added. See the job log for how to reduce it.

[all-hands-bot]

This review was posted by an AI agent (OpenHands).

I re-fetched the current state at head 7173546cb333ecc39f4bc1b2bafabae1fc958050, read AGENTS.md and .agents/skills/custom-codereview-guide.md, and checked the change against issue #551 (open, ready-for-dev).

Scope: correctly placed in this repository — the git-sync export lives in openhands/automation/git_sync/.

Verified correct

• apply_git_sync_config_override compares the effective key merged over base_git_sync_settings() before and after the update, so it marks the org's AutomationGitSyncState rows dirty only when the effective key actually changes (set, rotate, clear), and a same-key save stays clean. The UPDATE is scoped by org_id, so other orgs' rows are untouched.
• _exported_content_is_current now treats a key-set cycle with any plaintext generated file on disk as stale; is_encrypted is the shared helper decrypt_file_tree also uses. The dirty marking makes the import skip those slugs for that cycle, so files left under an old key are not re-read with the new one.
• Focused tests pass locally: tests/test_git_sync.py::TestEncryptionKeyChangeReExports + tests/test_git_sync_serializer.py (40 passed). The new router test needs Docker/Postgres and could not run in this workspace; CI unit-tests is green.

Material finding
The required backend check fails on this head. ci.yml runs uv run pre-commit run --all-files, which lints the tracked evidence script .pr/e2e_key_change.py: ruff-format reformats it and ruff-check then reports 5 E501 errors (the long f-strings cannot be auto-split). The PR is BLOCKED because of it. Formatting the script (or excluding .pr/ from the ruff hooks) clears the check; .pr/ is removed on approval anyway, but the check must be green to merge.

Non-blocking: the settings-resolution race you documented (a cycle that resolved settings before the PUT consumes the new dirty flags under the old key) is real, but it is confined to that interleaving and does not affect the direct PUT-then-sync path issue #551 exercises.

🔄 CHANGES REQUESTED

[all-hands-bot]

This review was posted by an AI agent (OpenHands).

Re-checked at head 0c5701e (merge of main/#564). The required backend check is now green, and so is unit-tests — the PR is unblocked.

The fix is byte-for-byte what I reviewed at c1c3093 (only .pre-commit-config.yaml and pyproject.toml came in from the merge; openhands/, tests/ and AGENTS.md are unchanged). Re-verified locally on this head: tests/test_git_sync.py + tests/test_git_sync_serializer.py = 96 passed, all four TestEncryptionKeyChangeReExports cases included; the backend hooks pass on the changed files; and ruff check --show-files . no longer picks up .pr/e2e_key_change.py, so the evidence stays tracked. Since #564 is in the base, the approval cleanup will leave .pr/ alone.

Approving. No code changes requested; the documented settings-resolution race remains a non-blocking follow-up.

[enyst]

Ran also a GPT-6.1-Sol on this, it recommends merge.

[all-hands-bot]

This review was posted by an AI agent (OpenHands).

Fresh review at head b0706e27af243f9904b587dcaf366e00ecb1e1a3. Re-read AGENTS.md and .agents/skills/custom-codereview-guide.md, and confirmed issue #551 is open with ready-for-dev/priority:medium.

Scope: belongs in this repository — the git-sync export lives in openhands/automation/git_sync/.

Change: the head differs from the previously reviewed revision only by the automated removal of the .pr/ artifacts (the tracked evidence script that was failing the backend pre-commit job). The source and tests are byte-identical to the earlier review. Re-verified:

• apply_git_sync_config_override compares the effective key merged over base_git_sync_settings() before and after the update, so it marks the org's AutomationGitSyncState rows dirty only when the effective key actually changes (set, rotate, clear); a same-key save stays clean, and the UPDATE is scoped by org_id so other orgs are untouched.
• _exported_content_is_current treats a key-set cycle with any plaintext generated file on disk as stale, via the shared serializer.is_encrypted helper that decrypt_file_tree now also uses. Marking those slugs dirty makes the import skip them for the cycle, so files left under an old key are not re-read with the new one.
• Focused tests pass locally: tests/test_git_sync.py::TestEncryptionKeyChangeReExports + tests/test_git_sync_serializer.py (40 passed). The new Postgres router test could not run here (no Docker), but CI unit-tests is green.
• Current GitHub Actions for this head are all green: backend, unit-tests, Build and Push Automation Image, Validate PR description, check-pr-artifacts, and the title checks. The earlier blocking backend failure is resolved.

The known settings-resolution race documented in the PR (a cycle that resolved settings before the PUT consumes the new dirty flags under the old key) is real but confined to that interleaving; it does not affect the direct PUT-then-sync path issue #551 exercises and is reasonably deferred to the follow-up. No material correctness, security, compatibility, or acceptance-criterion defect found on this head.

✅ APPROVED