Skip to content

[BUG] audit --ci falsely rejects skills subsets when checkout apm_modules is absent #3136

Description

@hcoona

Describe the bug

apm audit --ci falsely fails skill-subset-consistency in a fresh checkout containing valid committed APM outputs when a dependency uses skills:. The same manifest, lockfile and deployed files pass immediately after installation. Fresh scratch replay reports no drift; all other nine checks pass.

Reproduced on 0.30.0 and current release 0.32.0 with one public pinned dependency and one selected skill. No private repository, custom APM patches, MCP configuration or org policy is involved.

To Reproduce

  1. Create a Git repository with this apm.yml and .gitignore containing apm_modules/:
name: audit-subset-repro
version: 1.0.0
description: Minimal public dependency audit reproduction.
targets:
  - copilot
policy:
  discovery_enabled: false
dependencies:
  apm:
    - git: dotnet/skills
      path: plugins/dotnet-diag
      ref: 4c72b17fa2c2aaa307d8f1e337ec75cab45ac5c7
      skills:
        - dump-collect
  1. Run apm install, then apm audit --ci --no-policy --no-fail-fast --format json. Both exit 0.
  2. Commit all generated files, manifest, lockfile and .gitignore.
  3. Run git clone --no-local <seed-path> <fresh-path>. The clone has no apm_modules.
  4. Run the same audit command in the clone. It exits 1, reporting only the selected skill missing. The checkout remains unchanged and apm_modules remains absent.

The complete script below automates these steps: python repro.py --apm /path/to/apm, or python repro.py with APM on PATH. It creates and retains temporary fixtures; it does not modify an existing repository. Public GitHub access is required.

Expected behavior

The fresh checkout should pass. Validate selected skill existence against the lock-pinned dependency tree prepared by audit, while preserving invalid-selection and deployed-drift checks.

The official audit-only pattern for committed outputs explicitly avoids installing into the checkout first, because installation overwrites the bytes audit should inspect.

Environment

  • Azure Linux 3.0, x86_64.
  • APM 0.30.0 / Python 3.12.11; APM 0.32.0 / Python 3.13.12.
  • 0.32.0 installed from PyPI into an isolated uv tool directory.
  • Fresh means no checkout-local apm_modules, not absence of all host caches.
  • Windows execution has not been tested.

Logs

Both versions: warm audit exit 0; fresh audit exit 1, 9/10 checks pass. Only failure:

skill-subset-consistency: 1 skill subset mismatch(es) -- regenerate lockfile (apm install)
dotnet/skills/plugins/dotnet-diag: recorded skill subset path(s) not found in package tree: dump-collect

Drift passes with no drift detected against lockfile and an empty drift list. The script verifies unchanged checkout contents.

Additional context / suspected cause

In 0.32.0 ci_checks.py, _check_skill_subset_consistency forwards project_root; _missing_recorded_skill_subset_paths uses dep_ref.get_install_path(project_root / APM_MODULES_DIR), which is absent in the clone. The adjacent configuration check instead consumes prepared_replay.modules_root.

This looks like a missing integration between subset validation and cold-checkout replay. Using the prepared dependency tree may be a fix direction; no fix is implemented or validated here. The lookup also remains in main at 4e916ecbddd14edb2f0c93d0199ac9bfa4a7d3dd (source inspected only).

This differs from #2172: drift passes here; the baseline subset-existence check fails specifically without checkout-local modules. Existing subset/audit issues were searched before filing.

Complete reproduction script
"""Reproduce APM subset audit failure using one public pinned dependency."""
import argparse
import hashlib
import json
import os
from pathlib import Path
import shutil
import subprocess
import tempfile

parser = argparse.ArgumentParser()
parser.add_argument('--apm', default='apm', help='APM executable (0.30.0 or 0.32.0)')
args = parser.parse_args()
apm = shutil.which(args.apm)
if not apm:
    raise SystemExit('APM executable not found')
root = Path(tempfile.mkdtemp(prefix='apm-subset-repro-'))
seed = root / 'seed'
seed.mkdir()
env = os.environ.copy()
# Keep public Git operations independent of user Git configuration.
env['GIT_CONFIG_GLOBAL'] = os.devnull
env['GIT_CONFIG_NOSYSTEM'] = '1'
env['GIT_TERMINAL_PROMPT'] = '0'
for key in list(env):
    if key.startswith(('GIT_CONFIG_KEY_', 'GIT_CONFIG_VALUE_')) or key in ('GIT_CONFIG_COUNT', 'GITHUB_TOKEN', 'GH_TOKEN', 'GITHUB_APM_PAT'):
        env.pop(key, None)
env['XDG_CACHE_HOME'] = str(root / 'cache')

def run(name, command, cwd, expected=0):
    result = subprocess.run(command, cwd=cwd, env=env, capture_output=True, text=True, timeout=180)
    (root / (name + '.log')).write_text(result.stdout + result.stderr, encoding='utf-8')
    print(f'{name}: exit {result.returncode}', flush=True)
    if result.returncode != expected:
        raise RuntimeError(f'{name}: expected {expected}; inspect {root}')
    return result

manifest = '''name: audit-subset-repro
version: 1.0.0
description: Minimal public dependency audit reproduction.
targets:
  - copilot
policy:
  discovery_enabled: false
dependencies:
  apm:
    - git: dotnet/skills
      path: plugins/dotnet-diag
      ref: 4c72b17fa2c2aaa307d8f1e337ec75cab45ac5c7
      skills:
        - dump-collect
'''
(seed / 'apm.yml').write_text(manifest, encoding='utf-8')
(seed / '.gitignore').write_text('apm_modules/\n', encoding='utf-8')
print(f'Artifacts: {root}', flush=True)
run('version', [apm, '--version'], seed)
run('git-init', ['git', 'init'], seed)
run('install', [apm, 'install'], seed)
run('warm-audit', [apm, 'audit', '--ci', '--no-policy', '--no-fail-fast', '--format', 'json'], seed)
run('git-add', ['git', 'add', '.'], seed)
run('git-commit', ['git', '-c', 'user.name=Repro', '-c', 'user.email=repro@example.invalid', 'commit', '-m', 'Add generated public fixture'], seed)
fresh = root / 'fresh'
run('git-clone', ['git', 'clone', '--no-local', str(seed), str(fresh)], root)
assert not (fresh / 'apm_modules').exists()
def snapshot():
    return {str(p.relative_to(fresh)): hashlib.sha256(p.read_bytes()).hexdigest() for p in fresh.rglob('*') if p.is_file() and '.git' not in p.relative_to(fresh).parts}
before = snapshot()
result = run('fresh-audit', [apm, 'audit', '--ci', '--no-policy', '--no-fail-fast', '--format', 'json'], fresh, expected=1)
payload = json.loads(result.stdout)
assert [c['name'] for c in payload['checks'] if not c['passed']] == ['skill-subset-consistency']
assert payload['drift']['drift'] == []
assert not (fresh / 'apm_modules').exists()
assert snapshot() == before, 'Audit modified checkout files'
print('Confirmed: warm audit passes; fresh audit exits 1; checkout unchanged.', flush=True)
print(f'Read {root / "fresh-audit.log"} to verify the failing check.', flush=True)
Full 0.32.0 fresh audit output
{
  "passed": false,
  "checks": [
    {
      "name": "lockfile-exists",
      "passed": true,
      "message": "Lockfile present",
      "details": []
    },
    {
      "name": "ref-consistency",
      "passed": true,
      "message": "All dependency refs match lockfile",
      "details": []
    },
    {
      "name": "deployment-ledger-owners",
      "passed": true,
      "message": "All deployment ledger owners are valid",
      "details": []
    },
    {
      "name": "deployed-files-present",
      "passed": true,
      "message": "All deployed files present on disk",
      "details": []
    },
    {
      "name": "no-orphaned-packages",
      "passed": true,
      "message": "No orphaned packages in lockfile",
      "details": []
    },
    {
      "name": "skill-subset-consistency",
      "passed": false,
      "message": "1 skill subset mismatch(es) -- regenerate lockfile (apm install)",
      "details": [
        "dotnet/skills/plugins/dotnet-diag: recorded skill subset path(s) not found in package tree: dump-collect"
      ]
    },
    {
      "name": "config-consistency",
      "passed": true,
      "message": "No MCP configs to check",
      "details": []
    },
    {
      "name": "content-integrity",
      "passed": true,
      "message": "No critical hidden Unicode or hash drift detected",
      "details": []
    },
    {
      "name": "includes-consent",
      "passed": true,
      "message": "No local content deployed -- includes consent check skipped",
      "details": []
    },
    {
      "name": "drift",
      "passed": true,
      "message": "no drift detected against lockfile",
      "details": []
    }
  ],
  "summary": {
    "total": 10,
    "passed": 9,
    "failed": 1
  },
  "drift": {
    "drift": []
  }
}
[>] Diffing scratch vs working tree...
[+] No drift detected

Prepared and reproduced with assistance from OpenAI Codex.

Activity

  1. danielmeppiel commented on Oct 2, 2026

    @danielmeppiel
    Collaborator

    Decision: approve
    Area: project
    Scope: Make skill-subset consistency validation use the prepared lock-pinned audit replay tree when available, rather than incorrectly requiring a local checkout apm_modules directory. Preserve the existing audit contract and all real invalid-selection, manifest/lock mismatch, integrity and deployed-drift checks.
    Done when: A clean checkout with valid committed outputs and skill subsets passes audit without first installing into the checkout; genuinely invalid selections, subset mismatches and deployed drift still fail; present, absent and stale checkout dependency trees are covered. The audit remains non-mutating to the checkout and related CI guidance stays accurate.
    Out of scope: Suppressing the subset check, weakening integrity or drift validation, making a hidden pre-install step mandatory, changing the lockfile subset format, or unrelated install/audit redesign.
    Review contact: Daniel Meppiel (@danielmeppiel)

    The pre-analysis traced this check to the absent checkout tree while adjacent validation already uses the prepared replay. This is source evidence, not a rerun of the reporter's reproduction. #3062 is a useful precedent for correcting audit false positives while preserving real checks, but it is a different defect and repair mechanism.

    Recorded by Copilot on behalf of Daniel Meppiel (@danielmeppiel) after his explicit "ok to all yes" confirmation of the four priority batches on 2026-10-02. This records the human-approved scope, not an autonomous recommendation. It does not assign implementation, promise a release, approve a merge, or replace required review.

  2. danielmeppiel commented on Oct 2, 2026

    @danielmeppiel
    Collaborator

    Decision: approve
    Area: project
    Scope: Make skill-subset consistency validation use the prepared lock-pinned audit replay tree when available, rather than incorrectly requiring a local checkout apm_modules directory. Preserve the existing audit contract and all real invalid-selection, manifest/lock mismatch, integrity and deployed-drift checks.
    Done when: A clean checkout with valid committed outputs and skill subsets passes audit without first installing into the checkout; genuinely invalid selections, subset mismatches and deployed drift still fail; present, absent and stale checkout dependency trees are covered. The audit remains non-mutating to the checkout and related CI guidance stays accurate.
    Out of scope: Suppressing the subset check, weakening integrity or drift validation, making a hidden pre-install step mandatory, changing the lockfile subset format, or unrelated install/audit redesign.
    Review contact: Daniel Meppiel (@danielmeppiel)
    Reason: Formatting-only republication of the unchanged human-approved scope at #3136 (comment); the original remains the rationale and context source. Recorded by Copilot on behalf of Daniel Meppiel (@danielmeppiel) following his explicit acceptance of these eight bounded scopes on 2026-10-02 and his subsequent instruction to start their delivery. No scope expansion, new product decision, assignment or merge approval.

  3. danielmeppiel commented on Oct 5, 2026

    @danielmeppiel
    Collaborator

    Decision: approve
    Area: project
    Scope: Resume end-to-end convergence of our existing PR #3147 for issue #3136 in the new convergence-20261005T122639Z run. Deliver the previously accepted use of the prepared lock-pinned audit replay tree for skill-subset consistency when available, without requiring a checkout apm_modules directory. Preserve all real invalid-selection, manifest/lock mismatch, integrity and deployed-drift checks and checkout non-mutation. Fold every correctness, regression, conformance, guidance and finalization gap within that behavior. Execute genuine general review through autopilot-pr-merge-worker and its composed review worker, with complete current conversation and actual final-head evidence; do not reuse self-authored persona-shaped JSON as independent review. Account for necessary generated copies and exact content-derived lock hashes while preserving unrelated dependency and lock state.
    Done when: Valid committed outputs audit successfully from a clean checkout without installing into it; invalid selections, subset mismatches and drift still fail with present, absent and stale checkout dependency trees covered. Actual exact-head functional and mutation evidence, complete conversation, genuine reviewer executions and accurate test identifiers are preserved. Current main is incorporated without rewriting published history; the complete local CI-mirror lint contract, canonical completion schema and owner semantic verifier pass with actual exits/output. Checkout is clean, locks accounted for, required CI is complete and successful, and live head/base/nonconflicting mergeability are rechecked. After all CI, review and PR-body work, one new terminal advisory truthfully recommends ship_now at the exact head, is verified as the latest top-level PR comment, and states any remaining human branch-protection review requirement. Old blocked receipts and advisories remain historical; do not merge.
    Out of scope: Suppressing subset checks, weakening integrity/drift validation, requiring hidden pre-install, changing lockfile subset format, unrelated install/audit redesign, unapproved conformance waivers, dependency upgrades or unexplained lock churn, contributor takeover or other PRs. No fabricated persona/CEO execution, synthetic test IDs or passing claims, weakened owner/schema/CI gates, reviewer bypass, human approval impersonation, auto-merge, merge, amend or published-history rewrite. This new run retains the merge-worker caps of four outer iterations, two Copilot rounds and three CI recoveries; preserve closed attempts and their exhausted budgets unchanged.
    Review contact: Daniel Meppiel (@danielmeppiel)
    Reason: Recorded by Copilot on behalf of Daniel Meppiel (@danielmeppiel) from the live human direction relayed by the product-lead session on 2026-10-05: "your goal is to keep looping until what we committed working on is in mergeable state with a ship_now comment as last comment", together with "allow everything, I need you to move forward fast and stop asking questions" for routine in-scope details. This renews bounded execution for this PR, not merging or additional product scope. It supplements the unchanged original acceptance #3136 (comment) and is the nominated current run scope; all previous records and blocked receipts remain intact. The authorized coordinator approves the exact in-scope plan before technical mutation. This strict machine record is evidence, never implementation permission alone. By the explicit strict-record exception, AI attribution is inside Reason; ordinary advisories retain their footer/watermark. This AI-generated transcription may contain errors.

  4. added a commit that references this issue on Oct 6, 2026
    0c0d689
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

status/acceptedHuman scope approval; verify the issue's approval record and review contact before work.

Type

No type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions