fix(autonomy): stop the toggle from erasing per-schedule enabled intent (#1945) - #1949
Conversation
…nt (#1945) `set_autonomy_status_logic` looped `db.set_schedule_enabled(id, enabled)` over every schedule on the agent, unfiltered and in both directions, so the agent gate and the per-schedule `enabled` flag shared one write path and only one survived. The first toggle destroyed per-schedule intent: a schedule the owner deliberately disabled was silently re-armed on the next autonomy-on, and a template-authored `enabled: false` was erased the same way. Autonomy-off was a set-all, not a pause -- nothing remembered the prior state. Since a template can materialize up to 20 declared schedules at creation, one unrelated toggle could arm all of them at once. The toggle is now a gate: it writes only `agent_ownership.autonomy_enabled`. The scheduler's existing cron-only gate (`_execute_schedule_with_lock` -> `get_autonomy_enabled`) is unchanged and now carries the whole load, so an enabled schedule on a paused agent is a normal state -- skipped, no execution row, `next_run_at` projection advanced (#1472), labelled "Will not fire -- autonomy off" in the UI (#1796). Manual triggers still bypass autonomy. No schema change: the fix is the removal of a write, so there is no column to migrate on either track. Existing rows are never rewritten -- an agent already flattened to all-disabled by a pre-fix toggle stays that way (the erased intent is unrecoverable) and the response now says so instead of silently re-arming. The response drops `schedules_updated` -- a count of a write that no longer happens -- for `total_schedules`, `enabled_schedules` and a server-authored `message` that names the case (no schedules / all disabled / N of M will run). `AgentDetail.vue` renders that message; both stores return the counts. Tests: `test_1945_autonomy_preserves_schedule_intent.py` covers the AC5 off->on cycle, proves no schedule row is written at all (unchanged `updated_at`/`next_run_at`, which `set_schedule_enabled` would bump), pins the response contract, and source-pins the scheduler gate that now carries the load. Verified failing against the pre-fix service. `test_1557`'s "still suppresses proactive work" guard was rewritten to pin the gate write and forbid the fan-out. Related to #1945 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Resolve by running |
vybe
left a comment
There was a problem hiding this comment.
/validate-pr: PASS.
- Base
dev✅ · 15 files ✅ · security scan clean ✅ · no packaging gaps ✅ - Docs: requirements + architecture + 4 feature flows ✅
- Test adequacy: non-happy-path coverage present, named to #1945 ✅
Verified the load-bearing claim. This PR deletes the per-schedule bulk write, so autonomy-off only still stops proactive work if the scheduler independently gates on the agent flag. It does: cron firing exists in exactly one place — src/scheduler/service.py:844, _execute_schedule_with_lock — and it returns early on not get_autonomy_enabled(agent_name) for triggered_by == "schedule", then advances next_run_at only (#1472). There is no second cron fire path in the backend (git grep get_autonomy_enabled over src/backend finds only read/report sites). So the gate is real and pre-existing, and the deletion is safe in both directions.
Removing a destructive bulk edit that silently re-armed owner-disabled and template-authored enabled: false schedules is the right fix, and it gets more load-bearing once #1946 lets a template materialize up to 20 schedules.
issue-status-on-merge.yml will NOT promote it. Bumping the label by hand at merge.
dev has since taken #1913/#1937/#1947/#1949/#1899. Seven conflicts, resolved so that no side's change is lost: SOURCE - static_checks.py the one real semantic conflict. ent#128 (#1899) flipped the per-check swallow from _skip to _fail so a crashed check is counted by _counts; ent#89 kept _skip and added logging. Taking this branch's side verbatim would have silently reverted the HARD-count fix. Merged: _fail from #1899 + logger.error(exc_info=True) from ent#89, which is strictly more diagnostic than the logger.warning it replaces. The docstring directly above already asserts "a check that could not evaluate is not a check that passed". - template_service three hunks, all adjacent additions: both import blocks kept (template_schedules got its own statement — the two sides shared a closing paren), both new functions kept, and both pre-literal computations kept at each of the two call sites. - crud.py import list, both symbols kept. DOCS - architecture.md two hunks where BOTH sides had edited the same three bullets (template_service / fork_to_own / crud). Not a pick — each line was 3-way merged at word granularity against the merge base; no edit pairs overlapped, so both sides' text survives verbatim. dev's bullet order preserved, ent#89's new template_schedules.py bullet appended. - feature-flows.md all rows kept, table stays reverse-chronological. - learnings.md both sets of entries kept. - registry.json both entry lists kept. The conflict opened after a bare '{' and closed before a bare '}', so each side was an object BODY -- a naive concatenation produced invalid JSON. Re-added the '},{' separator. 109 -> 112 entries, none dropped. Verified: zero markers tree-wide, registry.json parses (112 entries), all three touched modules compile, and every deletion vs origin/dev is one of ent#89's own intended replacements (crud docstring three->four, the cron helper replaced by the shared validator, T-018 wired into the dispatch map).
Problem
set_autonomy_status_logicwrote the autonomy flag to every schedule on the agent, unfiltered and in both directions:So the agent-level gate and the per-schedule
enabledflag shared one write path, and only one survived. Per-schedule intent was destroyed on the first toggle: a schedule the owner deliberately disabled was silently re-armed on the next autonomy-on, a template-authoredenabled: falsewas erased the same way, and autonomy-off was a set-all rather than a pause. With a template able to materialize up to 20 declared schedules at creation, one unrelated toggle could arm all of them at once.Fix — a gate, not a bulk edit (no schema change)
The toggle now writes exactly one row:
agent_ownership.autonomy_enabled. Per-scheduleenabledis owner intent and is never touched.The design note weighed a
paused_by_autonomydiscriminator column against a schema-free approach. Schema-free wins because the mechanism it needs already exists and is load-bearing: the scheduler's cron-only gate (src/scheduler/service.py::_execute_schedule_with_lock→get_autonomy_enabled) already refuses to fire for a paused agent, and #1472 already built the supporting behaviour for exactly this state — an enabled schedule on a paused agent is skipped, records no execution row, and has itsnext_run_atprojection advanced so no stale "Next: Nd ago" appears. #1796 already labels it "Will not fire — autonomy off" in the Schedules tab, and #1806 applies the same hold-don't-mutate semantics to reminders. A discriminator column would have added a dual-track migration to reproduce a state the system already models correctly.So the fix is the removal of a write. Manual triggers still bypass autonomy by design; admin fleet ops (
/api/ops/schedules/pause|resume,emergency_stop) still writeenabledin bulk, which is what those incident tools are for.Acceptance criteria
src/scheduler/service.pyis untouched; a source-level test pins the gate, since it now carries the whole load.tests/unit/test_1945_autonomy_preserves_schedule_intent.py.API change
PUT /api/agents/{name}/autonomydropsschedules_updated(a count of a write that no longer happens) in favour oftotal_schedules,enabled_schedules, and a server-authoredmessage. Only Trinity's own frontend consumed the old field (no MCP tool, no test);AgentDetail.vuenow renders the server message, and both stores return the counts.Verification
Manual: disable one of two schedules → toggle autonomy off → on → the disabled one is still disabled; the enabled one resumes. Both
updated_atandnext_run_atare unchanged on every schedule row across the cycle.Trade-off worth naming
A paused agent's schedules stay registered with the scheduler and tick-and-skip (one gate read, and at most one
next_run_atrepair write, per cron occurrence — no execution row, no LLM spend). That is the pre-existing #1472 path, previously reached only when someone enabled a schedule on a paused agent; it is now the normal paused state. I deliberately did not add anautonomy_enabledfilter to the scheduler'slist_all_enabled_schedules: that would stop the projection repair and make canary E-06 (overduenext_run_aton an enabled schedule) fire for every paused agent.Docs
architecture.mdendpoint row,feature-flows/autonomy-mode.md(overview/side-effects/testing/revision history),autonomy-toggle-component.md,agent-network.md,agents-page-ui-improvements.md, requirementsscheduling.md§10.2.1, and the four user-doc claims that said the toggle switches all schedules on and off.Related to #1945
🤖 Generated with Claude Code