feat(cron): expose structured cron context via adapter send metadata (#26004)

[briandevans]

What does this PR do?

cron/scheduler.py's _deliver_result calls adapter.send(chat_id, content, metadata=...) with only thread_id in metadata. Plugin adapters that want the cron's job_id, job_name, schedule, deliver, or origin today have to regex-parse the "Cronjob Response: <name>\n(job_id: <id>)" envelope — which is brittle and breaks entirely when cron.wrap_response: false is configured.

This PR adds those fields under metadata["cron"] as a structured side-channel, keeping the existing metadata["thread_id"] key. Implements Step 1 (minimal, fully backwards-compatible) of #26004. Adapters that ignore unknown metadata keys are unaffected — they already received {"thread_id": ...} today and continue to.

Related Issue

Fixes #26004 (Step 1)

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)
• ✨ New feature (non-breaking change that adds functionality)
• 🔒 Security fix
• 📝 Documentation update
• ✅ Tests (adding or improving test coverage)
• ♻️ Refactor (no behavior change)
• 🎯 New skill (bundled or hub)

Changes Made

• cron/scheduler.py — build cron_meta = {"job_id", "job_name", "schedule", "deliver", "origin"} (filtered to non-empty values) from data already in scope in _deliver_result, then merge as send_metadata["cron"] = cron_meta alongside the existing thread_id key. No run_job signature change.
• tests/cron/test_scheduler.py — TestDeliverResultCronMetadata covering populated fields, name fallback to job_id, coexistence with thread_id, omission of empty optional fields (4 cases).

How to Test

1. uv run --with pytest --with pytest-xdist --with pytest-asyncio python3 -m pytest tests/cron/test_scheduler.py::TestDeliverResultCronMetadata -v
2. Expected: 4 passed.
3. Adjacent: uv run --with pytest --with pytest-xdist --with pytest-asyncio python3 -m pytest tests/cron/ -v — 341 passed.
4. Regression guard: revert production hunk — all 4 new tests fail with TypeError: 'NoneType' object is not subscriptable on metadata["cron"]. Restore — green.

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix/feature (no unrelated commits)
• I've run focused tests for the touched code and all pass (4/4 + 341/341 adjacent)
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: macOS 15.x

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) — N/A, side-channel metadata is plugin-facing
• I've updated cli-config.yaml.example if I added/changed config keys — N/A, no config change
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
• I've considered cross-platform impact (Windows, macOS) per the compatibility guide — platform-independent dict construction
• I've updated tool descriptions/schemas if I changed tool behavior — N/A

Related / Positioning

• Fixes [Feature]: Pass structured cron metadata (job_id, response_id) to BasePlatformAdapter.send via metadata= #26004 — Step 1 (locally-available fields). Step 2 (response_id and session_id) would require widening run_job's (success, output, final_response, error) return tuple — out of scope for a minimal patch. Happy to follow up if the maintainer wants that wired through.

Audited siblings: _deliver_result is the single metadata-build site for cron-originated sends. No widening needed.

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a structured cron side-channel to the metadata kwarg passed to live platform adapters in _deliver_result, so plugin adapters can recover cron job context (id/name/schedule/deliver/origin) without regex-parsing the wrapped envelope text.

Changes:

• Build a cron_meta dict with job context and merge it into send_metadata alongside thread_id.
• Add a new TestDeliverResultCronMetadata test class covering presence, name fallback, coexistence with thread_id, and omission of empty fields.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
cron/scheduler.py	Constructs cron metadata dict and merges into send_metadata for live adapter sends.
tests/cron/test_scheduler.py	Adds tests verifying cron metadata is forwarded to live adapter send calls.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[briandevans]

@copilot Both findings addressed in 362ab6d49:

1. Hoisted cron_meta above the for target in targets: loop — every field reads job.* (and a per-job-resolved origin), all invariant across targets. Now built once before the loop; only thread_id varies per iteration.
2. Switched the strip-empty filter from truthiness to is not None — falsy-but-meaningful values (e.g. an explicitly-empty schedule a future job type might set) now reach adapters. Only true absence is dropped.

[briandevans]

CI audit — all 7 test failures (3 in test, 4 in e2e) are pre-existing baselines on clean origin/main (4695d2716). Zero failures are in touched code (cron/scheduler.py / tests/cron/test_scheduler.py).

Branch is now rebased onto current origin/main (was 4 commits behind earlier; the previous CI run picked up stale copies of tests/agent/test_context_compressor_summary_continuity.py, tests/run_agent/test_compression_feasibility.py, tests/providers/test_plugin_discovery.py, etc., causing 12 spurious test failures that no longer reproduce post-rebase).

Test	Symptom	Root cause on main
tests/run_agent/test_provider_parity.py::TestDeveloperRoleSwap::test_developer_role_via_nous_portal	ValueError: Model has a context window of 15,000 tokens	model_metadata caches stale 15K context for inference-api.nousresearch.com/v1; test resolves nameless model → fails minimum-64K guard at run_agent.py:2349
tests/run_agent/test_provider_parity.py::TestBuildApiKwargsNousPortal::test_includes_nous_product_tags	same ValueError: 15,000 tokens	same root cause
tests/run_agent/test_provider_parity.py::TestBuildApiKwargsNousPortal::test_uses_chat_completions_format	same ValueError: 15,000 tokens	same root cause
tests/e2e/test_discord_adapter.py::TestMentionStrippedCommandDispatch::test_mention_then_command	TypeError: catching classes that do not inherit from BaseException is not allowed	gateway/platforms/discord.py:3730 except clause catches a SimpleNamespace (mocked fixture) — not a BaseException subclass
tests/e2e/test_discord_adapter.py::TestMentionStrippedCommandDispatch::test_nickname_mention_then_command	same TypeError	same root cause
tests/e2e/test_discord_adapter.py::TestMentionStrippedCommandDispatch::test_text_before_command_not_detected	same TypeError	same root cause
tests/e2e/test_discord_adapter.py::TestAutoThreadingPreservesCommand::test_command_detected_after_auto_thread	same TypeError	same root cause

Reproduced both clusters locally on clean origin/main (4695d2716) with identical error text.

[deestax]

Hi @briandevans 👋 — I opened #29291 yesterday without spotting this PR; just closed mine and pointed at yours. After re-reading both, your structured-side-channel is the right shape for #26004 (centralized field set, fixed contract adapter authors can rely on, easier to extend to Step 2 — response_id is just another key in cron_meta_full once run_job's tuple widens).

A few extensions the failure-route / audit-logging / inbox-UI use cases would benefit from, in case any of these are in scope for this PR (happy to defer to a follow-up if not):

1. status field

cron_meta_full = {
    ...
    "status": "ok",   # or "error" when run_job returned success=False
}

tick() knows the success/failure state of each run, so it could thread a status_hint kwarg through _deliver_result (status_hint='ok' if success else 'error') and into cron_meta_full. Failure-route adapters (e.g. an inbox UI that needs to render failed jobs differently, or an audit-logging adapter stamping severity) need this and can't reliably infer it from the wrapped content.

2. ran_at field

cron_meta_full = {
    ...
    "ran_at": _hermes_now().isoformat(),
}

The execution timestamp is invaluable for any adapter routing cron output to a UI that displays "when did this fire?". Cheap to add at the same call site as the existing fields.

3. Failure-path regression test

Your current TestDeliverResultCronMetadata covers four happy paths cleanly. One additional case that would harden the contract: verify metadata["cron"] is still attached on the wrap_response=false + failed-run combination (i.e., when the content is unwrapped AND we'd want adapters to be able to detect the run failed). Probably 30 lines on top of your _build_adapter_send_mock helper.

I have a working implementation of all three in a branch (extracted from my closed #29291) if you'd like me to send a follow-up PR against your branch, or you'd rather fold them into this one — your call.

Either way, thanks for picking this up — your hoisting fix on the per-target loop and the is not None filtering both read better than what I had. Looking forward to seeing this land.

[briandevans]

Thanks @deestax — appreciate the deferential close. Happy to incorporate any specific shape feedback from #29291 (e.g. extra fields you needed for Step 2) into a fix-up here if useful.

[deestax]

Cool, here's what those would look like in code so you can copy-paste any/all/none. Each one's independent.

1. status field (~6 lines)

Thread status_hint through _deliver_result — tick() already knows the run's success bool, so it's just plumbing:

# signature
def _deliver_result(
    job: dict,
    content: str,
    adapters=None,
    loop=None,
    status_hint: str = "ok",   # NEW
) -> Optional[str]:

# inside _deliver_result, add to cron_meta_full where you build it now:
"status": status_hint,

# tick() callsite (~line 1913)
delivery_error = _deliver_result(
    job, deliver_content, adapters=adapters, loop=loop,
    status_hint="ok" if success else "error",
)

2. ran_at field (1 line)

_hermes_now is already imported at the top of the file — just one more key in cron_meta_full:

"ran_at": _hermes_now().isoformat(),

3. Failure-path test (~30 lines)

A clone of one of your existing TestDeliverResultCronMetadata cases, with a failed run + wrap_response=false:

def test_cron_metadata_attached_on_failed_run_with_wrap_disabled(self):
    from gateway.config import Platform

    adapter, loop, fake_run_coro = self._build_adapter_send_mock()
    pconfig = MagicMock()
    pconfig.enabled = True
    mock_cfg = MagicMock()
    mock_cfg.platforms = {Platform.TELEGRAM: pconfig}

    job = {
        "id": "fail-job",
        "name": "nightly-report",
        "deliver": "origin",
        "origin": {"platform": "telegram", "chat_id": "999"},
    }

    with patch("gateway.config.load_gateway_config", return_value=mock_cfg), \
         patch("cron.scheduler.load_config", return_value={"cron": {"wrap_response": False}}), \
         patch("asyncio.run_coroutine_threadsafe", side_effect=fake_run_coro):
        _deliver_result(
            job,
            "Job hit an exception",
            adapters={Platform.TELEGRAM: adapter},
            loop=loop,
            status_hint="error",
        )

    cron_ctx = adapter.send.call_args.kwargs["metadata"]["cron"]
    assert cron_ctx["status"] == "error"
    assert cron_ctx["job_id"] == "fail-job"

If you'd rather cherry-pick than retype, happy to push a branch on my fork — just say. Otherwise this is just for reference, no obligation.

[briandevans]

Thanks @deestax — pulled all three into f758e5496:

• status field — added status_hint: str = "ok" to _deliver_result, threaded status_hint="ok" if success else "error" from tick(), included "status": status_hint in cron_meta_full.
• ran_at field — "ran_at": _hermes_now().isoformat() in cron_meta_full (already imported).
• Failure-path test — test_cron_metadata_attached_on_failed_run_with_wrap_disabled asserts cron_ctx["status"] == "error" plus full context survival when wrap_response=false.

Also updated the existing test_cron_metadata_omits_empty_fields exact-match assertion to pop the (clock-dependent) ran_at and expect status: "ok" on the default path. Focused tests: 5/5 pass; full tests/cron/test_scheduler.py: 126/126 pass.

The defaulting of status_hint="ok" keeps the existing public-API call sites (_deliver_result(job, content, adapters=..., loop=...)) unchanged — only tick() opts into the failure path, so this stays additive for any out-of-tree callers.

[briandevans]

CI audit — the only failure in the touched scope was tests/cron/test_scheduler.py::TestDeliverResultTimeoutCancelsFuture::test_live_adapter_thread_fallback_records_delivery_error. That test was added in d81b888 (after my last rebase) and asserts metadata={"thread_id": "7072"} exactly — incompatible with the new metadata["cron"] block this PR adds, but the load-bearing invariant (thread_id preserved on fallback) is unchanged.

Pushed 49cb370f6:

• Rebased onto current origin/main (318 commits forward — clean rebase, no conflicts in scheduler.py).
• Updated the assertion to check call count, positional args, metadata["thread_id"] == "7072", and a couple of metadata["cron"] fields so the structured path stays covered.

The other two test job failures (tests/plugins/web/test_web_search_provider_plugins.py::test_all_seven_plugins_present_in_registry — missing xai from the expected plugin list; tests/hermes_cli/test_update_hangup_protection.py::test_wraps_stdout_and_stderr_with_mirror — _UpdateOutputStream identity check) reproduce on clean origin/main and are not in touched code.

[briandevans]

Closing to focus the queue on security/file-safety work where civilian merges are landing. Happy to reopen if maintainers want this picked up.