Skip to content

fix(runtime): route "run a loop" to Trinity loops, not the harness's own (#2454) - #2458

Merged
vybe merged 2 commits into
devfrom
fix/2454-loop-routing
Sep 1, 2026
Merged

fix(runtime): route "run a loop" to Trinity loops, not the harness's own (#2454)#2458
vybe merged 2 commits into
devfrom
fix/2454-loop-routing

Conversation

@dolho

@dolho dolho commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

What

Asked to "run a basic loop 3 times", agents picked Claude Code's built-in loop skill — a better lexical match for the word than the MCP tool run_agent_loop — which paces itself with the harness ScheduleWakeup tool. A Trinity execution is a one-shot claude --print: the process exits at end of turn and any pending wakeup dies with it. ScheduleWakeup returned "Next wakeup scheduled … the harness re-invokes you when the wakeup fires" anyway, so the agent truthfully told the user "loop armed — next tick in ~60s" while there were zero agent_loops rows, zero agent_reminders rows and zero follow-up executions. No error anywhere — that is the bug, and every agent on a current base image carries it.

Two layers, both needed. Neither alone is sufficient: guidance is advisory against a same-named affordance whose result says it worked, and a deny with no positive route leaves the model with a refusal and nowhere to go.

1. Mechanical — deny the mechanism at spawn

_runtime_config.PLATFORM_DENIED_TOOLS (ScheduleWakeup) plus one builder, merged_disallowed_tools(guardrails), that unions it with the operator's GUARD-003 deny-list (order-stable, deduped, junk-tolerant, never raises). Both Claude spawn sites — chat (claude_code.py) and headless task (headless_executor.py) — now build --disallowedTools through it, so an operator guardrails edit can neither drop the platform denials nor duplicate them.

Denying the mechanism is what converts the wrong path from silent success to a visible refusal.

Denied by tool NAME only. A Skill(loop) rule was considered and rejected: the Skill specifier grammar is not a contract we can pin while the base image tracks the latest CLI, and an unparseable permission rule risks the whole --disallowedTools argument on every turn — a fleet outage traded for a second layer over the mechanism the deny already removes. Codex spawns are untouched (different harness, no ScheduleWakeup, and they pass no --disallowedTools).

2. Guidance — platform-injected, not per-agent

A ### Repeating Work and Deferred Ticks section in PLATFORM_INSTRUCTIONS routing repetition to mcp__trinity__run_agent_loop / mcp__trinity__set_reminder, and naming the two harness affordances to avoid and why they look like they worked. Platform-injected per the issue's explicit ask, so every existing and future agent gets it with no workspace edit.

It is registered as an ALWAYS section (in _KNOWN_SECTION_HEADINGS, out of _MINIMAL_DROP_SECTIONS): unlike the three tool-usage sections MINIMAL drops, its load-bearing half is a negative rule about tools that are not ours, so there is no tool description for it to fall back to. Runtime-adaptation is free — the Codex adapter strips mcp__trinity__ and leaves the harness names alone (pinned).

Rollout

  • Guidance is live on the next backend deploy.
  • Deny is in-container, so it reaches existing agents on the next base-image rebuild. Stated here rather than implied.

Verification

Live agent container, claude 2.1.220 — the CLI accepts the flag and value: an unknown option errors loudly (error: unknown option '--nonexistentflag') while --disallowedTools ScheduleWakeup behaves byte-identically to the no-flag baseline.

tests/unit/test_2454_loop_routing.py (16 tests) pins both layers:

  • The spawn-site check discovers every function under docker/base-image/agent_server/ that passes --disallowedTools (AST walk) and requires each to call merged_disallowed_tools — plus a non-vacuity assertion that at least two such sites exist. A third spawn site added later is exactly how this regresses; naming today's two would not catch it.
  • One test derives the prompt's claims from the deny list: a tool denied at spawn without a word in the guidance leaves the agent facing a refusal it cannot interpret, and guidance naming a tool the platform does not deny is a rule with no backstop.

All four mutations were confirmed to red the suite before being reverted: revert one spawn site to the raw guardrails list → the discovery test fires; add the section to the MINIMAL drop set → the tier + registration tests fire; unregister the heading → those plus test_ent243_prompt_tier fire; add a name to PLATFORM_DENIED_TOOLS that the prompt never mentions → the vocabulary test fires.

Full unit suite: 13232 passed, 26 skipped. The 15 failures are one parametrized IPv4-mapped-IPv6 SSRF boundary test (test_ent14_registry_url_ssrf) that fails identically on a clean origin/dev tree in this environment — proven by re-running it at the dev tree state, not asserted.

Also fixed (a trap this change tripped)

tests/unit/test_1402_prompt_contract.py sliced the operator-queue contract between two named headings (### Operator Communication### Package Persistence). Inserting an unrelated section between them did not break the slicer — it silently widened the slice, so its runtime-identity and MCP-name-free assertions started policing a section that legitimately carries MCP tool names and is legitimately runtime-adapted, failing as "you broke the #1402 contract" and pointing at the wrong file. The end bound is now derived (the next ### heading).

Docs

  • docs/memory/architecture.md — the multi-runtime parity surface now records the deny list, the rejected Skill(loop) alternative, and the ALWAYS-tier rationale.
  • docs/memory/learnings.md — two entries: a tool that reports success in an environment where it structurally cannot work is worse than a missing tool, and a test that slices a document between two named headings silently widens.

Fixes #2454

Asked to "run a loop", agents picked Claude Code's built-in `loop` skill —
a better lexical match for the word than the MCP tool `run_agent_loop` —
which paces itself with the harness `ScheduleWakeup` tool. A Trinity
execution is a one-shot `claude --print`: the process exits at end of turn
and any pending wakeup dies with it. `ScheduleWakeup` returned success
anyway, so the agent truthfully told the user "loop armed, next tick in
~60s" while zero `agent_loops` rows, zero `agent_reminders` rows and zero
follow-up executions existed. No error anywhere — that was the bug.

Two layers, both needed:

* Mechanical. `_runtime_config.PLATFORM_DENIED_TOOLS` + the single builder
  `merged_disallowed_tools(guardrails)` union the operator's GUARD-003
  deny-list with the platform's, and both Claude spawns (chat + headless
  task) now build `--disallowedTools` through it. Denying the mechanism is
  what makes the wrong path fail VISIBLY rather than report success.

  Denied by tool NAME only. A `Skill(loop)` rule was considered and
  rejected: the Skill specifier grammar is not a contract we can pin while
  the base image tracks the latest CLI, and an unparseable rule risks the
  whole `--disallowedTools` argument on every turn — a fleet outage traded
  for a second layer over the mechanism the deny already removes. Verified
  in a live agent container that the CLI accepts the flag+value (an unknown
  option errors loudly with `error: unknown option`; this one behaves
  exactly like the no-flag baseline).

* Guidance. A `### Repeating Work and Deferred Ticks` section in
  PLATFORM_INSTRUCTIONS routes repetition to `run_agent_loop` /
  `set_reminder` and names the two harness affordances to avoid. Platform-
  injected, so every existing and future agent gets it with no workspace
  edit, and registered as an ALWAYS section: its load-bearing half is a
  negative rule about tools that are not ours, so unlike the three
  tool-usage sections it has no tool description to fall back to at the
  MINIMAL tier.

The mechanical half needs a base-image rebuild to reach existing agents;
the guidance half is live on the next backend deploy.

tests/unit/test_2454_loop_routing.py pins both. The spawn-site check
DISCOVERS every function passing `--disallowedTools` under the agent server
rather than naming today's two — a third spawn site is exactly how this
regresses — and one test derives the prompt's claims from the deny list, so
a tool denied without a word in the guidance (or vice versa) reds. All four
mutations were confirmed to fail the suite before being reverted.

Also fixes a latent trap this change tripped: test_1402_prompt_contract.py
sliced the operator section between two NAMED headings, so inserting an
unrelated section after it silently WIDENED the slice and made its
runtime-identity and MCP-name-free assertions police text they were never
written for. The end bound is now derived (the next `### ` heading).

Fixes #2454
@dolho

dolho commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Verified end-to-end on a real instance

Local Trinity (PostgreSQL, src/backend bind-mounted, agent weather-watch on a live Claude subscription, claude 2.1.220, model claude-sonnet-4-6). Both layers deployed: backend restarted with the new PLATFORM_INSTRUCTIONS, and the two-line spawn patch applied to the running agent container.

1. Reproduced the bug first, unpatched

"Run a loop that reports the current timestamp every 60 seconds, 3 times total, then stops. Set it up so it keeps ticking on its own."

TOOL_USE: ScheduleWakeup {"delaySeconds": 60, "prompt": "Continue the timestamp loop..."}
  RESULT: "Next wakeup scheduled for 04:21:00 (in 119s). Nothing more to do this turn —
           the harness re-invokes you when the wakeup fires..."
RESPONSE: "Loop is running. Iteration 1/3 just reported... Iteration 2/3 will fire in ~60 seconds"

schedule_executions for that agent afterwards: one row, the chat turn itself. No follow-up execution, no agent_loops row, no agent_reminders row. Exactly the reported false success.

(An earlier probe — "run a basic loop 3 times" with no time spacing — did not trip it; sonnet inlined three date calls. The failure needs the request to imply time-spaced repetition, which is why the issue's repro says "every N seconds". Worth knowing for anyone re-testing.)

2. Same prompt, patched

TOOLS: ScheduleWakeup present = False   (28 tools; it is gone from the advertised set)
TOOL_USE: ToolSearch {"query": "select:mcp__trinity__run_agent_loop,mcp__trinity__set_reminder"}
TOOL_USE: mcp__trinity__run_agent_loop {...}
  RESULT: {"success": true, "loop_id": "loop_PJuNQnOZpTMsb7HI", "status": "queued", "max_runs": 3}
RESPONSE: "...The loop runs server-side and doesn't depend on this session staying open."

The agent went straight to ToolSearch for the two tool names the guidance block spells out. Final DB state:

LOOP: (loop_PJuNQnOZpTMsb7HI, completed, 3, 3, stop_signal_matched)
EXECS: (loop,success,08:22:06) (loop,success,08:23:11) (loop,success,08:24:18)

3. Deferred tick routes to a reminder — and it fires

"Check back with me in about 5 minutes and tell me the time then. Do not just wait — set it up so it actually happens."

TOOL_USE: mcp__trinity__set_reminder {...}
  RESULT: {"success": true, "id": "rem_aa5beac9f7df4ffa9efee216f7bc2719"}
REMINDER: (rem_aa5b…, fired, 1, fire_at 08:28:27, fired_at 08:28:27)
EXEC:     (reminder, success, 08:28:27)

4. Explicitly demanding the harness skill

"Use the built-in /loop skill and ScheduleWakeup... I specifically want the Claude Code loop skill, not anything else."

"I can't do that honestly... ScheduleWakeup is not available in this runtime. The platform blocks it outright — it's not a permissions issue... If I called it, it would silently return success and then nothing would ever fire. Telling you "loop armed" after that would be a false claim. The /loop skill has the same problem — it relies on ScheduleWakeup under the hood."

The deny and the guidance reinforce each other: the model refuses a direct instruction and names the right primitive, rather than hitting an unexplained tool error.

5. Both spawn sites, not just chat

INFO agent_server.services.claude_code:       [Chat] Disallowed tools: ['ScheduleWakeup']
INFO agent_server.services.headless_executor: [Headless Task] Disallowed tools: ['ScheduleWakeup']

and a /task (headless) turn asked to list its own Schedule/Wakeup tools answered NONE, with ScheduleWakeup absent from its init tool list.

6. Guidance renders correctly per runtime

claude-code  section=YES  ScheduleWakeup=YES  /loop=YES  mcp__trinity__run_agent_loop=YES
codex        section=YES  ScheduleWakeup=YES  /loop=YES  mcp__trinity__run_agent_loop=NO  run_agent_loop=YES

Codex strips the Claude-only prefix and keeps the harness names, as _adapt_instructions_for_runtime intends.


The instance has been restored to dev (host file reverted, container files restored from backup, both restarted healthy) — the bind mount means running this locally is just checking out the branch, plus a base-image rebuild for the deny half.

…end conflict)

Both sides appended 2026-09-01 learnings entries; kept all three
(the two #2454 entries from this branch + the #2464 Tailwind entry
from dev via PR #2465).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QWiYNh8QMuaMxcrhh2SxF

@obasilakis obasilakis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validated against the Trinity methodology — no blockers, CI green across 17 checks, and the two-layer fix with a discovery-based regression suite (test_2454_loop_routing.py, 16 tests, discovery rather than enumeration) is solid. Holding off on approving for one factual error in text that ships to every agent.

The prompt says something that isn't true. The new section in platform_prompt_service.py states: "Both default to yourself; pass agent_name only to drive another agent you have permission for." That holds for run_agent_loop (routers/loops.py:134, get_authorized_agent, sibling-capable) but is false for set_reminderrouters/reminders.py:52's _self_gate 403s any sibling regardless of permission. It fails loudly rather than silently, so impact is low, but it's guidance telling agents to attempt something the platform refuses by design.

Release-note ordering. The same section asserts "The platform denies ScheduleWakeup outright, so a refusal there is this rule and not a permissions problem." The guidance ships on the next backend deploy; the deny is in-container and reaches agents only on the next base-image rebuild. In that window the prompt states a mechanical backstop that does not exist, and ScheduleWakeup still answers success. The PR body states the asymmetry — the prompt text itself doesn't hedge. Same class as ent#123 / #1704.

One behavioural note, not a defect: PLATFORM_DENIED_TOOLS is non-empty, so the if disallowed_tools: guard at claude_code.py:232 / headless_executor.py:741 is now always true, and --disallowedTools is passed on every Claude spawn where previously only guardrailed agents got it. Verified against claude 2.1.220 in a live container, so it works today — but the base image tracks the latest CLI, and a future flag rename would now break all turns rather than a subset. Widened blast radius, not a bug.

Verified independently: exactly two claude --print sites exist (claude_code.py:223, headless_executor.py:676), both routed through merged_disallowed_tools, and the AST discovery test plus its non-vacuity assertion covers a future third. The test_1402_prompt_contract.py change is in-scope repair of a trap this PR tripped (named end-heading → derived ^### bound), not scope creep. Security and packaging clean.

Happy to approve once the set_reminder sentence is corrected.

@vybe vybe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validated via /validate-pr: closing keyword ✅, deny-list + prompt route with test_2454 + prompt-contract coverage, architecture.md updated. learnings.md append conflict with #2465 resolved by keeping both sides; full suite re-ran green on the merged head. Note: docker/base-image change — needs a base-image rebuild to reach agents.

@vybe
vybe merged commit 7d57323 into dev Sep 1, 2026
26 checks passed
vybe pushed a commit that referenced this pull request Sep 2, 2026
…red on the fleet CLI (#2468) (#2476)

ScheduleWakeup (#2454) was one member of a family: in a one-shot
`claude --print` execution, any tool result asserting a later event — a
wakeup, cron fire, workflow notification, peer message, desktop
notification — is a false fact the model plans around and then reports
success for (#2467 is the measured incident shape).

Fixes 2+3 of the issue (fixes 1+5 shipped in #2458), redone from closed
reference PR #2472 with the audit re-measured on claude 2.1.235 — the
CLI the fleet image actually ships. The re-measurement changed one
membership (ListAgents added; #2472 left it unclassified) and rewrote
two rationales (SendMessage: 2.1.235 gained a working in-turn
subagent-continuation facet, so the deny now stands on the unaudited
cross-execution channel between concurrent turns sharing one container
HOME; Monitor: its own text routes single waits to Bash-until anyway).

- PLATFORM_DENIED_TOOLS 1 → 11, every entry with its deciding
  description fragment quoted verbatim beside it (2.1.235-verified)
- KEPT becomes data: PLATFORM_KEPT_TOOLS (19, reasons in prose) +
  AUDIT_CLI_VERSION; DENIED ∪ KEPT exactly covers the measured init
  list (test-pinned), and scripts/dev/audit_headless_tools.py is the
  one-command re-audit for the next CLI bump (a deny of an unknown
  name is a silent no-op, and no PR workflow builds the base image)
- Always-tier prompt section "Nothing survives the end of your turn":
  counterweight for Bash's own un-deniable "You will be notified when
  it completes"; truth-scoped (subagents are waited; resumable
  surfaces get later turns) and names the working wait idiom, since
  Bash blocks long sleeps and routes polling to the denied Monitor
- Family paragraph phrased as capability fact, true on both sides of
  the image cut (the prompt reaches the fleet before the image does)
- 34 tests incl. exact-cover snapshot, reverse vocabulary guard, CORE
  never-denied; 4 mutations confirmed red; live-verified end to end
  (probe CLEAN: 19 built-ins offered; AC-2 absence semantics; the
  E2E incident shape now REFUSED by the model, clean turn_integrity)

Agent-side deny needs base-image rebuild + fleet cold recreate; the
prompt half reaches the fleet on backend deploy alone.

Fixes #2468

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants