Skip to content

feat(goose): add experimental Goose (Block) harness#1833

Open
Dorn- wants to merge 5 commits into
microsoft:mainfrom
Dorn-:feature/goose-harness
Open

feat(goose): add experimental Goose (Block) harness#1833
Dorn- wants to merge 5 commits into
microsoft:mainfrom
Dorn-:feature/goose-harness

Conversation

@Dorn-

@Dorn- Dorn- commented Jun 18, 2026

Copy link
Copy Markdown

feat(goose): add experimental Goose (Block) harness

TL;DR

Adds Goose (by Block) as a new experimental APM target, gated behind the goose flag. APM agents compile to Goose recipes that are headless-runnable (goose run --recipe) and parameterizable ({{ }} variables), skills land in the cross-tool .agents/skills/ standard Goose reads natively, MCP servers write to Goose's YAML extensions: block in ~/.config/goose/config.yaml, and instructions reach Goose through a .goosehints stub that imports the generated AGENTS.md. Like the other frontier targets, goose is never auto-detected and is excluded from --target all.

Problem (WHY)

APM's promise is one manifest deployed across every agent harness, yet Goose — a widely-used on-machine agent — had no target. Goose also does not fit the existing target mould on several axes, so a naive copy of another adapter would have been wrong:

  • Goose has no project-level config file: MCP servers ("extensions") live only in a single home config ~/.config/goose/config.yaml, in YAML, under an extensions: key with a Goose-native per-server schema (type: stdio / cmd / args / envs / timeout) — not the JSON mcpServers schema every other adapter writes.
  • Goose's "packaged agent" unit is a recipe (goose run --recipe), which is the natural target for APM's agents primitive — no other harness maps agents this way.
  • Instructions are read from .goosehints, which supports an @./path import preprocessor, so the right move is a stub that imports AGENTS.md rather than a second on-disk copy — context pulled at load time, anchor: PROSE, "Context arrives just-in-time, not just-in-case.".
  • Skills already match Goose's native .agents/skills/ standard, so that surface is reused as-is.

Approach (WHAT)

Each APM primitive maps onto the native Goose surface it belongs to:

APM primitive Goose surface Location
agents Recipe (goose run --recipe) .goose/recipes/<name>.yaml (project scope)
skills Skills (SKILL.md) .agents/skills/<name>/ (project) · ~/.agents/skills/<name>/ (--global)
instructions .goosehints importing AGENTS.md .goosehints + AGENTS.md at project root
MCP servers YAML extensions: block ~/.config/goose/config.yaml (user scope)

Gating is experimental: enable with apm experimental enable goose. Until then the target is inert — hidden from detection, excluded from --target all, and an explicit --target goose exits with an enable hint.

Implementation (HOW)

File Change
adapters/client/_yaml_config.py (new) YamlMcpClientAdapter base — YAML round-trip, sibling-key preservation, atomic 0o600 write, malformed-file refusal, configure_mcp_server flow.
adapters/client/hermes.py Refactored onto the shared base (behaviour and _to_hermes_format surface unchanged).
adapters/client/goose.py (new) GooseClientAdapter~/.config/goose/config.yaml path (honours $XDG_CONFIG_HOME), extensions key, stdio/streamable_http per-server transform.
integration/agent_integrator.py _write_goose_recipe (agent .md → recipe .yaml: instructions + a prompt for headless, verbatim parameters: + preserved {{ }}, settings.goose_model) + shared _parse_agent_frontmatter factored out of the Codex writer.
core/target_detection.py detect_target echoes an explicit/config goose (added to TargetType) so apm compile -t goose routes the .goosehints stub instead of falling back to minimal; _validate_canonical_v2 (the MCP-install --target resolver) now accepts experimental + explicit-only targets, fixing a pre-existing crash where apm install --target <goose|antigravity|hermes> aborted whenever the package declared an mcp: dependency.
integration/mcp_integrator_install.py Goose joins the MCP runtime-discovery list (flag on + ~/.config/goose or goose on PATH), so apm install --target goose writes the package's mcp: deps to ~/.config/goose/config.yaml instead of silently skipping them.
utils/yaml_io.py yaml_to_str(multiline_block=True) renders instructions/prompt as readable literal blocks; default path (Hermes) unchanged.
compilation/{gemini_formatter,goose_formatter,agents_compiler}.py GeminiFormatter parameterised by stub attributes so GooseFormatter reuses it; _compile_gemini_md generalised into _compile_import_stub.
integration/targets.py, factory.py, core/experimental.py, core/target_detection.py, commands/install.py goose TargetProfile, adapter registration, experimental flag, detection/description wiring, help text.
docs/, CHANGELOG.md Goose integration page, experimental-flag reference, compile note, changelog entry.

Diagrams

Legend: how one apm install / apm compile -t goose fans each APM primitive out to its native Goose surface.

flowchart LR
  agents["APM agents"] --> recipes[".goose/recipes/*.yaml"]
  skills["APM skills"] --> agentsdir[".agents/skills/"]
  instr["APM instructions"] --> agentsmd["AGENTS.md"]
  agentsmd --> hints[".goosehints imports AGENTS.md"]
  mcp["MCP dependencies"] --> cfg["config.yaml extensions block"]
Loading

Legend: the YAML-backed adapters now share one base, so the new target adds no duplicated config-I/O code.

classDiagram
  CopilotClientAdapter <|-- YamlMcpClientAdapter
  YamlMcpClientAdapter <|-- HermesClientAdapter
  YamlMcpClientAdapter <|-- GooseClientAdapter
  class YamlMcpClientAdapter {
    update_config()
    get_current_config()
    configure_mcp_server()
    to_native_format()
  }
Loading

Trade-offs

  • MCP servers stay in config.yaml, not embedded in recipes (Option A). An APM agent declares no MCP servers (only model + a tool-name whitelist), so per-recipe extensions would have to source the package's resolved MCP deps and thread them through the agent integrator — a cross-phase change. Goose reads config.yaml globally at run time, so recipes still work. Embedding remains a clean follow-up.
  • Recipes are project-scope only. Goose loads recipes from the cwd or $GOOSE_RECIPE_PATH with no canonical user-scope home, so the agents primitive is excluded at --global (skills + MCP still deploy at user scope).
  • Reuse over copy. The YAML adapter base, _compile_import_stub, and _parse_agent_frontmatter were extracted rather than duplicated — both to satisfy the R0801 duplication gate and on principle, anchor: PROSE, "Favor small, chainable primitives over monolithic frameworks.".
  • Experimental, not GA. New harness ships flag-gated (matching hermes/openclaw) so it stays out of default flows until proven.

Benefits

  1. Goose users get APM's full surface: agents, skills, instructions, and MCP — from the same apm.yml.
  2. Zero net new duplication: the YAML-config and import-stub logic is now shared by Hermes and Goose (R0801 clean at min-similarity-lines=10).
  3. Credentials in config.yaml are written atomically with 0o600 and never interpolated into errors/logs.
  4. Fail-closed: a malformed existing config.yaml is refused, never clobbered.
  5. Additive and inert by default — no behaviour change for any existing target.

Validation

Test suite, lint, duplication gate
$ uv run pytest tests/unit/integration tests/unit/core tests/unit/compilation tests/unit/adapters -q
3929 passed   # touched areas; pre-existing env-only failures elsewhere unaffected

$ uv run ruff check src/ tests/      ->  All checks passed!
$ uv run ruff format --check src/ tests/  ->  already formatted
$ uv run python -m pylint --disable=all --enable=R0801 --min-similarity-lines=10 --fail-on=R0801 src/apm_cli/
  ->  rated 10.00/10

Also verified end-to-end against the real goose CLI: apm compile -t goose writes AGENTS.md + .goosehints, apm install --target goose writes .goose/recipes/<name>.yaml + .agents/skills/, and a recipe with no extensions: block inherits the MCP server from the global ~/.config/goose/config.yaml at run time (so MCP is not embedded per-recipe).

A supply-chain-security review pass (per the repo's supply-chain-security-expert persona) returned APPROVE with no required findings: config.yaml write is 0o600 + atomic, path containment via ensure_path_within runs before the recipe dispatch, symlink sources are refused, and no new install-time network/code-execution path is introduced.

Scenario Evidence

User-promise scenario Proof (test) Principle
MCP servers configured for Goose, credentials owner-only test_goose_update_config_writes_extensions_block_0600 least privilege
Native config never clobbered on malformed input test_goose_update_config_refuses_malformed_yaml fail closed
APM agent runs as a Goose recipe test_goose_agent_compiles_to_recipe_yaml one manifest, every harness
Recipe is headless-runnable (always has a prompt) test_goose_recipe_falls_back_to_default_prompt, test_goose_recipe_uses_authored_prompt_verbatim usable in automation
Recipe parameters + {{ }} templating pass through test_goose_recipe_passes_parameters_and_preserves_templating parameterized reuse
apm compile -t goose actually emits the .goosehints stub test_goose_detect_target_echoes_explicit_and_config one manifest, every harness
apm install --target goose with an mcp: dep configures Goose (no crash) test_goose_accepted_by_resolve_targets, test_goose_runtime_discovered_when_opted_in MCP as a primitive
Skills land where Goose reads them test_goose_skills_deploy_to_agents_skills cross-tool standard
Instructions reach Goose via .goosehints test_goose_compile_writes_agents_md_and_goosehints one manifest, every harness
Inert until enabled; excluded from --target all test_goose_excluded_from_target_all, test_goose_resolves_only_when_named_with_flag_enabled safe defaults

How to test

  • apm experimental enable goose
  • In a repo with .apm/agents/<name>.md, run apm compile -t goose → confirm AGENTS.md, .goosehints (contains @./AGENTS.md), and .goose/recipes/<name>.yaml (valid recipe).
  • apm install --target goose → confirm skills under .agents/skills/.
  • apm install --target goose --global with an MCP dep → confirm an extensions: entry in ~/.config/goose/config.yaml with mode 0o600.
  • apm compile --all → confirm .goosehints is not generated (goose is excluded from all).

Spec conformance note

apm-spec-waiver: This PR introduces Goose-target critical-path behavior before spec text lands; we will follow with a dedicated spec citation/update PR to formally map these semantics.

Copilot AI review requested due to automatic review settings June 18, 2026 12:40
@Dorn- Dorn- force-pushed the feature/goose-harness branch from b03dfb7 to e92b6da Compare June 18, 2026 12:41

Copilot AI 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.

Pull request overview

Adds a new experimental goose target (Block Goose) to APM, including Goose-specific compilation artifacts (.goosehints, recipes) and a YAML-backed MCP config adapter shared with Hermes.

Changes:

  • Introduces GooseClientAdapter and a shared YamlMcpClientAdapter base to manage YAML MCP config documents (Hermes + Goose).
  • Adds Goose agent recipe generation (.goose/recipes/*.yaml) and .goosehints stub compilation that imports AGENTS.md.
  • Wires the new target through detection, install help text, docs, and tests.

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
tests/unit/integration/test_targets_registry_completeness.py Registers Goose adapter + adds extensions to known MCP keys.
tests/unit/integration/test_goose_target.py End-to-end integration tests for Goose target surfaces and gating.
tests/unit/integration/test_data_driven_dispatch.py Adds agents_goose bucket to dispatch parity test.
tests/unit/core/test_target_detection.py Updates experimental targets membership to include goose.
tests/unit/core/test_scope.py Adds goose to the known targets set assertion.
src/apm_cli/utils/yaml_io.py Adds optional multiline block-scalar YAML rendering for readability.
src/apm_cli/integration/targets.py Defines the goose TargetProfile and primitive mappings.
src/apm_cli/integration/agent_integrator.py Adds Goose recipe writer and factors shared agent-frontmatter parsing.
src/apm_cli/factory.py Registers goose adapter in the client factory.
src/apm_cli/core/target_detection.py Routes explicit/config goose, adds .goosehints compile predicate, and updates descriptions/experimental set.
src/apm_cli/core/experimental.py Adds goose experimental flag metadata and hint text.
src/apm_cli/compilation/goose_formatter.py New formatter subclass for .goosehints stub generation.
src/apm_cli/compilation/gemini_formatter.py Generalizes stub behavior via overridable class attributes.
src/apm_cli/compilation/agents_compiler.py Factors a shared import-stub compiler and adds Goose stub compilation.
src/apm_cli/commands/install.py Extends --target help to describe experimental Goose behavior.
src/apm_cli/adapters/client/hermes.py Refactors Hermes adapter to use the shared YAML base class.
src/apm_cli/adapters/client/goose.py New Goose YAML extensions: MCP adapter and transform logic.
src/apm_cli/adapters/client/_yaml_config.py New shared YAML round-trip + atomic write base for YAML MCP adapters.
docs/src/content/docs/reference/experimental.md Documents the goose experimental flag and its surfaces.
docs/src/content/docs/producer/compile.md Documents Goose compile behavior (.goosehints + AGENTS.md).
docs/src/content/docs/integrations/goose.md New Goose integration page.
CHANGELOG.md Adds an entry describing the Goose experimental target.

Comment thread src/apm_cli/utils/yaml_io.py Outdated
Comment thread src/apm_cli/adapters/client/_yaml_config.py
Comment thread src/apm_cli/adapters/client/goose.py
Comment thread src/apm_cli/adapters/client/goose.py Outdated
Comment thread src/apm_cli/integration/agent_integrator.py
Comment thread src/apm_cli/integration/agent_integrator.py Outdated
@Dorn-

Dorn- commented Jun 18, 2026

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

@Dorn- Dorn- force-pushed the feature/goose-harness branch 3 times, most recently from c7e7b53 to 54c858c Compare June 18, 2026 12:52
@sergio-sisternes-epam sergio-sisternes-epam added the panel-review Trigger the apm-review-panel gh-aw workflow label Jun 18, 2026
@github-actions github-actions Bot mentioned this pull request Jun 18, 2026
@Dorn- Dorn- force-pushed the feature/goose-harness branch from 54c858c to 546f52a Compare June 19, 2026 12:56
Dorn- and others added 4 commits June 26, 2026 17:20
Hermes was the only MCP client adapter writing a YAML config document
(a single top-level servers mapping) rather than the JSON mcpServers
schema. Lift its YAML round-trip / sibling preservation / atomic 0o600
write / malformed-file refusal / configure_mcp_server flow into a new
YamlMcpClientAdapter base so a second YAML-backed adapter can reuse it
without duplicating those blocks (the R0801 duplication guard would
otherwise reject the copy).

HermesClientAdapter now only declares its config path, mcp_servers_key,
display name, and the per-server schema transform; behaviour and the
_to_hermes_format static surface are unchanged.
Adds Goose (https://goose-docs.ai) as a first-class, experimental APM
target gated behind the `goose` flag (`apm experimental enable goose`).
Like the other frontier targets it is hidden from auto-detection and
excluded from `--target all`.

Goose differs structurally from every existing target, so the mapping
follows its native surfaces:

- MCP servers: GooseClientAdapter writes the YAML `extensions:` block of
  ~/.config/goose/config.yaml (honouring $XDG_CONFIG_HOME) in Goose's
  own per-server schema (type: stdio / cmd / args / envs / enabled /
  timeout; remotes -> streamable_http / uri). Reuses the shared
  YamlMcpClientAdapter base. User-scope only (Goose has no project
  config), with the new `extensions` key wired into the conflict
  detector's generic path.
- agents -> recipes: each .apm/agents/<name>.md compiles to
  .goose/recipes/<name>.yaml, the native packaged-agent unit Goose runs
  via `goose run --recipe` (title/description/instructions, plus
  settings.goose_model when the agent pins a model). A shared
  _parse_agent_frontmatter helper is factored out of the Codex writer to
  avoid duplicating the frontmatter preamble. MCP extensions are not
  embedded per recipe -- an APM agent declares no MCP servers; those stay
  in config.yaml, which Goose reads globally at run time.
- skills -> the cross-tool .agents/skills/ standard Goose reads natively
  (.agents/skills/ project, ~/.agents/skills/ with --global).
- instructions: compile_family="agents" emits AGENTS.md and a thin
  .goosehints stub that imports it via Goose's `@./AGENTS.md`
  preprocessor (GeminiFormatter is parameterised by overridable stub
  attributes so GooseFormatter reuses it; _compile_gemini_md is
  generalised into a shared _compile_import_stub).

Includes path-fidelity acceptance tests, registry/dispatch invariant
updates, the integration docs page, and the experimental-flag reference.
…mat (closes Mode B gate for microsoft#1833)

Add two normative requirements to spec section 8.5 (Deploy directory
contract):

- req-tg-005: A consumer writing MCP config for a YAML-based target
  MUST use the target's registered config key and per-server schema,
  preserving sibling keys not managed by APM; JSON-format mcpServers
  MUST NOT be written to YAML-config target files.
- req-tg-006: A consumer supporting the agents primitive for a target
  MUST compile each agent to the target's registered native format and
  deploy under the target's registered root; a different target's
  format MUST NOT be emitted.

Both requirements generalize the Goose and Hermes YAML-config pattern
and the recipe/agent compilation surface. Spec count: 95 -> 97
(92 MUST, 5 SHOULD). Manifest + Appendix C + conformance tests updated.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rements)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@sergio-sisternes-epam sergio-sisternes-epam added panel-review Trigger the apm-review-panel gh-aw workflow and removed panel-review Trigger the apm-review-panel gh-aw workflow labels Jun 26, 2026
@github-actions github-actions Bot mentioned this pull request Jun 26, 2026
@sergio-sisternes-epam sergio-sisternes-epam added panel-review Trigger the apm-review-panel gh-aw workflow and removed panel-review Trigger the apm-review-panel gh-aw workflow labels Jun 26, 2026
@sergio-sisternes-epam sergio-sisternes-epam added panel-review Trigger the apm-review-panel gh-aw workflow and removed panel-review Trigger the apm-review-panel gh-aw workflow labels Jun 27, 2026
@github-actions

Copy link
Copy Markdown

APM Review Panel: ship_with_followups

PR #1833 adds Goose (Block) as APM's third harness target, compiling apm.yml agents to native Goose recipes -- the strongest multi-harness proof point the project has shipped.

cc @sergio-sisternes-epam -- a fresh advisory pass is ready for your review.

Nine panelists reviewed this PR and produced zero findings above the recommended tier. The core implementation is solid across every dimension: atomic 0o600 config writes, malformed-YAML refusal, fail-closed env-value transforms, a clean YamlMcpClientAdapter DRY extraction that will make the fourth and fifth harness adapters faster to add, and a 633-line test suite that meets the integration-with-fixtures floor on every new surface. The GooseFormatter/GeminiFormatter inheritance axis is a future maintainability concern worth tracking but does not affect correctness today.

The highest-signal panel convergence is on a narrow but concrete problem: the ExperimentalFlag description and post-enable hint both describe this feature as MCP-server-only. Two panelists (devx-ux, cli-logging) arrived at the same user-visible regression from independent inspection angles -- a user who runs apm experimental enable goose reads "Configure MCP servers for the Goose agent (Block)." and follows a hint pointing exclusively to --global, which skips recipe compilation entirely. The headline differentiator -- native Goose recipe deployment via apm install --target goose -- is completely invisible in the discovery surface. This is a two-line in-PR fix in src/apm_cli/core/experimental.py lines 144-147 that prevents the first cohort of Goose adopters from missing the primary feature.

The two test-coverage gaps (no CliRunner integration test, binary-only opt-in branch untested) are straightforward post-merge additions with clear Hermes parity targets already named. The XDG_CONFIG_HOME bounds check is a recommended security hardening on a surface that mirrors the pre-existing $HERMES_HOME pattern; the risk exposure is unchanged from what shipped with Hermes, making this a tracked hardening rather than a new vulnerability.

Dissent. No meaningful dissent between panelists. The security expert's XDG_CONFIG_HOME finding carries the same pre-existing risk profile as the Hermes $HERMES_HOME surface; no panelist argued for a higher severity.

Aligned with: Multi-harness support (adds Goose as a first-class experimental target; the .agents/skills/ cross-tool standard deployed here is the first artifact shared across Copilot, Claude, and Goose from a single apm.yml). Portable by manifest (Goose recipe compilation from agent metadata without manual recipe authoring). Secure by default (atomic 0o600 writes, malformed-YAML refusal, fail-closed env transforms). Pragmatic as npm (flag gate with progressive disclosure; flag-off path emits a clear enable hint before any I/O). OSS community-driven (Block/Goose integration opens a cross-community referral loop).

Growth signal. The Block/Goose integration unlocks the "write once in APM, run in Copilot, Claude, and Goose" story -- a positioning claim no incumbent package manager can match. The .agents/skills/ cross-tool standard is a standout narrative hook for cross-posting to the Goose community Discord and Block developer blog. To maximize release-note pickup, the CHANGELOG entry should lead with "APM agents now run natively in Goose via goose run --recipe" rather than the current MCP-config-first framing.

Panel summary

Persona B R N Takeaway
Python Architect 0 1 2 Clean DRY extraction; GooseFormatter/GeminiFormatter coupling is a naming smell worth tracking; abstract methods should use @abstractmethod.
CLI Logging Expert 0 1 2 CLI output is ASCII-clean and uses correct helpers; bare 'Error configuring MCP server' catch-all needs a follow-up line; flag description scope mismatch affects apm experimental list.
DevX UX Expert 0 2 3 Two recommended fixes: flag description and post-enable hint misrepresent feature scope, steering users away from recipes (the primary capability).
Supply Chain Security Expert 0 1 2 Security posture is solid: 0o600 atomic writes, malformed-YAML refusal, fail-closed transforms; XDG_CONFIG_HOME path lacks bounds check (pre-existing Hermes pattern).
OSS Growth Hacker 0 1 1 Adding Block/Goose strengthens the multi-harness story; cross-tool .agents/skills/ standard is a standout narrative hook; CHANGELOG buries the headline recipe feature.
Auth Expert 0 1 1 APM auth tokens untouched; literal-env-embedding choice is documented; no user-facing warning for ${VAR} passthrough.
Doc Writer 0 2 1 goose.md is comprehensive and well-structured; two gaps: goose omitted from compile.md recommended-targets note, GOOSE_RECIPE_PATH example uses a relative path.
Test Coverage Expert 0 2 1 Two recommended gaps: no CliRunner integration test (Hermes has one), _goose_runtime_opted_in binary branch untested. Unit/fixture coverage is otherwise thorough.
Performance Expert 0 0 1 No hot-path regressions; read-all/write-all config pattern is acceptable for small YAML files; minor: _goose_runtime_opted_in instantiates GooseClientAdapter on every call.

B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.

Top 5 follow-ups

  1. [DevX UX Expert + CLI Logging Expert] Fix ExperimentalFlag description and post-enable hint to surface the recipe feature, not just MCP servers -- Two panelists independently flagged this from different angles. The current description ("Configure MCP servers for the Goose agent (Block).") and hint (pointing exclusively to --global) steer every user who enables the goose flag away from the headline capability. Recipe compilation is only reachable via project-scope apm install --target goose, which the hint never mentions. Suggested fix for src/apm_cli/core/experimental.py line 144: description="Deploy APM agents as Goose recipes (.goose/recipes/), skills, MCP extensions, and a .goosehints stub." and line 147 hint: "Run 'apm install --target goose' to install package agents as Goose recipes (.goose/recipes/) and skills (.agents/skills/). Add '--global' to also write MCP servers to ~/.config/goose/config.yaml. Run 'apm compile -t goose' to emit AGENTS.md + .goosehints at the project root."

  2. [Test Coverage Expert] Add CliRunner integration test suite for apm install --target goose (Hermes parity gap) -- The CLI parser contract -- TargetParamType accepting goose, flag-off hint emission, flag-on recipe deployment -- is unverified at the command surface. tests/integration/test_hermes_target.py has TestHermesParserE2E and TestHermesDeployE2E; Goose has no equivalent. A tests/integration/test_goose_target.py should cover: (1) flag-off parser accepts goose and emits enable hint, (2) flag-on + real bundle deploys .goose/recipes/ and .goosehints. Principle: devx, multi-harness-support.

  3. [Test Coverage Expert] Add unit test for _goose_runtime_opted_in binary-on-PATH branch (Hermes parity gap) -- The config_dir branch is tested; the "binary on PATH, config dir absent" branch is not. tests/integration/test_hermes_target.py has test_flag_on_with_binary_only_opts_in for this exact case. Without this test, the opt-in path silently regresses if the binary-detection branch is ever refactored. One-liner fix: monkeypatch XDG_CONFIG_HOME to an absent dir and find_runtime_binary to return a path.

  4. [Supply Chain Security Expert] Add bounds check on XDG_CONFIG_HOME before using it as a config write target -- GooseClientAdapter._config_path() uses XDG_CONFIG_HOME without validating it resolves inside an expected directory. A malicious dependency's post-install hook could set it to /etc or an attacker-controlled path. The same gap exists in the pre-existing $HERMES_HOME surface; worth closing both adapters together with a simple absolute-path + home-dir anchor check.

  5. [OSS Growth Hacker] Reorder CHANGELOG entry to lead with the Goose recipe feature, not MCP config details -- The current entry front-loads subordinate MCP config details and buries the headline in clause three. Release-note aggregators stop at the first clause. Suggested lead: "APM agents compile to Goose recipes at .goose/recipes/<name>.yaml and run natively via goose run --recipe <name>."

Architecture

classDiagram
    class CopilotClientAdapter {
        +configure_mcp_server()
        +_fetch_server_info()
        +_format_server_config()
    }
    class YamlMcpClientAdapter {
        +mcp_servers_key: str
        +_display_name: str
        +_supports_runtime_env_substitution: bool
        +_config_path() Path
        +_to_native_format() dict
        +_load_document() dict
        +update_config() bool
    }
    class GooseClientAdapter {
        +target_name = goose
        +mcp_servers_key = extensions
        +_config_path() Path
        +_to_native_format() dict
    }
    class HermesClientAdapter {
        +target_name = hermes
        +mcp_servers_key = mcp_servers
        +_config_path() Path
        +_to_native_format() dict
    }
    class GeminiFormatter {
        +_stub_filename: str
        +_stub_title: str
        +_generate_stub() str
    }
    class GooseFormatter {
        +_stub_filename = .goosehints
        +_stub_title = Goose hints
    }
    CopilotClientAdapter <|-- YamlMcpClientAdapter : extends
    YamlMcpClientAdapter <|-- GooseClientAdapter : extends
    YamlMcpClientAdapter <|-- HermesClientAdapter : extends
    GeminiFormatter <|-- GooseFormatter : extends
Loading
flowchart TD
    CMD["apm install --target goose"] --> EXP["_goose_runtime_opted_in()"]
    EXP -->|"flag off"| HINT["Enable hint to user"]
    EXP -->|"flag on + runtime present"| INST["Install pipeline"]
    INST --> AGI["AgentIntegrator._write_goose_recipe()"]
    INST --> SKI["SkillIntegrator SKILL.md"]
    INST --> MCI["GooseClientAdapter.update_config()"]
    AGI --> RECIPE[".goose/recipes/name.yaml"]
    SKI --> SKILL[".agents/skills/name/SKILL.md"]
    MCI --> CONFIG["~/.config/goose/config.yaml (0o600)"]
    CMD2["apm compile -t goose"] --> COMP["AgentsCompiler"]
    COMP --> AMD["AGENTS.md"]
    COMP --> GH[".goosehints (@./AGENTS.md)"]
Loading
sequenceDiagram
    participant U as User
    participant CLI as CLI
    participant EXP as ExperimentalGate
    participant AGI as AgentIntegrator
    participant GCA as GooseClientAdapter
    U->>CLI: apm install --target goose
    CLI->>EXP: _goose_runtime_opted_in()
    EXP-->>CLI: True (flag on + runtime present)
    CLI->>AGI: integrate_agents_for_target(goose)
    AGI->>AGI: _write_goose_recipe(agent.md to .yaml)
    AGI-->>CLI: IntegrationResult
    CLI->>GCA: update_config(mcp_servers)
    GCA->>GCA: _load_document() then atomic_write 0o600
    GCA-->>CLI: True
    CLI-->>U: success output
Loading

Recommendation

The implementation is ready to ship: zero findings above the recommended tier across nine panelists, strong security posture, and a test suite that covers new surfaces at fixture and unit depth. The single highest-priority in-PR action is the ExperimentalFlag description and hint text fix in src/apm_cli/core/experimental.py lines 144-147 -- a two-line change that prevents the initial Goose adopter cohort from missing recipe compilation entirely; if it lands before merge, the PR ships cleanly. If it slips, track it as the first post-merge patch alongside the CHANGELOG reorder. The Hermes-parity CliRunner test suite (tests/integration/test_goose_target.py) is the most important post-merge addition and should target the next patch cycle.


Full per-persona findings

Python Architect

  • [recommended] GooseFormatter subclasses GeminiFormatter -- wrong inheritance axis; prefer shared ImportStubFormatter base at src/apm_cli/compilation/goose_formatter.py
    GooseFormatter inherits from GeminiFormatter solely to reuse two class attribute overrides. If GeminiFormatter gains Gemini-specific behavior, GooseFormatter silently inherits it. The correct shape is a shared ImportStubFormatter base that both extend.
    Suggested: Extract class ImportStubFormatter with _stub_filename, _stub_title, _generate_stub(); let both GeminiFormatter and GooseFormatter subclass it.
  • [nit] _config_path and _to_native_format on YamlMcpClientAdapter should use @abstractmethod at src/apm_cli/adapters/client/_yaml_config.py:64
    Without @abstractmethod, the base can be instantiated directly and missing-override errors surface at call time rather than construction.
  • [nit] _write_goose_recipe hardcodes version: 1.0.0 -- extract as a module-level constant at src/apm_cli/integration/agent_integrator.py:404

CLI Logging Expert

  • [recommended] Bare _rich_error('Error configuring MCP server') catch-all gives users no next step at src/apm_cli/adapters/client/_yaml_config.py:181
    The suppression is correct (avoids credential leakage) but the message is not actionable. A follow-up _rich_info('Check registry connectivity and server name, then retry.') costs nothing.
  • [nit] Experimental flag description "Configure MCP servers for the Goose agent (Block)." understates feature scope in apm experimental list at src/apm_cli/core/experimental.py:144
  • [nit] Goose target description in RUNTIME_DESCRIPTIONS omits recipes and next-step hint at src/apm_cli/core/target_detection.py

DevX UX Expert

  • [recommended] Flag description in registry says only "Configure MCP servers" -- hides recipes, skills, and instructions at src/apm_cli/core/experimental.py:144
    Suggested: description="Deploy APM agents as Goose recipes (.goose/recipes/), skills, MCP extensions, and a .goosehints stub.",
  • [recommended] Post-enable hint steers users exclusively to --global, causing them to miss recipes (unsupported at user scope) at src/apm_cli/core/experimental.py:147
    Running apm install --target goose --global skips recipes entirely (unsupported_user_primitives=("agents",)). The hint never mentions project-scope install.
  • [nit] --global help text lists global-capable MCP runtimes but omits Goose at src/apm_cli/commands/install.py:964
  • [nit] GOOSE_RECIPE_PATH doc example uses a relative path that silently fails outside project root at docs/src/content/docs/integrations/goose.md
    Suggested: export GOOSE_RECIPE_PATH=$(pwd)/.goose/recipes
  • [nit] Stale "no runtimes support user-scope MCP" warning omits Goose at src/apm_cli/integration/mcp_integrator_install.py

Supply Chain Security Expert

  • [recommended] XDG_CONFIG_HOME path is used without bounds check at src/apm_cli/adapters/client/goose.py:55
    Path(os.environ.get('XDG_CONFIG_HOME')) / 'goose' / 'config.yaml' with no validation; same pre-existing pattern as Hermes $HERMES_HOME. In a CI environment where env vars may be attacker-influenced, this is a write-anywhere primitive bounded only by user permissions.
  • [nit] os.chmod after atomic_write_text has a narrow TOCTOU window between rename and chmod at src/apm_cli/adapters/client/_yaml_config.py:141
    Microsecond window, accepted trade-off; contextlib.suppress is correct for cross-platform compat.
  • [nit] _supports_runtime_env_substitution class attribute could be inadvertently overridden by a future subclass without enforcement at src/apm_cli/adapters/client/_yaml_config.py:58

OSS Growth Hacker

  • [recommended] CHANGELOG entry leads with MCP config details, burying the headline feature (Goose recipes) in clause three at CHANGELOG.md
    Suggested lead: "APM agents compile to Goose recipes at .goose/recipes/<name>.yaml and run natively via goose run --recipe <name>."
  • [nit] goose.md does not link to the Goose GitHub repo (github.com/block/goose) -- missed contributor funnel at docs/src/content/docs/integrations/goose.md

Auth Expert

  • [recommended] No user-visible warning when an MCP server config uses ${VAR} syntax -- values embedded literally at src/apm_cli/adapters/client/_yaml_config.py:55
    _supports_runtime_env_substitution = False means ${MY_API_KEY} in an MCP env block is written as the literal string to ~/.config/goose/config.yaml. Security rationale is documented in comments but nothing warns the user at install time.
  • [nit] ValueError message in _to_native_format interpolates name -- confirmed non-sensitive (registry server key) at src/apm_cli/adapters/client/goose.py:74

Doc Writer

  • [recommended] compile.md intro note box lists recommended targets but omits goose at docs/src/content/docs/producer/compile.md:20
    Goose requires apm compile -t goose to emit .goosehints -- the primary instruction mechanism. Without it in the "recommended for every other target" note, users skip compile and get no instruction context.
  • [recommended] GOOSE_RECIPE_PATH example uses a relative path that silently fails outside the project root at docs/src/content/docs/integrations/goose.md:92
    export GOOSE_RECIPE_PATH=.goose/recipes resolves against the cwd at goose-run time, not the project root. Change to $(pwd)/.goose/recipes.
  • [nit] goose.md sidebar order 10 -- verify consistent with other integration pages at docs/src/content/docs/integrations/goose.md:5

Test Coverage Expert

  • [recommended] No CliRunner integration test for apm install --target goose -- Hermes has a full parity suite at tests/integration/test_hermes_target.py
    tests/integration/test_hermes_target.py has TestHermesParserE2E and TestHermesDeployE2E; Goose has no equivalent. The CLI parser contract is unverified.
    Proof (missing at integration-with-fixtures): tests/integration/test_goose_target.py::TestGooseParserE2E::test_flag_off_parser_accepts_and_emits_hint -- proves: Running apm install --target goose (flag off) is accepted by the CLI parser and prints the experimental enable hint [devx, multi-harness-support]
  • [recommended] _goose_runtime_opted_in binary-on-PATH branch not tested; Hermes has test_flag_on_with_binary_only_opts_in at tests/unit/integration/test_goose_target.py:145
    Proof (missing at unit): tests/unit/integration/test_goose_target.py::test_goose_runtime_opts_in_via_binary_only -- proves: Goose MCP opt-in fires when the goose binary is on PATH even if ~/.config/goose does not exist [multi-harness-support, devx]
  • [nit] yaml_to_str(multiline_block=True) has no direct unit test in tests/unit/test_yaml_io.py
    Exercised indirectly via test_goose_recipe_renders_multiline_instructions_as_block; a direct unit test in test_yaml_io.py would isolate the _BlockStringDumper regression surface.

Performance Expert

  • [nit] _goose_runtime_opted_in() constructs a GooseClientAdapter() instance on every call to get the config path at src/apm_cli/integration/mcp_integrator_install.py:222
    Path computation is pure; expose a module-level _goose_config_path() function and call it directly. Not a measurable regression at current call frequency.

This panel is advisory. It does not block merge. Re-apply the panel-review label after addressing feedback to re-run.

Generated by PR Review Panel for issue #1833 · 758.7 AIC · ⌖ 8.23 AIC · ⊞ 5.5K ·

@github-actions github-actions Bot removed the panel-review Trigger the apm-review-panel gh-aw workflow label Jun 27, 2026
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.

3 participants