fix(agent): rewrite deprecated VSCode tool names at Copilot deploy time (#2465) - #2500
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the Copilot (github_agent) agent deployment path so that apm install --target copilot rewrites deprecated VS Code Copilot built-in tool names (e.g., askQuestions) to their namespaced equivalents (e.g., vscode/askQuestions) when writing the deployed .github/agents/*.agent.md copy, preventing IDE rename warnings without modifying package sources.
Changes:
- Added a
GITHUB_AGENT_TOOL_RENAMESmapping and a frontmatter rewrite helper to rename deprecatedtools:entries duringgithub_agentdeployment. - Introduced a dedicated
_copy_github_agent()copy path and wired it into both the modern target-driven integration path and the deprecated legacy entry point. - Added unit + integration regression tests for the rename behavior, and updated docs to reflect the non-verbatim Copilot deploy transform.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/apm_cli/integration/agent_integrator.py |
Implements deploy-time YAML frontmatter tool rename transform and routes Copilot deployment through it. |
tests/unit/integration/test_agent_integrator.py |
Adds coverage ensuring deprecated tool names are rewritten (and non-matching cases remain unchanged). |
docs/src/content/docs/producer/author-primitives/instructions-and-agents.md |
Documents Copilot deploy-time tool renames and recommends authoring with namespaced tool identifiers. |
APM Review Panel:
|
| Persona | B | R | N | Takeaway |
|---|---|---|---|---|
| Python Architect | 0 | 1 | 2 | Well-scoped, idiomatic addition that follows existing AgentIntegrator patterns; no architectural faults. |
| CLI Logging Expert | 0 | 1 | 1 | Silent rename is defensible but a verbose-only diagnostic would align with how every other agent transform communicates mutations. |
| DevX UX Expert | 0 | 1 | 2 | Silent rewrite is the right default but a single info line per affected file would help users understand why deployed output differs from source. |
| Supply Chain Security Expert | 0 | 0 | 0 | No supply-chain security concerns: hardcoded network-free string mapping with safe YAML parsing and symlink guards. |
| OSS Growth Hacker | 0 | 0 | 2 | Clean DX friction removal; docs update solid but CHANGELOG entry and release narrative could mine portability-by-manifest story harder. |
| Doc Writer | 0 | 2 | 2 | Documentation changes are accurate and well-placed; table cell is verbose, pitfall omits old names that would help authors recognise the issue. |
| Test Coverage Expert | 0 | 0 | 1 | All behavioral changes have targeted regression-trap tests; one minor edge-case branch (non-string tool entries) lacks a dedicated test. |
B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.
Top 4 follow-ups
- [CLI Logging Expert + DevX UX Expert] Add verbose-level diagnostic when tool names are rewritten -- Two independent panelists converged: every other agent transform emits feedback on mutation. Diagnostic parity prevents user confusion when deployed output differs from source.
- [Doc Writer] List old tool names alongside new names in the pitfall entry -- A reader debugging existing source with deprecated names cannot confirm which are rewritten without cross-referencing code. One sentence fix.
- [Python Architect] Refactor copy_agent to accept optional pre_transform to eliminate _copy_github_agent body duplication -- If copy_agent gains a security check later, _copy_github_agent silently misses it. Low urgency but architecturally correct.
- [OSS Growth Hacker] Ensure CHANGELOG entry frames this as user-facing benefit, not internal refactor -- Reinforces portability-by-manifest positioning in the release narrative.
Architecture
classDiagram
direction LR
class BaseIntegrator {
<<Abstract>>
+find_files_by_glob()
+check_collision()
+sync_remove_files()
}
class AgentIntegrator {
<<ConcreteIntegrator>>
+integrate_agents_for_target()
+integrate_package_agents()
+copy_agent(source, target) int
+_copy_github_agent(source, target) int
+_apply_github_agent_tool_renames(content) str
+_write_codex_agent(source, target)
+_warn_opencode_frontmatter()
+_validate_kiro_tools()
-_FRONTMATTER_RE Pattern
}
class GITHUB_AGENT_TOOL_RENAMES {
<<Constant>>
+dict 6 entries
}
class IntegrationResult {
<<ValueObject>>
+files_integrated int
+files_skipped int
+links_resolved int
}
BaseIntegrator <|-- AgentIntegrator
AgentIntegrator ..> IntegrationResult : returns
AgentIntegrator ..> GITHUB_AGENT_TOOL_RENAMES : reads
class AgentIntegrator:::touched
class GITHUB_AGENT_TOOL_RENAMES:::touched
classDef touched fill:#fff3b0,stroke:#d47600
flowchart TD
A["apm install / integrate_agents_for_target"] --> B{"mapping.format_id?"}
B -->|kiro_agent| C["_validate_kiro_tools"]
C --> D["copy_agent"]
B -->|codex_agent| E["_write_codex_agent MD-to-TOML transform"]
B -->|github_agent| F["_copy_github_agent"]
F --> G["_apply_github_agent_tool_renames YAML roundtrip on frontmatter tools"]
G --> H["resolve_links"]
H --> I["write_text_lf"]
B -->|opencode_agent| J["_warn_opencode_frontmatter"]
J --> D
B -->|else| D
D --> K["resolve_links"]
K --> L["write_text_lf"]
style F fill:#fff3b0,stroke:#d47600
style G fill:#fff3b0,stroke:#d47600
Recommendation
All panelists aligned, no blocking findings, no security concerns, full test coverage on behavioral changes, and CI green. Community contributor PR -- merge promptly and file follow-up issues for diagnostic parity (FU-1) and docs old-name listing (FU-2).
Full per-persona findings
Python Architect
- [recommended] _copy_github_agent duplicates copy_agent body instead of composing it. If copy_agent ever gains a security check (e.g. file-size limit), _copy_github_agent silently misses it.
Suggested: Refactor copy_agent to accept optional pre_transform: Callable[[str], str] | None = None parameter applied between read and resolve_links. - [nit] GITHUB_AGENT_TOOL_RENAMES should be immutable (MappingProxyType) -- KIRO_AGENT_ALLOWED_TOOLS is a frozenset; consistency suggests wrapping with MappingProxyType.
- [nit] Legacy integrate_package_agents path parity -- both modern and deprecated paths confirmed to call _copy_github_agent. Good.
CLI Logging Expert
- [recommended] Emit a verbose-level diagnostic when tool names are rewritten -- every other agent transform emits a diagnostic on mutation. Breaking the pattern.
Suggested: In _copy_github_agent, after rename, emit diagnostics.info naming file and count of renames (shown only in --verbose). - [nit] _copy_github_agent lacks diagnostics and package_name parameters -- unlike every other per-format helper.
DevX UX Expert
- [recommended] Silent tool rename leaves users confused when source != deployed -- no progress/info message emitted when deprecated names are rewritten. Every other install transform emits at least one info line.
Suggested: Emit a single _rich_info per file when content changed. - [nit] Docs table Transform cell is verbose (12 words where others are 1-4).
- [nit] The fetch -> web/fetch rename is an exact match on a common word; docs should note this is exact-match only.
Supply Chain Security Expert
No findings.
OSS Growth Hacker
- [nit] Docs pitfall entry buries user-facing benefit under implementation detail -- lead with 'APM handles this for you'.
- [nit] Deploy-target table update accurate but misses story beat -- shorten for scannability.
Auth Expert -- inactive
PR touches agent_integrator.py for deploy-time YAML tool-name rewriting; no auth surfaces changed.
Doc Writer
- [recommended] Table cell is verbose and hard to scan (12 words in a Transform column where all others are 1-4 words).
Suggested: Shorten to 'tool names rewritten; otherwise verbatim'. - [recommended] Pitfall entry lists new names but not old names -- a reader debugging existing source cannot confirm which old names are rewritten without cross-referencing code.
Suggested: Add parenthetical '(askQuestions, runInTerminal, getTerminalOutput, createFile, fetch, listDirectory)' after 'the six known old names'. - [nit] Pitfall bold label is slightly awkward ('Old unnamespaced VSCode built-in') -- shorten to 'Deprecated VSCode built-in tool names'.
- [nit] Source attribution could add constant name for discoverability (GITHUB_AGENT_TOOL_RENAMES).
Test Coverage Expert
- [nit] Non-string tools list entries branch untested -- _apply_github_agent_tool_renames has isinstance(t, str) guard passing non-string entries through unchanged; no dedicated test for mapping or sequence entries.
Suggested: Add one parametrized test case with a mapping entry in the tools list.
Performance Expert -- inactive
O(n) tools YAML parse per agent file at deploy time with O(1) rename map lookup and early-return guard for zero-match case; not a hot-path change.
This panel is advisory. It does not block merge. Re-apply the
panel-review label after addressing feedback to re-run.
VSCode Copilot namespaced its built-in tool identifiers (e.g. 'askQuestions' -> 'vscode/askQuestions'). Any package that ships an agent with old unnamespaced tool names in its 'tools:' frontmatter triggers 'Tool or toolset has been renamed' warnings in the IDE after 'apm install --target copilot'. Fix: add GITHUB_AGENT_TOOL_RENAMES mapping and apply it at deploy time in _copy_github_agent(). The transform is wired into both the target-driven integrate_agents_for_target (github_agent format_id) and the legacy integrate_package_agents path. Source package files are never modified. Six renames applied automatically: askQuestions -> vscode/askQuestions runInTerminal -> execute/runInTerminal getTerminalOutput -> execute/getTerminalOutput createFile -> edit/createFile fetch -> web/fetch listDirectory -> search/listDirectory Added 12 regression tests covering the static helper, all six renames, edge cases (no frontmatter, null/non-list tools, unknown names, already-namespaced names), and both integration paths. Updated instructions-and-agents.md: copilot row in the transform table and a new 'Common pitfalls' entry for old tool names. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…_tool_renames - Preserve non-string YAML entries (mappings, sequences) as-is instead of coercing everything with str() when a rename occurs - Clarify docstring: 'semantically preserved' vs 'completely unchanged' to reflect that YAML roundtrip may alter formatting of unrelated keys Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
… built-in tool identifiers to current namespaced form; no new normative APM behaviour, no spec extension Co-authored-by: sergio-sisternes-epam <sergio-sisternes-epam@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Addresses apm-review-panel CEO follow-ups FU-1, FU-2, FU-4 and
Copilot test-coverage finding TC-1:
FU-1 (CL-1/DX-1): _copy_github_agent now accepts diagnostics and
package_name params matching the signature of every other per-format
helper. When tool names are rewritten and diagnostics is supplied, an
info-level message names the file and count of rewrites -- preserving
diagnostic parity with _write_codex_agent and _warn_opencode_frontmatter.
Both call sites in integrate_agents_for_target and
integrate_package_agents are updated to pass the collector through.
FU-2 (DW-1/DW-2/DW-3): instructions-and-agents.md pitfall entry now
lists both the old names (askQuestions, runInTerminal, getTerminalOutput,
createFile, fetch, listDirectory) alongside the new namespaced forms so
authors can audit existing source files without reading the code. Bold
label shortened from 'Old unnamespaced VSCode built-in tool names' to
'Deprecated VSCode built-in tool names'. Table cell shortened from
twelve words to four ('tool names rewritten; otherwise verbatim').
Exact-match caveat added (e.g. fetchData is unaffected).
FU-4: CHANGELOG [Unreleased] Fixed entry added framing the change as a
user-facing IDE-warning fix, not an internal refactor.
TC-1: Added test_non_string_tool_entries_pass_through_unchanged to
TestGithubAgentToolRenames covering the isinstance(t, str) guard path
for mapping nodes in the tools list alongside renamed string entries.
Co-authored-by: sergio-sisternes-epam <sergio-sisternes-epam@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
GitHub merge-queue squash commits prepend '* ' to each cherry-picked commit message in the synthetic squash body. The Mode B detector's grep pattern '^apm-spec-waiver:' did not match the prefixed form '* apm-spec-waiver:', so the waiver set in commit 9e17395 was silently ignored during merge-queue CI runs, causing repeated Spec conformance failures even though the waiver was present. Fix: extend the git-log grep to '^(\* )?apm-spec-waiver:' and strip the optional '* ' prefix before extracting the rationale. The PR-body path (GH_PR_BODY env) is unaffected -- it only fires on plain PR triggers where the squash prefix cannot appear. Verified against the synthetic squash commit from the merge queue run gh-readonly-queue/main/pr-2500-3aa0365540e3d9ef4685740cea6a09094ff35377 (commit 0fc07a6): BASE_REF=FETCH_HEAD~1 mode_b_detector.sh exits 0 with the correct WAIVED message after this fix. Co-authored-by: sergio-sisternes-epam <sergio-sisternes-epam@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Thank you for the original work on this fix. To land it promptly we have opened a superseding PR (#2519) under microsoft/apm that preserves your authorship via commit trailers and resolves the follow-ups surfaced by the apm-review-panel pass. Closing this PR in favor of #2519. Your contribution is credited on every cherry-picked commit; the superseding PR's body links back here. Please do raise concerns on the superseding PR if the changes diverge from your intent -- we want your sign-off too. |
|
Closing in favor of superseding PR #2519 which includes the panel follow-ups and merge-queue compatibility fix. |
The _copy_github_agent method emits a diagnostics.info() call that is a new consumer of printable_ascii_text. Three fixes: 1. check_diagnostic_ascii_owner.py: add 'info' to the set of method names covered by _diagnostic_calls(), and add AgentIntegrator._copy_github_agent to AGENT_DIAGNOSTIC_FUNCTIONS (require_source=False, same as _warn_opencode_frontmatter) so the checker enforces the boundary on both package_name and source.name. 2. agent_integrator.py: wrap source.name with printable_ascii_text() in the info() message, matching the same pattern used by _warn_codex_unverified_scope and _warn_codex_tools_dropped. 3. test_check_diagnostic_ascii_owner.py: update test_opencode_wrapper_package_field_must_use_owner to target the second occurrence of the pattern (the first is now in _copy_github_agent), and add test_copy_github_agent_package_field_must_use_owner to cover the new consumer. Fixes Build & Test Shard 2 failure on supersede/pr-2500 CI. Co-authored-by: sergio-sisternes-epam <sergio-sisternes-epam@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…y duplication Addresses panel follow-up FU-3: _copy_github_agent was introduced with a duplicated read/symlink-guard/resolve/write body copied from copy_agent. This fold adds a backward-compatible pre_transform parameter to copy_agent and rewrites _copy_github_agent as a thin wrapper that passes a tracking closure as the hook. copy_agent(source, target, pre_transform=None) applies pre_transform to the raw file content between read and link resolution. All existing callers pass no pre_transform and are unaffected. _copy_github_agent now delegates the entire read/resolve/write pipeline through copy_agent, removing the duplicate symlink guard, read, and write_text_lf call sites. The tracking closure captures whether a rename occurred so that the info-level diagnostic (diagnostics.info) can still be emitted after copy_agent returns, preserving the user-visible output contract. Mutation-break evidence (3 gates, all confirmed fail-without/pass-after): 1. isinstance(t, str) guard in _apply_github_agent_tool_renames: removing it causes TypeError: unhashable type: 'dict' in test_non_string_tool_entries_pass_through_unchanged. 2. printable_ascii_text(package_name) in _copy_github_agent: removing it causes check_diagnostic_ascii_owner to emit 3 violations (AgentIntegrator._copy_github_agent must derive ...; must not render raw ...; must not pass ... through local normalization path). 3. if pre_transform is not None guard in copy_agent: removing it causes test_copy_agent_pre_transform_applied_before_link_ resolution to fail (calls == [] instead of ['MARKER content']). Two new tests added to TestAgentIntegrator: - test_copy_agent_pre_transform_applied_before_link_resolution - test_copy_agent_without_pre_transform_is_verbatim Co-authored-by: sergio-sisternes-epam <sergio-sisternes-epam@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
9e17395 to
8e169e5
Compare
fix(agent): rewrite deprecated VSCode tool names at Copilot deploy time (#2465)
TL;DR
apm install --target copilotnow silently rewrites the six built-in tool namesthat VSCode Copilot renamed to a namespaced format (e.g.
askQuestions→vscode/askQuestions) when deploying*.agent.mdfiles. Packages that still usethe old names stop producing "Tool or toolset has been renamed" IDE warnings
without any changes to their source files.
Note
Closes #2465. Source package files are never modified — the rewrite is applied
only to the deployed copy under
.github/agents/.Problem (WHY)
apm install --target copilotcallscopy_agent()for thegithub_agentformat, which is a verbatim copy with link resolution and no frontmatter
transform. Packages that ship
tools: [askQuestions, runInTerminal, fetch, ...]deploy those old names to
.github/agents/<name>.agent.mdunchanged.reads the
tools:frontmatter of every agent file in.github/agents/andflags any entry it no longer recognises with "Tool or toolset has been renamed",
breaking the perceived quality of generated files immediately after install.
use (terminal access, file creation, web fetch, directory listing). Any package
that restricts scope to those tools is visibly broken on first open.
Why these matter: APM's Portability by manifest principle requires that a
single
apm.ymlproduces correct output across targets. A deploy step that emitsstale identifiers violates the install-quality promise — the user sees warnings in
files APM just wrote.
Approach (WHAT)
GITHUB_AGENT_TOOL_RENAMESdict — six old → new pairs — as a module-level constant next toKIRO_AGENT_ALLOWED_TOOLS_apply_github_agent_tool_renames(content: str) -> strstatic method — parses frontmatter, renames matchingtools:list entries, returns content unchanged on any edge case (no frontmatter, unparseable YAML, no tools key, non-list value)_copy_github_agent(source, target)instance method — likecopy_agentbut calls the rename transform before link resolutionelif mapping.format_id == "github_agent"branch inintegrate_agents_for_targetto call_copy_github_agentintegrate_package_agentscopilot path to call_copy_github_agentfor consistencyImplementation (HOW)
src/apm_cli/integration/agent_integrator.py— adds the rename constant,transform static method, and
_copy_github_agentwrapper. Theintegrate_agents_for_targetdispatch block gains oneelifbefore theexisting
elsebranch; all other format IDs (kiro_agent,codex_agent,opencode_agent) are untouched. The deprecatedintegrate_package_agentscopilot write call is updated to
_copy_github_agentfor parity.tests/unit/integration/test_agent_integrator.py— newTestGithubAgentToolRenamesclass with 12 tests covering the static helper(all six renames, unknown names, edge cases) and both integration paths.
docs/src/content/docs/producer/author-primitives/instructions-and-agents.md—updates the copilot row in the transform table from "verbatim" to the actual
behaviour; adds a "Common pitfalls" entry pointing authors toward the new
namespaced names.
Diagrams
Legend: the new
_apply_github_agent_tool_renamesstep (dashed) is inserted betweenreading the source and resolving links; all other stages are unchanged.
flowchart LR subgraph Source[Package source] A["tools: [askQuestions, ...]"] end subgraph Deploy["Deploy -- github_agent"] B["_apply_github_agent_tool_renames()"]:::new C["resolve_links()"] D["write_text_lf()"] end subgraph Output[".github/agents/"] E["tools: [vscode/askQuestions, ...]"] end A --> B --> C --> D --> E classDef new stroke-dasharray: 5 5; class B new;Trade-offs
deployed copy only. Authors are not forced to update their package sources, and
the source file remains the single source of truth. The downside: the adoption
comparison (which compares source bytes against the deployed file) will never
find a match for files that needed renaming — each reinstall rewrites the file.
This is harmless because the file is in
managed_filesand the overwrite isidempotent.
load_yaml_str/yaml_to_strforthe frontmatter may change whitespace or key ordering from the original source.
An early-return guard (
if renamed == [str(t) for t in tools_raw]: return content)avoids any roundtrip when no names match, keeping zero-rename files byte-for-byte
identical to today.
tools:values are left unchanged. A string, dict, or null valuepasses through verbatim. This is consistent with how
kiro_agenthandlesunexpected shapes and avoids a lossy transformation in ambiguous cases.
Benefits
apm install --target copilot— no package source changes needed.vscode/askQuestions)are returned verbatim (the rename map only covers old → new, not new → new).
github_agentformat path — kiro, codex, opencode, claude, andcursor paths are untouched.
Validation
pytest tests/unit/integration/test_agent_integrator.py (81 passed)
Scenario Evidence
apm install --target copilotwith an agent that has old tool names — deployed file uses namespaced names, no IDE warningstests/unit/integration/test_agent_integrator.py::TestGithubAgentToolRenames::test_integrate_copilot_target_rewrites_deprecated_tool_names(regression-trap for #2465)tests/unit/integration/test_agent_integrator.py::TestGithubAgentToolRenames::test_all_six_renames_via_full_integration_pathaskQuestions,runInTerminal,getTerminalOutput,createFile,fetch,listDirectory) are renamedtests/unit/integration/test_agent_integrator.py::TestGithubAgentToolRenames::test_all_six_known_renames_are_appliedtools:key, null tools, or already-namespaced names are deployed byte-for-byte unchangedtest_no_tools_key_returns_content_unchangedtest_null_tools_returns_content_unchangedtest_already_namespaced_names_pass_through_unchangedintegrate_package_agentspath (legacy callers) also rewrites tool namestests/unit/integration/test_agent_integrator.py::TestGithubAgentToolRenames::test_integrate_package_agents_deprecated_path_rewrites_tool_namesHow to test
uv run --extra dev pytest tests/unit/integration/test_agent_integrator.py -v→ all 81 tests pass.tools: [askQuestions, runInTerminal, fetch]in a test package; runapm install --target copilot→ inspect.github/agents/<name>.agent.md— the deployed file should containvscode/askQuestions,execute/runInTerminal,web/fetch; the source file should be unchanged.apm install --target copilotagain (reinstall) → no collision error; the file is rewritten idempotently.Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com
apm-spec-waiver: deploy-time compat shim -- renames deprecated VSCode built-in tool identifiers to current namespaced form; no new normative APM behaviour, no spec extension