Skip to content

fix: pin the copilot target so contributor installs are deterministic - #2771

Open
Travis Illig (tillig) wants to merge 7 commits into
microsoft:mainfrom
tillig:fix/pin-targets-and-ignore-harness-deploy-roots
Open

fix: pin the copilot target so contributor installs are deterministic#2771
Travis Illig (tillig) wants to merge 7 commits into
microsoft:mainfrom
tillig:fix/pin-targets-and-ignore-harness-deploy-roots

Conversation

@tillig

@tillig Travis Illig (tillig) commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

TL;DR

apm.yml omitted targets:, so apm install auto-detected the harness. A contributor whose working tree carries a second harness signal hit a hard Multiple harnesses detected error on the plain apm install that CONTRIBUTING.md documents, and the suggested workaround appended 3206 lines of machine-specific paths to the tracked lockfile. This pins copilot -- the target that reproduces the committed tree byte-for-byte -- and documents a contributor-local exclusion for optional harness deploy roots.

Closes #2770.

Problem (WHY)

Auto-detection is fine on a clean checkout: only copilot signals, and bare apm install reproduces the committed tree exactly. It breaks as soon as a contributor's tree carries a second signal, which happens without them doing anything deliberate -- Claude Code writes .claude/settings.local.json on the first permission approval, and .claude/ is the claude detection signal. Cursor, Codex, and Gemini are equivalent.

From there:

  1. apm install fails closed with Multiple harnesses detected: claude, copilot. There is no user-level default-target escape hatch in the resolution chain (--target > apm.yml targets: > auto-detect), so the documented first-run command simply does not work.
  2. Following the error's own advice, apm install --target claude, appends .claude/** to the tracked apm.lock.yaml and leaves .claude/ untracked -- .gitignore covered .copilot/ and .cursor/mcp.json, but no other harness deploy root.
  3. Committing that lockfile without the files, which git add -A makes easy, blocked deployed-files-present for everyone: the paths were neither present nor gitignored, and no apm install on another machine could restore them.

Approach (WHAT)

Both edits are to this repo's own configuration; no CLI behavior changes.

apm.yml pins targets: [copilot]. copilot is the target that reproduces both governed deploy roots -- .github/ directly, and .agents/skills/ through skills convergence. Declaring it makes every contributor's install produce the same tree regardless of local signals, and removes the ambiguity error entirely.

CONTRIBUTING.md documents a local exclusion for an optional harness deploy root. A contributor who deliberately opts in with --target claude can add .claude/ to Git's repository-local exclude file before installation. deployed-files-present then treats the paths as expected-absent (#2452) without making security-sensitive harness configuration invisible to every contributor. CONTRIBUTING.md still tells contributors to leave the resulting lockfile change uncommitted.

CONTRIBUTING.md documents the opt-in, and drops a stale claim: it promised deployment into .github/skills/, which is not a deploy root, since skills converge to .agents/skills/.

Rejected alternative

Declaring every target. It would deploy eleven near-identical trees into every contributor's working tree, require committing or ignoring all of them, and work against the skills convergence that exists precisely so one .agents/skills/ serves Copilot, Cursor, Codex, Gemini, OpenCode, Windsurf, and Antigravity.

Known residual

With targets: declared, an explicit --target claude still appends its deploy paths to the shared lockfile. #2059 fixed the mirror case (paths install never writes); this is paths install writes on one machine only, into a file every machine shares. Whether deployed_files should record a deploy root outside the manifest's declared target set is a maintainer call, so it is out of scope here.

Validation evidence

All runs use the 0.29.0 (5ac6733) release binary against base b100108, in throwaway clones.

Scenario Principle Before After Automated test
Clean clone, bare apm install Portability by manifest reproduces committed tree, zero diff unchanged Target-resolution suite
Clone + .claude/settings.local.json, bare apm install Portability by manifest, multi-harness support, DevX Error: [x] Multiple harnesses detected: claude, copilot [i] Targets: copilot (source: apm.yml), tree clean test_repository_manifest_pin_overrides_second_harness_signal
Clone + local .claude/ exclusion, apm install --target claude DevX apm.lock.yaml +3206/-0, ?? .claude/ untracked .claude/ locally excluded; only M apm.lock.yaml Manual fixture scenario
apm audit --ci on the branch Governed by policy n/a all 10 checks pass, including drift and deployed-files-present apm audit --ci
Churned lockfile + deploy root absent (fresh-checkout simulation) Governed by policy deployed-files-present blocks, unfixable all 10 checks pass apm audit --ci

Automated validation on the final head: the full Lint mirror passed; the target-resolution and MCP-scope suites passed with 75 tests (2 expected xfails); and all required GitHub checks passed.

How to test

git clone -b fix/pin-targets-and-ignore-harness-deploy-roots https://github.com/tillig/apm && cd apm

# Simulate any contributor who uses a second harness.
mkdir .claude && echo '{}' > .claude/settings.local.json

apm install          # [i] Targets: copilot (source: apm.yml) -- no ambiguity error
git status --short   # clean

apm audit --ci       # all 10 checks pass

# Opt in to another harness while keeping its generated root local.
printf '.claude/\n' >> "$(git rev-parse --git-path info/exclude)"
apm install --target claude
git status --short   # M apm.lock.yaml only; .claude/ is locally excluded

# Confirm a fresh checkout still audits clean with the deploy root absent.
rm -rf .claude && apm audit --ci

Check out main and repeat the first three commands to see the failure.

apm.yml omitted `targets:`, so `apm install` auto-detected the harness.
A contributor whose tree carries a second signal -- `.claude/` from Claude
Code, `.cursor/`, `.codex/` -- hit `Multiple harnesses detected` on the
plain `apm install` CONTRIBUTING.md documents. Resolving that with
`--target claude` appended 3206 lines of `.claude/**` paths to the tracked
lockfile and left the deploy root untracked, breaking
`deployed-files-present` for everyone if committed.

`copilot` reproduces the committed tree byte-for-byte, so pin it and
ignore the harness roots this repo does not govern.

Closes microsoft#2770

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.

🟡 Changes recommended

The added changelog entry does not follow the project’s “one line per PR” changelog rule and should be condensed accordingly.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Pins this repository’s APM installation target to copilot so contributor installs are deterministic even when other harness detection signals exist locally, and updates repo hygiene/docs to avoid accidental lockfile churn and untracked harness outputs.

Changes:

  • Add targets: [copilot] to apm.yml to remove harness auto-detect ambiguity for apm install.
  • Expand .gitignore to cover additional harness deploy roots that this repo does not govern.
  • Update contributor documentation and add a changelog entry describing the fix.
File summaries
File Description
CONTRIBUTING.md Updates contributor instructions to reflect pinned copilot target and opt-in --target usage for other harnesses.
CHANGELOG.md Adds an Unreleased “Fixed” entry describing deterministic installs and ignoring non-governed harness deploy roots.
apm.yml Pins targets: [copilot] to make apm install deterministic across contributor environments.
.gitignore Ignores additional harness deploy roots (e.g., .claude/, .codex/, .opencode/) to keep opt-in installs untracked.
Review details
  • Files reviewed: 3/4 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread CHANGELOG.md Outdated
Comment on lines +18 to +21
- `apm.yml` now pins the `copilot` target and `.gitignore` covers the harness
deploy roots this repo does not govern, so a contributor using another
harness no longer hits `Multiple harnesses detected` on plain `apm install`
or commits machine-specific lockfile paths. (#2771)
@danielmeppiel

Copy link
Copy Markdown
Collaborator

APM Review Panel: needs_rework

PR #2771 makes contributor installs deterministic, but its broad ignore rules weaken repository visibility over security-sensitive harness configuration.

cc Travis Illig (@tillig) Sergio Sisternes (@sergio-sisternes-epam) -- a fresh advisory pass is ready for your review.

The panel agrees that pinning the repository-local Copilot target is a clean use of existing manifest authority, improves contributor setup, and requires no new Python abstraction or CLI behavior.

The security concern outweighs the convenience of blanket root ignores. Hiding prompt, settings, MCP, or hook files from git status weakens review and governance. Keep the deterministic pin, but narrow the ignored paths to generated artifacts or direct contributors to .git/info/exclude. The documentation findings are also valid: --target overrides rather than supplements the manifest target, and the CHANGELOG should describe repository-local behavior without claiming explicit overrides cannot affect the lockfile.

Dissent. DevX favors the convenience of the alternate harness setup, while supply-chain security identifies a visibility cost in making broad ignores the default; security should govern the default, with local exclusions preserving opt-in ergonomics. The Copilot wrapping objection is not persuasive because one logical bullet may span physical lines, although the substantive wording corrections remain warranted.

Aligned with: Portable by manifest: the repository manifest becomes the authoritative, reproducible default for contributor installs. Secure by default: broad ignore rules should not conceal security-sensitive configuration. Governed by policy: harness configuration should remain visible unless locally excluded. Multi-harness support: explicit overrides preserve choice. OSS community driven: deterministic setup reduces onboarding friction. Pragmatic as npm: the manifest supplies a default and the command line remains the escape hatch.

Growth signal. Deterministic dogfooding is a meaningful contributor-activation improvement, but it is maintenance hygiene rather than a standalone launch story.

Panel summary

Persona B R N Takeaway
Python Architect 0 0 1 The declarative pin uses the existing target authority and introduces no parallel Python decision path.
CLI Logging Expert 0 0 0 No CLI output or diagnostic implementation changes; no concerns from this lens.
DevX UX Expert 0 0 0 No DevX UX concerns; bare installs are deterministic, and alternate-target behavior and lockfile churn are clearly documented.
Supply Chain Security Expert 0 1 0 Avoid blanket ignores that hide agent-readable and executable harness configuration.
OSS Growth Hacker 0 1 0 The deterministic contributor setup reduces friction, but the changelog must clearly scope the change to this repository.
Doc Writer 0 2 1 Changelog wrapping matches one-entry-per-PR practice, but its lockfile claim overreaches; CONTRIBUTING needs clearer explicit-target wording.

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

Top 3 follow-ups

  1. [Supply Chain Security Expert] Replace whole-root ignore patterns with narrowly enumerated generated paths or contributor-local .git/info/exclude entries. -- Git status and APM governance should continue exposing security-sensitive prompt, settings, MCP, and hook files.
  2. [Doc Writer] State that --target overrides the manifest target rather than adding another target. -- Contributors need an accurate mental model for deterministic defaults and explicit escape hatches.
  3. [Doc Writer and OSS Growth Hacker] Rewrite the CHANGELOG entry as one repository-scoped bullet and avoid claiming explicit overrides cannot write machine-specific target choices to the lockfile. -- This preserves the one-entry-per-PR convention while preventing a repository dogfooding change from being mistaken for a product-wide default.

Architecture

classDiagram
    direction LR
    class ApmYml {
      <<ConfigurationArtifact>>
      +targets: copilot
    }
    class GitIgnore {
      <<RepositoryPolicy>>
      +alternate harness roots
    }
    class EffectiveTargetDecision {
      <<FrozenValueObject>>
      +value
      +source
    }
    class CIChecks {
      <<IOBoundary>>
      +deployed-files-present
    }
    ApmYml ..> EffectiveTargetDecision : supplies manifest policy
    GitIgnore ..> CIChecks : supplies local visibility policy
Loading
flowchart TD
    A[apm install] --> B{--target supplied?}
    B -- yes --> C[Use explicit target]
    B -- no --> D{apm.yml targets present?}
    D -- yes --> E[Use pinned copilot target]
    D -- no --> F[Auto-detect harness signals]
    C --> G[Deploy target artifacts]
    E --> G
    F --> G
    G --> H[Write lockfile]
Loading

Recommendation

Preserve the target pin and contributor guidance, but narrow the ignore policy before shipping and correct the override and CHANGELOG wording in the same pass.


Full per-persona findings

Python Architect

  • [nit] No additional abstraction is warranted for this repository-local policy change at apm.yml:12.
    The existing target-resolution authority remains canonical.

CLI Logging Expert

No findings.

DevX UX Expert

No findings.

Supply Chain Security Expert

  • [recommended] Keep security-sensitive harness files visible to repository review at .gitignore:100.
    Whole-root ignores conceal prompts, settings, MCP configuration, and hooks. Prefer contributor-local .git/info/exclude entries.

OSS Growth Hacker

  • [recommended] Clarify that the changelog entry describes this repository, not global APM behavior at CHANGELOG.md:21.
    This is contributor-workflow maintenance rather than a product-wide default.

Auth Expert -- inactive

The diff changes repository config and contributor documentation, with no auth surface.

Doc Writer

  • [recommended] Scope the changelog claim to plain installs at CHANGELOG.md:21.
    Explicit --target installs still add target-specific paths to apm.lock.yaml.
  • [recommended] Clarify that --target overrides the manifest target at CONTRIBUTING.md:60.
    "As well" incorrectly suggests additive behavior.
  • [nit] Name the command that changes the lockfile at CONTRIBUTING.md:66.

Test Coverage Expert -- inactive

The diff contains no src/**/*.py files.

Performance Expert -- inactive

The diff contains no runtime or performance-sensitive changes.

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

Keep alternate harness configuration visible by default while documenting a local exclusion workflow. Clarify target override semantics and scope the changelog entry. Addresses panel follow-ups and Copilot review on CHANGELOG.md:21.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep the local exclude example aligned with its selected harness and classify the repository configuration under Changed. Addresses final panel documentation follow-ups.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Pass an explicit empty manifest to scope-only tests so the repository's newly pinned target does not narrow their runtime candidates. Addresses CI Build and Test Shard 2 failures.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Exercise the repository manifest through a plain install with a second harness signal and point alternate-target contributors at the canonical output-directory catalogue. Addresses terminal panel follow-ups.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@danielmeppiel

Copy link
Copy Markdown
Collaborator

APM Review Panel: ship_now

PR #2771 makes contributor installs deterministic by pinning the canonical Copilot target while preserving explicit multi-harness opt-in.

cc Travis Illig (@tillig) Sergio Sisternes (@sergio-sisternes-epam) -- a fresh advisory pass is ready for your review.

The panel converges on shipping. The target pin follows the canonical resolution chain, requires no new abstraction, and introduces no security, logging, documentation, performance, or UX concern. Earlier ignore, wording, CI-isolation, and regression-test concerns are folded.

The integration guard at tests/integration/test_target_resolution_e2e.py proves the promise through a successful bare install with a second harness signal and provenance from apm.yml; the mutation break confirms the test detects removal of the pin. All required CI is green.

Aligned with: Portable by manifest: the manifest selects one deterministic target regardless of local signals. Governed by policy: the pinned configuration and regression test keep installs reproducible. Multi-harness support: explicit overrides remain available. OSS community driven: the fix removes environment-dependent contributor friction. Pragmatic as npm: plain apm install works without extra flags.

Growth signal. Deterministic first-run installs reduce contributor setup failures and strengthen trust that the committed manifest reproduces the same project across environments.

Panel summary

Persona B R N Takeaway
Python Architect 0 0 1 The pin routes through the canonical target decision owner.
CLI Logging Expert 0 0 0 Target provenance stays concise and explicit.
DevX UX Expert 0 0 0 Default and alternate-harness flows are clear and canonical.
Supply Chain Security Expert 0 0 0 Determinism improves without weakening integrity or containment.
OSS Growth Hacker 0 0 0 Contributor-funnel friction is resolved.
Doc Writer 0 0 0 Target, exclusion, lockfile, catalogue, and changelog claims are accurate.
Test Coverage Expert 0 0 0 Integration evidence is passing and mapped in the PR body.

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

Architecture

classDiagram
    direction LR
    class RepositoryManifest {
      <<Configuration>>
      +targets copilot
    }
    class EffectiveTargetDecision {
      <<ValueObject>>
      +value
      +source
    }
    class InstallPipeline {
      <<Module>>
      +run_install_pipeline()
    }
    class TargetResolutionTests {
      <<IntegrationTest>>
      +test_repository_manifest_pin_overrides_second_harness_signal()
    }
    RepositoryManifest ..> EffectiveTargetDecision : supplies manifest policy
    InstallPipeline ..> EffectiveTargetDecision : consumes canonical decision
    TargetResolutionTests ..> RepositoryManifest : verifies pin
Loading
flowchart TD
    A[apm install] --> B{--target supplied?}
    B -- yes --> C[Use explicit target]
    B -- no --> D{apm.yml targets present?}
    D -- yes --> E[Use pinned copilot target]
    D -- no --> F[Auto-detect harness signals]
    C --> G[Deploy target artifacts]
    E --> G
    F --> G
Loading

Recommendation

Ship the current head. The user promise is documented, regression-tested, mutation-validated, and green across required CI, with no remaining panel follow-ups.

Folded in this run

  • (panel) Replaced blanket harness-root ignores with a contributor-local exclusion workflow -- resolved in 6c01e7f.
  • (panel) Clarified target override, lockfile, catalogue, and changelog guidance -- resolved in 6c01e7f, bc758f2, and 0afa722.
  • (copilot) Condensed the changelog to one repository-scoped line -- resolved in 6c01e7f.
  • (panel) Isolated MCP scope tests from the repository manifest -- resolved in 6528ac1.
  • (panel) Added a mutation-verified integration regression test and Scenario Evidence mapping -- resolved in 0afa722.

Copilot signals reviewed

  • CHANGELOG.md:21 -- LEGIT: the original entry spanned four physical lines despite the one-line-per-PR contract (resolved in 6c01e7f).

Regression-trap evidence (mutation-break gate)

  • tests/integration/test_target_resolution_e2e.py::test_repository_manifest_pin_overrides_second_harness_signal -- removed apm.yml targets: [copilot]; test failed as expected; guard restored.

Lint contract

The full CI-mirror ruff, pylint R0801, auth-boundary, and architecture-boundary checks exited 0. CI Lint passed.

CI

All required checks passed on 0afa7223737503bd3b4fd88b41e1fb90429251da, including CI, CodeQL, spec conformance, NOTICE, and merge gate, after 1 CI fix iteration.

Mergeability status

PR head SHA CEO stance iters folds defers Copilot rounds CI mergeable mergeStateStatus notes
#2771 0afa722 ship_now 4 8 0 2 green MERGEABLE BLOCKED pending required review

Convergence

4 outer iterations; 2 Copilot rounds. Final panel stance: ship_now.

Ready for maintainer review.


Full per-persona findings

Python Architect

  • [nit] No additional abstraction is warranted. The declarative pin uses the existing target resolution chain.

CLI Logging Expert

No findings.

DevX UX Expert

No findings.

Supply Chain Security Expert

No findings.

OSS Growth Hacker

No findings.

Auth Expert -- inactive

No authentication surface changed.

Doc Writer

No findings.

Test Coverage Expert

No findings after the regression test and Scenario Evidence mapping were folded.

Performance Expert -- inactive

No runtime performance surface changed.

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

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.

[BUG] apm.yml omits targets:, so contributor apm install fails closed or churns the committed lockfile

3 participants