Keep the ids of instructions a wrapper toolset passes on from the toolsets it wraps

Merged
DO
10d ago
+282-46

Part of #8965. #10906 needs this.

A wrapper toolset that has an id and overrides get_instructions() to pass on what it wraps, for example return [*await super().get_instructions(ctx), own_part], used to claim every relayed part without an id of its own. Such parts came out keyed toolset:<wrapper id>. When there were several, they all shared that one key, and the key pointed at text the wrapper never wrote. Only relayed parts that already had a key below the wrapper were handled correctly.

CodeModeToolset is one such wrapper. #10906 gives it the id code_mode so that its catalog block can be addressed (toolset:code_mode:catalog, with on_change='append'). Without this fix, that change would also have given every anonymous toolset instruction under CodeMode the id toolset:code_mode.

[!WARNING]
Compatibility impact: this affects a wrapper toolset that overrides get_instructions() and builds its result from super().get_instructions(ctx). Two of its blocks are now addressed differently:

  • A block it passes through unchanged from a toolset without an id used to be reported as toolset:<wrapper id> when the wrapper has an id. It now has no id, the same as without the wrapper.
  • A block it rewrote, built new, or edited in place while keeping a wrapped toolset's id (toolset:<wrapped id>[:<name>]) used to stay under that key. It is now keyed by the wrapper (toolset:<wrapper id>[:<name>]).

Blocks passed through unchanged keep their own toolset's id, as before.

Why this can ship in a minor release: both old ids were bugs. The first named text the wrapper didn't write, and several passed-through blocks could share it. The second put the wrapper's own text under the key of a toolset that didn't write it, so an application addressing that toolset's block found the wrapper's text instead. No built-in wrapper is affected: WrapperToolset.id is None, and the durable-exec wrappers return their own text.

Migration: for a passed-through block, give it an id of its own on the toolset it comes from. Set that toolset's id, or return an InstructionPart with a name, which is qualified under the toolset's id. Then address it as toolset:<wrapped id> or toolset:<wrapped id>:<name>. For a block the wrapper rewrites, address it by the wrapper's key, or pass the wrapped block on unchanged and add the wrapper's text as a separate block.

Change

  • While an authoring container's get_instructions() runs, AbstractToolset._collect_instruction_contributions records, in a context variable, the contributions its children produce. They are keyed by part identity, and _collect_child_instruction_contributions fills them in. This is the path WrapperToolset.get_instructions() and CombinedToolset.get_instructions() take.
  • When the result is collected, a part keeps its author's attribution, with or without an id, only if the container returned the same object with the same value. That value is a snapshot taken when the child contributed it. Everything else is the container's own text and is keyed by its id: new parts, rewritten copies, and parts edited in place, even when they carry a wrapped toolset's id. Relays are recorded only for the author and the containers below it through _instruction_children. A toolset the author reaches another way, such as the toolset a DynamicToolset builds, isn't relaying for it, so what that toolset says is keyed by the author, as before. An unchanged copy (replace(part)) also counts as the author's own part. This replaces the old fallback that attributed any part carrying a key found below the container to that key's owner. The fallback let a wrapper write under a wrapped toolset's key, and it is removed along with _instruction_sources_by_key.
  • No public API changes. The docs for building a custom toolset now say how relayed parts are attributed.

A wrapper can't use this to claim another toolset's key. The registry only holds what its own children contributed while its get_instructions() ran. A sibling's parts are collected in other tasks, each with its own registry. test_a_wrapper_cannot_relay_a_key_for_a_toolset_it_does_not_wrap still holds.

Compatibility note (see the warning above): in one case an id changes. A wrapper toolset with an id that relays, via super().get_instructions(), a block from a wrapped toolset without an id used to report that block as toolset:<wrapper id>. It now reports no id, the same as without the wrapper. Blocks the wrapper writes or rewrites keep its id. No built-in wrapper is affected: WrapperToolset.id is None, and the durable-exec wrappers return their own text, not relayed parts.

Tests (tests/test_instruction_ids.py)

  • A named wrapper relays an unnamed toolset's blocks without an id.
  • A named wrapper that adds a block owns only that block. An anonymous relayed block stays without an id, and a named one keeps its own.
  • A block the wrapper rewrote is keyed by the wrapper.
  • A wrapper can't write under a key it wraps: a named wrapped part that was rewritten with its id kept, a new part built with a wrapped id, and a wrapped part edited in place all come out under toolset:wrapper:limits, and so does an unchanged copy.
  • DynamicToolset(id='runtime') building a CombinedToolset whose anonymous member has an on_change='append' block named policy: the block stays toolset:runtime:policy with append on every request, with no downgrade warning, and its change is appended once. Durable dynamic toolsets resolve these instructions in get_tools, outside any collection, and get them back from serialization as new objects, so they're keyed by the durable wrapper (id runtime) as before. Relay tracking never sees them.

Checklist

  • Any AI generated code has been reviewed line-by-line by the human PR author, who stands by it.
  • No breaking changes in accordance with the version policy.
  • Any permitted compatibility impact has a label, warning, migration, release note, and exact API-check waiver.
  • PR title is fit for the release changelog.

🤖 Generated with Claude Code