feat: migrate polling automations to KV store for cloud-safe state persistence

[tofarr]

• A human has tested these changes.

---

Why

The three polling skills (github-pr-reviewer, github-repo-monitor, slack-channel-monitor) each persisted run-to-run state — last-poll timestamps, active conversation IDs, processed-event dedup sets — to JSON files under WORKSPACE_BASE. On cloud deployments where each cron run lands on a fresh pod, WORKSPACE_BASE points to a new ephemeral directory, so all state was silently lost between runs. This meant polling automations could re-process already-handled events or lose track of active conversations every run.

OpenHands/automation#69 landed a built-in per-automation KV store that survives across runs. AUTOMATION_KV_TOKEN and AUTOMATION_API_URL are now injected into every run by the dispatcher.

Summary

• Three main.py scripts (github-pr-reviewer, github-repo-monitor, slack-channel-monitor): replace load_state(path) / save_state(path, state) with KV-first equivalents that fall back to the existing file path when AUTOMATION_KV_TOKEN is absent (local/dev). The full state document is stored under a single KV key "state". Also fixes the slack-channel-monitor debug log path, which previously navigated two directory levels above WORKSPACE_BASE — broken on fresh pods.
• openhands-automation skill docs (SKILL.md + references/custom-automation.md): add a new "State Persistence (KV Store)" section with the KV API endpoint table, ready-to-copy kv_get / kv_set helpers, and a full load_state / save_state pattern with fallback; add AUTOMATION_KV_TOKEN and AUTOMATION_API_URL to the env vars table.
• Three state-schema.md reference docs: updated to document KV store as primary storage and file as local fallback.

Screenshots

Slack Integration:

Github Code Review Agent:

Github Repository Monitor

Issue Number

Related: OpenHands/automation#69

How to Test

1. Deploy or run a local instance with AUTOMATION_KV_SECRET set (so AUTOMATION_KV_TOKEN is injected into runs).
2. Create a github-repo-monitor or slack-channel-monitor automation and trigger it twice. Confirm that last_poll and conversations from run 1 are present at the start of run 2 (visible in run logs: "State loaded from KV store").
3. Without AUTOMATION_KV_SECRET (local dev), confirm the automation still works by falling back to the file path (visible in logs: "State saved to /path/to/state.json").

Unit tests: uv sync --group test && uv run pytest -q — 339 tests pass.

Notes

• No API or schema changes. The state document shape is identical; only the storage backend changes.
• The single-key ("state") pattern matches the KV store's single-document model, keeping all reads/writes atomic and minimising round-trips.
• Backward compatible: existing local-dev automations continue working with the file fallback.

This PR was created by an AI agent (OpenHands) on behalf of the user.

[tofarr]

Pushed a fix in commit 9e43cf4 for a bug affecting all three scripts introduced in this PR.

Root cause: _kv_get was returning json.loads(r.read()) — the full KVKeyResponse envelope {"key": "state", "value": {...}} — rather than just data["value"] (the actual state dict). On the first run there is no KV entry yet, so _kv_get returns None and load_state falls back to _default_state(), which succeeds. On the second run the KV store returns 200 with the wrapped response, load_state hands back {"key": "state", "value": {...}} as the state dict, and any access to a real state field (e.g. state["last_poll"]) raises KeyError.

Fix: Change return json.loads(r.read()) to return json.loads(r.read())["value"] in _kv_get in all three scripts. The second json.loads call in each file (inside get_settings) is a different API and is unchanged.

This comment was added by an AI agent (OpenHands) on behalf of the user.

[all-hands-bot]

✅ Review complete.

This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here.

[all-hands-bot]

🟡 Taste Rating: Acceptable - the KV-store direction is the right fix, but the current fallback semantics and copy-paste docs still leave real correctness traps.

[CRITICAL ISSUES]

• [skills/github-pr-reviewer/scripts/main.py, Line 142] Correctness: When KV is configured, falling back to a local file on KV read/write errors silently reintroduces ephemeral state in cloud runs. That can duplicate label/comment processing or lose active conversation tracking. The file fallback should only be used when KV is not configured; KV operation failures should fail the run so it can retry against the durable store.
• [skills/openhands-automation/references/custom-automation.md, Line 536] Documentation Correctness: The ready-to-copy KV helpers still return the full {"key": "state", "value": ...} envelope instead of the stored value, while the scripts were fixed to unwrap ["value"]. Users pasting the documented pattern will load the wrong state shape.

[RISK ASSESSMENT]

• [Overall PR] ⚠️ Risk Assessment: 🟡 MEDIUM
  This changes state persistence for three polling automations and touches token-based access to a new service-side store. The blast radius is limited to these skills/scripts, but incorrect state handling can cause duplicate automation work or lost conversation tracking across runs.

VERDICT:
❌ Needs rework: The durable-state path is sound, but cloud runs should fail closed on KV errors and the docs must match the actual KV response shape before this is safe to merge.

KEY INSIGHT:
Durable state only helps if KV remains the single source of truth whenever it is configured; otherwise transient KV failures degrade back to the exact ephemeral-state behavior this PR is trying to eliminate.

Improve this review? If any feedback above seems incorrect or irrelevant to this repository, you can teach the reviewer to do better:

1. Add a .agents/skills/custom-codereview-guide.md file to your branch (or edit it if one already exists) with the /codereview trigger and the context the reviewer is missing (e.g., "Security concerns about X do not apply here because Y"). See the customization docs for the required frontmatter format.
2. Re-request a review - the reviewer reads guidelines from the PR branch, so your changes take effect immediately.
3. When your PR is merged, the guideline file goes through normal code review by repository maintainers.

Resolve with AI? Install the iterate skill in your agent and run /iterate to automatically drive this PR through CI, review, and QA until it's merge-ready.

Was this review helpful? React with 👍 or 👎 to give feedback.

This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation

[all-hands-bot]

✅ Review complete.

This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here.

[all-hands-bot]

🟡 Taste Rating: Acceptable - the KV-store migration is directionally sound, and the previously flagged KV fallback/envelope issues are fixed. The PR still leaves an existing skill test suite broken, so it is not merge-ready yet.

[CRITICAL ISSUES]

• [skills/github-repo-monitor/scripts/main.py, Line 183] Testing Regression: The changed load_state / save_state signatures break the existing skills/github-repo-monitor/tests/test_main.py state tests. I ran uv run --group test pytest skills/github-repo-monitor/tests/test_main.py -q; 6 tests fail with TypeError because they still call load_state(path) and save_state(path, state). Update the tests to cover the new file-fallback behavior and add KV-path coverage, or preserve a compatible helper seam.

[TESTING GAPS]

• The runtime change touches durable state persistence for three polling automations, but only the repo-monitor has local state tests and those are now stale. Before merging, the PR should include regression coverage that proves: missing/corrupt local fallback still starts from defaults, valid local state still loads, KV 404 returns defaults, KV GET unwraps value, and configured KV errors propagate instead of falling back to ephemeral files.

[RISK ASSESSMENT]

• [Overall PR] ⚠️ Risk Assessment: 🟡 MEDIUM
  This changes run-to-run state persistence for three polling automations and introduces token-authenticated calls to the automation KV store. The blast radius is limited to these skills, but regressions can duplicate GitHub/Slack event handling or lose active conversation tracking across runs. Existing CI checks pass, but a relevant in-repo skill test suite fails when run directly.

VERDICT:
❌ Needs rework: Core direction is sound, but the stale/broken tests need to be updated before this is safe to merge.

KEY INSIGHT:
Durable state is now the critical contract for these pollers, so the tests need to exercise the new storage boundary rather than only the old path-argument file helpers.

Improve this review? If any feedback above seems incorrect or irrelevant to this repository, you can teach the reviewer to do better:

1. Add a .agents/skills/custom-codereview-guide.md file to your branch (or edit it if one already exists) with the /codereview trigger and the context the reviewer is missing (e.g., "Security concerns about X do not apply here because Y"). See the customization docs for the required frontmatter format.
2. Re-request a review - the reviewer reads guidelines from the PR branch, so your changes take effect immediately.
3. When your PR is merged, the guideline file goes through normal code review by repository maintainers.

Resolve with AI? Install the iterate skill in your agent and run /iterate to automatically drive this PR through CI, review, and QA until it is merge-ready.

Was this review helpful? React with 👍 or 👎 to give feedback.

This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation

[all-hands-bot]

🔍 Review in progress…

We are performing the review through OpenHands Cloud Automation. You can log in and view the conversation here.

This literally just crashed on a second run.

[all-hands-bot]

✅ Review complete.

This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here.

[all-hands-bot]

🟢 Taste Rating: Good taste

The PR solves a real operational problem with a simple KV-first state model while preserving the local-file fallback for development. The core choice to keep the existing state document shape and store it under a single per-automation key avoids migration complexity and keeps the live poller logic easy to reason about.

I reviewed the three poller state changes, the KV helper documentation, generated catalog updates, and existing review threads. The earlier fallback/error-handling and KV envelope concerns appear resolved: configured KV failures now propagate instead of silently using ephemeral files, and GET responses are unwrapped to the stored value.

Validation checked:

• uv run --group test pytest skills/github-repo-monitor/tests/test_main.py -q - 43 passed, 3 subtests passed
• uv run python scripts/sync_extensions.py --check - passed with only the existing non-blocking coverage warning
• uv run python scripts/sync_openhands_sdk_skill.py --check - SDK skill is up to date
• GitHub PR checks are all successful

[RISK ASSESSMENT]

• [Overall PR] ⚠️ Risk Assessment: 🟡 MEDIUM
  This touches persisted state handling for multiple polling automations and depends on the automation service KV API, so the blast radius is higher than a documentation-only change. The implementation is intentionally narrow, keeps existing state schemas intact, preserves local fallback behavior, and has both focused tests plus passing CI, so I do not see a blocking merge risk.

VERDICT:
✅ Worth merging: Core logic is sound and the PR addresses the cloud-persistence failure mode directly.

KEY INSIGHT:
The important design win is that state ownership stays local to each automation while storage durability moves behind a service-provided per-automation KV namespace.

This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation

[malhotra5]

LGTM