fix(service): expire and reconcile task records stuck in active states

[akushonkamen]

Description

A task record whose finalization was missed stayed at running forever. If the process died between persisting result and running _finalize_task, or the last work item was released through a path that does not re-finalize, the record kept reporting work in progress while the work had finished long ago (the issue observed ten session_commit tasks stuck for 19–21 h, each with a populated result and no error).

Root cause (real execution path). TaskTracker._is_expired (openviking/service/task_tracker.py, _is_expired at L471–475 on 6006fc3; the issue cites the older L308–313) only gave a TTL to terminal states — COMPLETED/CANCELLED (24 h) and FAILED (7 d). RUNNING matched no branch, so it returned False unconditionally, the cleanup loop (_evict_expired, every CLEANUP_INTERVAL = 300 s) was a no-op for it, and the record stayed on disk indefinitely. _finalize_task (L905–913) early-returns while TaskWorkIndex.has_work(task_id) is true, so a missed call was never retried. Startup reconciliation did not help either: prepare_task_tracking → restore_work_tasks only loaded the records referenced by rebuilt QueueFS work and never closed an active record whose work was gone.

Behavior before → after.

• Before: an orphaned running record (no queue work, updated_at 8 days old) → _is_expired is False; never evicted; still running after restart.
• After: active statuses (pending/running/cancelling) get a 7-day recovery window (TTL_ACTIVE), so an orphaned record becomes expirable and is collected by the cleanup loop; and at startup restore_work_tasks finalizes every persisted active record that owns no rebuilt work, deriving the terminal state from what was persisted (result → completed, error/nothing → failed) and appending the missing terminal status_changed event. A genuinely-running task that still owns queue work is never collected, however stale its record looks.

Entrypoint / owner module. openviking/service/task_tracker.py (TaskTracker), openviking/service/task_store.py (PersistentTaskStore) — Platform / tasks.

Human Involvement

• A human participated in the implementation or review loop
• This PR was generated entirely by AI agents without human participation in the loop

Related Issue

Fixes #6130

Type of Change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Refactoring (no functional changes)
• Performance improvement
• Test update

Changes Made

• task_tracker.py: add TTL_ACTIVE = 604_800 (7 days) and make _is_expired return age > TTL_ACTIVE for active statuses. The check is deliberately kept a pure age comparison: _evict_expired consults it while holding the tracker lock, and TaskWorkIndex.has_work takes its own lock; the work-index callbacks (finalize_before_ack/register) already run index-lock → tracker-lock, so calling has_work from inside _is_expired would create an ABBA deadlock. The has-work confirmation therefore lives in the collectors.
• task_tracker.py: guard _delete_expired_task — an active record for which has_work(task_id) is still true is never collected (only the recovery window passed). _prune_persisted_expired already had the equivalent guard.
• task_tracker.py: startup reconciliation — restore_work_tasks now calls _reconcile_abandoned_tasks(owners), which scans all persisted records via store.list_all(), skips anything in owners or still owning work, and runs _finalize_task on remaining active records. It runs once at boot before any consumer starts, so an active record with no rebuilt work can only be a leftover from a process that died before finalizing. Malformed records are logged and skipped per record.
• task_store.py: add PersistentTaskStore.list_all() (and the TaskStore protocol method) for the startup scan. It iterates account/user directories under the task root and skips unreadable accounts, so one corrupt file cannot disable reconciliation. Scoped readers keep using list/list_page.
• tests/test_task_tracker.py: 7 new tests —
  test_is_expired_gives_active_statuses_a_recovery_window,
  test_evict_reclaims_orphaned_running_record,
  test_evict_keeps_running_task_that_still_owns_work,
  test_ttl_terminal_behaviour_unchanged,
  test_restore_work_tasks_finalizes_orphaned_running_record,
  test_restore_work_tasks_finalizes_interrupted_orphan_as_failed,
  test_restore_work_tasks_leaves_owned_active_records_alone.
  Four of them fail on main and pass with this change.

No public API change. Terminal-state TTLs (TTL_COMPLETED, TTL_FAILED) are unchanged (covered by test_ttl_terminal_behaviour_unchanged). No migration: existing orphaned records are settled by the startup reconciliation on the next boot, or collected by the cleanup loop once they are older than 7 days.

Testing

• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• I have tested this on the following platforms:
  • Linux
  • macOS
  • Windows

The issue was reproduced, not only inferred from the code. Red/green with a standalone script that builds TaskRecord(status=RUNNING, updated_at=now-8d) on a TaskTracker whose work index holds nothing for the task and asserts _is_expired(...) is True:

• on main (6006fc3): _is_expired=False → AssertionError, exit 1 (red)
• with this change: _is_expired=True, exit 0 (green)

Validation commands run against this commit (macOS, Python 3.13):

• pytest tests/test_task_tracker.py tests/test_task_tracker_concurrency.py -q --no-cov → 117 passed (includes the 7 new tests).
• ruff check openviking/service/task_store.py openviking/service/task_tracker.py tests/test_task_tracker.py → All checks passed; ruff format --check on the same three files → 3 files already formatted.
• timeout 300 pytest tests/unit -q --no-cov --ignore=tests/unit/service/test_core_embedder_lifecycle.py → 49 failed / 1979 passed / 2 skipped on this branch and the identical 49 failed / 1979 passed / 2 skipped on main, with a byte-identical FAILED list on both sides — i.e. zero regressions. The 49 pre-existing failures are environmental (the RAGFS native binding is not initialized in this environment, RAGFSBindingClient will be None), and tests/unit/service/test_core_embedder_lifecycle.py hangs identically on main and on this branch, which is why it is excluded.
• Related subsets tests/test_task_backend_config.py tests/unit/service/test_session_auto_commit.py tests/unit/service/test_core_consistency.py tests/storage → 34 failed + 84 errors, identical to main line-by-line (same RAGFS-binding environment cause).

Checklist

• My code follows the project's coding style
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• Any dependent changes have been merged and published

Screenshots (if applicable)

N/A

Additional Notes

• Why a 7-day window rather than immediate reclamation: _is_expired cannot consult the work index (lock ordering above), and updated_at alone cannot distinguish a long-running task from an orphan. The window matches TTL_FAILED; the startup reconciliation is what settles orphans promptly, the window is the backstop for a process that keeps running after the finalize was missed.
• _reconcile_abandoned_tasks uses getattr(store, "list_all", None) so an alternate TaskStore implementation without list_all simply skips reconciliation instead of failing boot.
• Disclosure: this change was implemented and validated by AI agents (see the claim comment on [Bug]: a task that never reaches a terminal status stays at running forever -- _is_expired has no branch for RUNNING #6130). The commit is DCO signed-off.

[akushonkamen]

Hi — triaging the failing check: the failure is environmental (fork PRs can't access the embedding credentials), not caused by this PR's changes.

What failed: In "API & CLI Integration Tests (ubuntu-24.04)", step Run API Tests (Unix) ended with 5 failed, 8 passed, 1 skipped. All 5 failures (4 × filesystem/test_fs_cp.py, 1 × filesystem/test_fs_read_write.py) are the same PROCESSING_ERROR: Failed to generate embedding: Volcengine embedding failed: Error code: 401 — 'The API key format is incorrect' — i.e. every test that needs a real embedding on fs_write gets a 401 from the Ark endpoint.

Why it happens: this workflow triggers on pull_request, so a fork PR cannot read repository secrets. In that case .github/workflows/api_test.yml ("Create OpenViking config file (Unix)") falls back to the minimal config containing the literal placeholder keys dummy-vlm-api-key / dummy-embedding-api-key, and any embedding-backed call then fails with exactly this 401.

Evidence it is not this branch:

• The PR diff touches only openviking/service/task_store.py, openviking/service/task_tracker.py and tests/test_task_tracker.py (task-record expiry/reconciliation) — no embedding, filesystem or auth code.
• The same job fails identically today on multiple unrelated fork PRs (e.g. run 38779543565 on fix/hermes-remember-empty-timeout: same 5 failed, 8 passed, 1 skipped, same 401 message), while the most recent runs on main all succeed (same-repo branches do get the secrets).
• Locally, the PR's own tests are green: uv sync --frozen --extra test then uv run pytest -q -o addopts='' tests/test_task_tracker.py tests/test_task_tracker_concurrency.py tests/test_session_task_tracking.py tests/test_task_backend_config.py → 137 passed.

Whether the integration lane should gate fork PRs at all (e.g. a maintainer dispatch with secrets, or treating the job as advisory for forks) is a maintainer decision — happy to re-run or adjust if there's a preferred path.