Skip to content

fix(cron,skills): tolerate non-UTF-8 cron archives and org markers - #118331

Open
beardthelion wants to merge 1 commit into
NousResearch:mainfrom
beardthelion:fix/nonutf8-failsafe-readers
Open

beardthelion wants to merge 1 commit into
NousResearch:mainfrom
beardthelion:fix/nonutf8-failsafe-readers

Conversation

@beardthelion

Copy link
Copy Markdown

Fixes #118330.

read_text(encoding="utf-8") raises UnicodeDecodeError (a ValueError, not an OSError), so readers whose except names only OSError crash on undecodable files instead of degrading as their own contracts promise. Two live sites:

  • cron/scheduler_prompt.py _inject_context_from: a corrupt .md in a context_from source dir escaped the (OSError, PermissionError) guard and crashed _build_job_prompt - which runs outside run_job's try - on every fire. The fix adds a per-file guard so a corrupt archive is skipped like a silent/blank one and iteration falls through to the next older archive, instead of skipping the whole source job.
  • agent/skill_utils.py read_active_org_id: a non-UTF-8 .active_org marker escaped into prompt_builder._build_skills_manifest, failing system-prompt assembly on every turn. Now returns None as the docstring promises.

Sibling arm agent/skill_bundles._load_bundle_file is already covered by open PR #112180 (same UnicodeError gap) and is intentionally not duplicated here.

Test plan

  • 4 new tests, all RED pre-fix (UnicodeDecodeError escapes), GREEN post: corrupt-newest archive falls through to older, corrupt-only source skipped, _build_job_prompt e2e survives corrupt archive, undecodable .active_org marker returns None and iter_skill_index_files survives
  • tests/cron/ + skill suites: 1606 pass; only the 4 pre-existing host-umask test_file_permissions failures (identical on unmodified source)
  • ruff check clean on all touched files

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cron Cron scheduler and job management comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint tool/skills Skills system (list, view, manage) labels Sep 21, 2026
read_text(encoding="utf-8") raises UnicodeDecodeError - a ValueError,
not an OSError - so readers guarded only by except OSError crash instead
of degrading:

- _inject_context_from: one corrupt .md in a source job's output dir
  crashed _build_job_prompt (which runs outside run_job's try), so the
  downstream job failed on every fire while the file remained. The read
  now skips the file like a silent/blank archive, so an older usable
  archive still gets used instead of masking the whole source.
- read_active_org_id: a non-UTF-8 .active_org marker escaped its
  fail-safe contract ("None = no org skills load") into
  _build_skills_manifest during system-prompt assembly - every turn.
  Now returns None as documented.

The sibling arm in skill_bundles._load_bundle_file is already covered
by open PR NousResearch#112180, which adds the same UnicodeError branch there.
@beardthelion
beardthelion force-pushed the fix/nonutf8-failsafe-readers branch from 2f9a65c to 2d88dbc Compare September 21, 2026 16:26
@kyssta-exe

Copy link
Copy Markdown

Summary

Fixes a real crash class: Path.read_text(encoding="utf-8") raises UnicodeDecodeError (a ValueError, not OSError), so two readers whose guards named only OSError crashed instead of degrading. Two minimal guards plus 4 tests that were RED pre-fix and GREEN post-fix.

What changed

  • cron/scheduler_prompt.py (_inject_context_from): per-file (OSError, UnicodeDecodeError) guard with a warning log; a corrupt newest archive falls through to the older good one instead of killing _build_job_prompt on every fire.
  • agent/skill_utils.py (read_active_org_id): catches UnicodeDecodeError, returns None as the docstring already promised, so prompt assembly survives an undecodable .active_org marker.
  • Tests: corrupt-newest falls through, corrupt-only source skipped, _build_job_prompt e2e survives, undecodable marker -> None with iter_skill_index_files intact.

Strengths

  • The per-file continue (rather than aborting the source loop) is the right granularity — one bad archive no longer poisons the whole context_from source.
  • Explicitly scoping out skill_bundles._load_bundle_file (owned by fix(skills): resolve same-root duplicate names instead of refusing them #112180) avoids a duplicate-fix conflict.
  • Failure mode matches each function's existing contract (skip-like-blank / return-None) instead of inventing new behavior.

Findings

  • Consider catching UnicodeError (the parent) rather than UnicodeDecodeError in both sites: read_text decoding surfaces as UnicodeDecodeError today, but the parent costs nothing and also covers exotic UnicodeEncodeError paths if these helpers ever write. Non-blocking either way.
  • Non-blocking: the new warning log in _inject_context_from includes the exception but not the job id — adding it would make corrupt-archive reports attributable in multi-source setups.

Verdict

Looks good to merge.

Reviewed using Hermes-Agent

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cron Cron scheduler and job management P2 Medium — degraded but workaround exists tool/skills Skills system (list, view, manage) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(cron,skills): non-UTF-8 archives and org markers escape fail-safe readers

3 participants