fix(tests): type-correct OpenViking skill-scaffolding test sentinels by kshitijk4poor · Pull Request #47973 · NousResearch/hermes-agent

kshitijk4poor · 2026-06-17T18:14:52Z

Problem

The skill-scaffolding regression tests added in #47311 (cherry-picked from @ehz0ah's #32663, commit e3adbb5) introduced new ty diagnostics on main: assigning a bare object() to the typed _client: Optional[_VikingClient] field, and calling .join() on _{sync,prefetch}_thread, typed Optional[threading.Thread]. Caught by the lint-diff bot on #47311 (+2 new ty issues).

Fix

Test-only, no production change. Fixes the whole pattern in tests/openviking_plugin/test_openviking.py, not just the 2 flagged lines:

_client = object() → cast(Any, object()) (×5) — the repo's idiomatic sentinel-cast pattern.
_{sync,prefetch}_thread.join(...) now preceded by assert provider._{sync,prefetch}_thread is not None (×3) — narrows the Optional for the type checker and documents the invariant that a background thread must have been spawned for that turn.

Testing

scripts/run_tests.sh tests/openviking_plugin/ → 22 passed
tests/agent/test_memory_skill_scaffolding.py → 35 passed (combined)
ruff check clean; ty check tests/openviking_plugin/test_openviking.py: 24 → 16 diagnostics (both flagged lines resolved)
git diff --check clean

Resolves conflicts from the OpenViking churn that merged after #32445 was opened (#48042/#47662 session-switch + write hardening, #47311/#47973): - plugins/memory/openviking/__init__.py: keep both __init__ field groups (the PR's _runtime_start_* alongside main's _prefetch_threads/_shutting_down). - tests/plugins/memory/test_openviking_provider.py: keep BOTH the PR's new setup-validation tests and main's session-switch/concurrency tests (disjoint additions to the same region). Two fixes layered while reconciling (contributor work otherwise preserved): - Restore the merged tenant-header contract (#22414/#21232). The PR had changed _VikingClient defaults to '' and made empty account/user OMIT the tenant headers; main's contract is that empty falls back to 'default' and the X-OpenViking-Account/User headers are ALWAYS sent (ROOT API keys need them). Reverted the constructor to 'account or os.environ.get(..., "default")' and updated the two PR tests that asserted the omit-when-empty behavior. - Close a secret-file TOCTOU in the setup writers. _write_env_vars and _write_ovcli_config wrote the api_key/root_api_key file and chmod 0600 AFTERWARD, leaving a world-readable window on newly-created files. Added _precreate_secret_file() to create with 0600 before any secret bytes land.

…-scaffolding fix(tests): type-correct OpenViking skill-scaffolding test sentinels

) Resolves conflicts from the OpenViking churn that merged after NousResearch#32445 was opened (NousResearch#48042/NousResearch#47662 session-switch + write hardening, NousResearch#47311/NousResearch#47973): - plugins/memory/openviking/__init__.py: keep both __init__ field groups (the PR's _runtime_start_* alongside main's _prefetch_threads/_shutting_down). - tests/plugins/memory/test_openviking_provider.py: keep BOTH the PR's new setup-validation tests and main's session-switch/concurrency tests (disjoint additions to the same region). Two fixes layered while reconciling (contributor work otherwise preserved): - Restore the merged tenant-header contract (NousResearch#22414/NousResearch#21232). The PR had changed _VikingClient defaults to '' and made empty account/user OMIT the tenant headers; main's contract is that empty falls back to 'default' and the X-OpenViking-Account/User headers are ALWAYS sent (ROOT API keys need them). Reverted the constructor to 'account or os.environ.get(..., "default")' and updated the two PR tests that asserted the omit-when-empty behavior. - Close a secret-file TOCTOU in the setup writers. _write_env_vars and _write_ovcli_config wrote the api_key/root_api_key file and chmod 0600 AFTERWARD, leaving a world-readable window on newly-created files. Added _precreate_secret_file() to create with 0600 before any secret bytes land.