Skip to content

fix(cron): keep fallback routing atomic and preserve unknown outcomes (#90089) - #90293

Open
andrexibiza wants to merge 1 commit into
NousResearch:mainfrom
andrexibiza:fix/pr-90122-review-blockers
Open

fix(cron): keep fallback routing atomic and preserve unknown outcomes (#90089)#90293
andrexibiza wants to merge 1 commit into
NousResearch:mainfrom
andrexibiza:fix/pr-90122-review-blockers

Conversation

@andrexibiza

@andrexibiza andrexibiza commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Supersedes #90122 as the sole current-base merge candidate while preserving @smfworks / Paula Rossi's implementation provenance and #90089's reporter lineage.

The verified repair is compacted into one independently testable commit rebased directly onto current main. Obsolete intermediate SHAs with failed or cancelled CI receipts are no longer part of the PR history. @smfworks' original implementation remains explicitly credited through its immutable source SHA, the contributor mapping, and the final commit's Co-authored-by trailer.

  • Keep provider + model as one configured fallback route. Every explicit job pin must match the candidate route; incompatible entries are skipped and the run fails closed instead of synthesizing a pair such as lmstudio/glm-4.5-air.
  • Preserve verified same-backend credential transports: xai-oauth and xai are explicitly one xAI provider family, so a pinned xai-oauth/grok-4.5 route may fall back to configured xai/grok-4.5 without weakening the model pin or allowing cross-provider substitution.
  • Add an immutable execution-ledger CAS for evidence-poor outcomes. A body that exits without a durable terminal result settles its exact execution as unknown; failed remains reserved for positive failure evidence.
  • Reclaim provably dead manual-run owners before dispatch so one-shot CLI runs do not inherit permanent running/claimed wedges.

Provenance

Validation

GitHub-hosted exact-target repair runs completed before publication:

Passed before publication:

  • git diff --check
  • modified-file py_compile
  • tests/cron/test_model_pin_fallback_90089.py
  • tests/cron/test_scheduler.py
  • tests/cron/test_silent_worker_death_90089.py
  • tests/cron/test_execution_ledger.py

Exact-head GitHub Actions for 00be78a2ecbed585958229858aa0ac7b38236c70:

Current published head: 00be78a2ecbed585958229858aa0ac7b38236c70.

Refs #90089. Supersedes #90122 with contributor credit preserved.

andrexibiza added a commit to andrexibiza/hermes-agent that referenced this pull request Aug 19, 2026
The repaired source now lives on the dedicated branch and upstream PR NousResearch#90293; remove the fork-main trigger.
@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/cli CLI entry point, hermes_cli/, setup wizard sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation needs-decision Awaiting maintainer decision before any implementation labels Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

Exact-head hosted verification receipt

Verified the live PR head e2f6661a44fcb92ae8d23561d543e9a434a58cb0 directly in NousResearch/hermes-agent:

  • CI 32306679057success
  • Docker Build, Test, and Publish 32306678386success
  • Nix flake check 32306678299success

This replaces the body’s reliance on the earlier fork-side targeted repair runs as merge evidence. The published cron fallback/unknown-outcome head itself is green in the repository’s normal hosted matrix.

@andrexibiza
andrexibiza force-pushed the fix/pr-90122-review-blockers branch from e2f6661 to 0d17340 Compare August 21, 2026 13:45
…ousResearch#90089)

Squash the verified NousResearch#90293 repair tree onto current main so the PR contains one independently testable commit instead of obsolete intermediate SHAs with failed CI receipts.

- reclaim provably dead manual-run execution owners before dispatch
- settle evidence-poor terminalization as unknown through an immutable ledger CAS
- keep provider/model fallback routes atomic and fail closed on incompatible pins
- preserve verified xai-oauth/xai same-service credential transport compatibility
- retain the focused regression witnesses and contributor mappings

Original implementation: 2b119d2 by Paula Rossi (@smfworks).
Review repairs: NousResearch#90293.

Co-authored-by: Paula Rossi <paula@smfworks.com>
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

Solid piece of durability engineering. The pin-compatibility filter (one configured provider/model route or nothing) fixes the synthetic-route bug class rather than the reported zai symptom, the xai/xai-oauth family collapse is data-driven and deliberately narrow ("never infer compatibility from suffixes" is the right rule), and settling lost-terminal executions as immutable unknown via CAS — instead of guessing failed/succeeded — is exactly the correct epistemics for "side effects may have run".

Points to consider:

  1. Silent safety net (cron/scheduler.py, the finally block calling mark_execution_unknown): the surrounding try/except Exception: pass means a persistently failing ledger write produces zero signal — the one path where you'd want noise. A logger.warning(..., exc_info=True) inside the except keeps the net best-effort without making it invisible.
  2. Issue refs in user-visible errors: "See #90089." in execution error details and the pinned-route RuntimeError leak your tracker numbering to cron delivery surfaces (email/chat/webhook readers). Move the reference into a code comment/docstring and keep the operator-facing text self-contained ("whether side effects ran is unknown" already says everything needed).
  3. Model-pin exact match: fb_model != pinned_model is byte-exact, so model="GLM-4.5-Air" vs fallback glm-4.5-air fails closed. Conservative direction is right, but model ids are routinely cased inconsistently across configs — a casefold comparison (or normalizing both sides through the same helper) would remove a likely false-skip without weakening the guarantee.
  4. Tree additions: contributors/emails/* in a cron fix PR looks like salvage-provenance metadata — worth confirming it matches this repo's contributor-tracking convention before merge, since unrelated-looking additions draw review friction.
  5. Nice touch on test_real_subprocess_stale_running_is_reclaimed — a genuine two-process E2E for the reaper beats mocked liveness checks, and the process_started_at=NULL rows exercising the dead-owner path match how crashed workers actually die.

@andrexibiza

Copy link
Copy Markdown
Contributor Author

Cron fallback routing and unknown-outcome handling are consolidated at head 00be78a2ecbed585958229858aa0ac7b38236c70. 00be78a2ecbe keeps provider and model pins as one compatible route, permits only the explicit xai-oauth to xai same-service credential pairing, reclaims provably dead manual-run owners, and settles evidence-poor terminal states through an immutable ledger CAS as unknown rather than guessing success or failure. The focused repair preserves the #90089 behavior boundary and contributor attribution while making incompatible fallback construction fail closed. The exact-head GitHub check rollup is SUCCESS.

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

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard comp/cron Cron scheduler and job management needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants