fix(hosted/orchestration): scope turn origin for local-agent spawn so cron tools aren't denied - #5527
Conversation
`AGENT_TURN_ORIGIN` is a `tokio::task_local`, so it does not cross `tokio::spawn`. `effect_executor::run_local_agent` fires the local sub-agent from a bare `tokio::spawn` with no agent turn on the stack, so `turn_origin::current()` was `None` on the spawned task, the approval gate read `AgentTurnOrigin::Unknown`, and every external_effect tool the sub-agent called (cron_add / cron_update / shell / …) was denied as "agent turn has no origin label". PR tinyhumansai#5465 fixed the four canonical spawn sites (orchestration spawn_agent, spawn_async_subagent, workflow runs, agent teams) via `capture`/`propagate`/`with_inherited_origin`. This closes the residual `effect_executor` gap. There is no ambient turn to inherit here (the device-tool bridge is not itself an agent turn), so an explicit `AgentTurnOrigin::Cli` is scoped around the spawned work — this path only runs after the device-authoritative Master-chat gate in `dispatch_device_tool` has passed, matching the turn-less internal dispatch label already used by `mcp::registry` and `task_dispatcher`. Also makes the gate's `Unknown` deny message specific and actionable: it names the missing origin label, the scheduling/external-effect tools it blocks, and frames it as an internal wiring gap rather than user error, without leaking internals. Tests: gate unit test that a `cron_add` call on a turn-less/delegated spawn resolves to a real origin (allowed) instead of `Unknown`-denied; turn_origin propagation test covering the explicit-`Cli`-across-spawn shape of the effect_executor site. Closes tinyhumansai#5508, tinyhumansai#5499
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe change establishes an explicit CLI origin for background local-agent tasks, preserves origin visibility across spawned tasks, improves unknown-origin denial guidance, and adds regression coverage for denied and allowed ChangesOrigin propagation and approval
Estimated code review effort: 3 (Moderate) | ~25 minutes Mergeability Score: 🔵 Low · up to The PR restores hosted background scheduling by assigning the work an existing broad command-line trust category, which also enables other eligible external-effect actions on that path. Local master-cycle checks limit reachability, but the remaining bounded security and audit risk is that hosted automation shares this broad authority and the provenance binding for the triggering cycle is not fully established; the change is mergeable with explicit owner awareness. Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…ect-executor-5508
There was a problem hiding this comment.
tinysweeper found nothing blocking. Approving.
$0.0344 · 30,885 in / 11,466 out · 26,008 cached (84%) · z-ai/glm-5.2
critique: $0.0159 · 10,587 in / 5,686 out · 9,095 cached (86%) · z-ai/glm-5.2
security: $0.0107 · 9,827 in / 3,484 out · 8,098 cached (82%) · z-ai/glm-5.2
tests: $0.0058 · 4,790 in / 1,952 out · 4,010 cached (84%) · z-ai/glm-5.2
description: $0.0021 · 5,681 in / 344 out · 4,805 cached (85%) · z-ai/glm-5.2
What this change touches4 files, +141 -16 across 4 components. The code graph knows nothing about these files yet — normal for newly added files, and a cold index otherwise. flowchart LR
n0["src/openhuman/security/approval<br/>1 file +68 -4"]:::changed
n1["src/openhuman/hosted/orchestration<br/>1 file +44 -11"]:::changed
n2["src/openhuman/agent<br/>1 file +28 -0"]:::changed
n3["tests/raw_coverage<br/>1 file +1 -1"]:::changed
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Green: changed. Grey: untouched, reached through an import or a call. Orange: has findings. Red: has a finding that blocks the merge.
Changed files
|
Summary
cron_add/cron_update(and every external-effect tool) failing with "agent turn has no origin label" when the turn's work runs on a spawned task.AGENT_TURN_ORIGINis atokio::task_local, which does not crosstokio::spawn; the hosted-orchestration local-agent path spawned without re-scoping it, so the approval gate sawAgentTurnOrigin::Unknownand fail-closed-denied.Problem
PR #5465 fixed the four canonical spawn sites (
agent/orchestration/{ops, workflow_runs/engine, tools/spawn_async_subagent, agent_teams/runtime}) so the turn origin survives delegation. One site was missed:src/openhuman/hosted/orchestration/effect_executor.rs::run_local_agentfiresrun_local_agent_and_forwardfrom a baretokio::spawnwith no origin scope. On that taskturn_origin::current()isNone→ the gate treats it asAgentTurnOrigin::Unknown(security/approval/gate.rs) and refuses external-effect tools with the raw "no origin label" marker. This is the multi-core / intermittentcron_addfailure reported in #5508 and #5499 (the Settings→Cron UI path is unaffected because it writes the store directly, never through the gate).Solution
effect_executor.rs: wrap the spawned agent future inturn_origin::with_origin(AgentTurnOrigin::Cli, …). This path is post-approval device automation with no ambient turn (current()isNone, sowith_inherited_originwould stayUnknown);Climatches the repo's existing turn-less internal-dispatch precedent (mcp/registry/ops.rs,agent/task_dispatcher/executor.rs). The gate is not weakened, cron is not granted at the gate, and no label is manufactured insiderun_subagent.gate.rs: rewrite theUnknowndeny message to state, specifically and actionably, that the external-effect/scheduling tool was refused because the turn carried no origin label — not a generic permissions error.Submission Checklist
gate.rs:cron_addon a baretokio::spawndenies with "origin label", and under an explicitwith_origin(Cli, …)scope resolves to a real origin and is allowed (across a real spawn boundary).turn_origin.rs: an explicitCliorigin survives a turn-less spawn. Updated the existing deny-message assertions to the new copy.ci-lite.yml); changed lines covered by the added gate/origin tests.cargo testgreen locally.N/A: behaviour-only change(security-gate regression fix, no new feature surface).## Related—N/A: behaviour-only change.N/A.N/A: no release-cut smoke surface.Closesin## Related.Impact
mainbut not yet in a release, so shipped builds need a release to carry the full fix.Related
TrustedAutomationsource for hosted device-exec if per-source cron auditing is wanted (larger change;Cliis the correct minimal fix here).AI Authored PR Metadata (required for Codex/Linear PRs)
N/A— human-authored.Summary by CodeRabbit