Skip to content

fix(audit): bound the deploy-tree hidden-Unicode walk to primitive directories - #2962

Merged
Daniel Meppiel (danielmeppiel) merged 21 commits into
microsoft:mainfrom
lkshrk:fix/audit-scan-primitive-roots
Sep 30, 2026
Merged

Daniel Meppiel (danielmeppiel) merged 21 commits into
microsoft:mainfrom
lkshrk:fix/audit-scan-primitive-roots

Conversation

@lkshrk

@lkshrk lkshrk (lkshrk) commented Sep 12, 2026 •

Copy link
Copy Markdown
Contributor

fix(audit): separate deployed primitive discovery from prompt checks

TL;DR

Automatic audit discovers recognized primitive files and native hook entries without walking entire tool roots. It checks prompt documents and documented decoded prompt fields, not executable commands or unrelated configuration. Unknown or unreadable recognized content fails with explicit incomplete coverage; discovering a manual hook does not imply unsafe content, ownership, or hash verification.

Warning

The bounded OpenAPM 0.1.42 amendment is proposed, not adopted. Its actual guardian recommendation is fold_and_ship (8/10), but Section 9.3 human approvals and the subsequent 14-day comment period remain outstanding. Technical ship_now and green CI do not waive that process.

Section 9.3 requires a spec-labelled issue and at least two non-author approvals covering implementation and consumer/integrator experience. Linked issue #2961 lacks the spec label, and the observed PR review history has no qualifying approvals. The 14-day clock follows that panel approval, not the September 30 proposed-history entry. No earliest adoption date is established.

Problem (WHY)

The maintainer-approved contract is the authority. Explicit per-format rules follow Agent Skills' guidance to provide "project-specific conventions, domain-specific procedures, non-obvious edge cases, and the particular tools or APIs to use", rather than infer native formats from arbitrary strings.

Approach (WHAT)

  • Derive discovery from scope-resolved target mappings, generated context filenames and the existing native hook registry.
  • Route recorded-file and untracked discovery through the same prompt applicability owner.
  • Decode JSON/TOML prompt fields before scanning, including escaped Unicode.
  • Preserve visible command-only hook inventory without executing or following commands.
  • Carry incomplete coverage through text, JSON, SARIF, Markdown and CI failures.
  • Keep hash, ownership and replay-drift evidence independent; refuse automatic strip on structured/shared, external-root and user-scope findings.

Implementation (HOW)

Files Intent
security/primitive_discovery.py Canonical target-derived patterns, bounded enumeration and containment/symlink checks.
security/file_scanner.py Typed inventory/coverage, shared claimed/discovered applicability, scope preservation, interpretation-aware caching and protected remediation.
integration/targets.py Declare Gemini prompt and Codex developer_instructions TOML fields in existing mappings; reuse canonical skill destinations.
hook_contract.py, integration/hook_integrator.py, integration/hook_native_formats.py Reuse handler/native-format declarations, including layouts actually preserved by Cursor/Windsurf integrators; never execute commands.
compilation/root_context_protection.py Expose the existing compile-family filename authority for discovery.
commands/audit.py, policy/ci_checks.py, policy/models.py Show inventory, fail incomplete coverage, retain independent ownership/hash checks and preserve exact CI finding pointers.
security/content_scanner.py, security/audit_report.py Preserve structured prompt coordinates and share ordinary/CI JSON/SARIF serialization; keep rendered Markdown safe without corrupting machine pointers.
install/manifest_reconcile.py Retain the author's opt-in bounded governance helper; default ownership/pruning behavior stays unchanged.
.apm/architecture/owners/hooks-integrations.json, scripts/architecture_linter/checks/audit_discovery.py, groups/mutation_writes.py Register discovery and serializer owners and enforce canonical consumer routing.
tests/integration/test_audit_primitive_contract.py, test_architecture_audit_discovery.py, test_audit_unrecorded_unicode_lifecycle.py Prove applicability, exclusions, no-execution, failure preservation, source-override sensitivity and installed-CLI lifecycle behavior.
tests/integration/test_architecture_owner_rule_mutations.py Add meaningful exhaustive-matrix cases for both audit owners and the inherited marketplace package-remote guard, preserving registry equality.
Audit command, policy, target-catalog, scanner and ownership tests Preserve exact expected metadata and valid native happy paths, alongside separate malformed/unsupported nonzero-no-write cases.
docs/src/content/docs/reference/cli/audit.md, reference/baseline-checks.md, enterprise/security.md, apm-usage commands.md Document supported patterns/fields, inventory versus checks, incomplete status, filesystem-only App coverage and remediation limits.
docs/src/content/docs/specs/openapm-v0.1.md, requirements manifest, policy conformance tests/generator, CONFORMANCE.* Propose optional deployed-prompt-audit capability and conditional req-pl-019/020, backed by real conformance tests and explicit pending adoption. No schema redesign.

Unqualified production paths above are under src/apm_cli/. The discovery implementation is independent of ownership evidence. Decoded prompt offsets are not represented as physical source coordinates.

Diagrams

Dashed nodes are the new discovery/applicability stages; command-only inventory reaches reports without becoming a Unicode finding.

flowchart LR
    subgraph Discovery["Discovery"]
        T["Target profiles and hook registry"] --> S["primitive_surfaces"]
        L["Recorded deployment claims"] --> U["scan_project_result"]
        S --> U
    end
    subgraph Applicability["Applicability"]
        U --> P["Documented prompt text"]
        U --> C["Command-only inventory"]
        U --> E["Unreadable or unsupported content"]
    end
    subgraph Reporting["Reporting"]
        P --> F["Unicode findings"]
        C --> R["Audit and CI reports"]
        E --> I["Incomplete coverage: exit 1"]
        F --> R
        I --> R
    end
    classDef new stroke-dasharray: 5 5;
    class S,U,P,C,E,I new;
Loading

Trade-offs

  • Recognized formats are explicit. Unsupported/malformed formats fail coverage rather than guessing or claiming a clean scan.
  • Skill discovery recognizes SKILL.md, not arbitrary resources/executables. Explicit recorded prompt documents retain claim-path coverage.
  • Structured/shared, external-root and user-scope findings require manual review. Explicit --file remains user-directed; discovery grants no configuration rewrite authority.
  • File-level tracked does not certify per-entry ownership. Canonical ledger/hash and owned-hook projection checks retain their evidence requirements.
  • The issue's original timing is historical author evidence, not a reproduced benchmark; no new speedup ratio is claimed.

Benefits

  1. Manual command hooks remain visible without false prompt-content verdicts.
  2. Escaped prompt-field Unicode is detected without scanning unrelated settings or commands.
  3. Invalid recognized content produces distinguishable nonzero incomplete coverage.
  4. Executed filesystem assertions prohibit transcript enumeration/reads, rather than merely expecting zero findings.

Validation

Published exact head: 0cac8097a9bf3e4a4a1cc8a01cc2f303787642ad.
Composed main: 73d6d0e280d9d898bb9ab84c7ec28f4348691199.
Last actual narrow composition CEO: technical ship_now at dc89f5fe, not a fresh review of this head. This pass repairs additive conflicts and stale lifecycle-test expectations; no new panel or guardian is claimed. Previous full/delta qualification remains historically attributed.

Observed exact-head qualification CI: 19 SUCCESS, 2 NEUTRAL, 1 SKIPPED. CI, spec conformance and merge gate passed; all six approved workflow executions completed successfully. Prior-head CI is not reused. CodeQL's aggregate and eligibility advisory are neutral; docs deployment is skipped. GitHub reports MERGEABLE/BLOCKED, not permission to merge.

The unchanged canonical completion schema validates. Strict deterministic owner verification returns verified: true, terminal_evidence_required: true, touched_owner_count: 8, with 31 actual exact-head passing consumer IDs. Incoming native/provenance owners remain in the full registry and mutation matrix.

The combined validate_composition.py selection and seven-gate verify_revision.sh produced:

3357 passed, 2 skipped, 2 subtests passed in 1256.77s (0:20:56)
All canonical lint and architecture checks passed on 0cac8097a9bf3e4a4a1cc8a01cc2f303787642ad

The unchanged combined selection covers the WHOLE exhaustive owner matrix, frozen inventory/registry, affected audit/native/policy/report suites, installed CLI/lifecycle, conformance and quality, plus every incoming changed test module. It retains HOME/APM_HOME aliases, ref freshness, native instruction/policy/cleanup, Cursor runtime/static values and provenance coverage from #2876, #3119 and #3116.

Merge 78b8b5a4 resolved two actual additive conflicts: hooks-integrations.json preserves audit and native projection owners; root_context_protection.py preserves audit filename discovery and protected-root/cleanup helpers. All 40 incoming and two prior helper function ASTs retain their parent definitions. The successor changes only three lifecycle test locations and related docs, not runtime/specification code. Seventy-seven incoming paths remain upstream-identical; fourteen contain faithful unions or the documented test-model correction. Both changelog entries and the single marketplace mutation case remain.

The intermediate merged-head run had seven stale expectations for redundant Claude roots. Corrected tests require exact installed native revisions, no redundant CLAUDE.md, and unchanged complete snapshots; mixed Claude/Hermes fixtures allow only Hermes AGENTS.md with the correct revision. Unmatched fallback, policy refusal and protected cleanup remain covered. The failed run stays archived under 78b8b5a4.

Additional evidence and limitations
  • Nine physical on-disk guard/owner-record removals caused assertion failures and passed after byte-exact restoration, including native eligibility removal against all three corrected lifecycle locations. Fifteen process-local runtime mutants and three actual-matrix detector removals were also rerun at this head. No mutants were committed.
  • Four ordinary/CI serializer bypasses and discovery/native/containment probes pass. User-scope runtime mutation fails a preliminary scanner assertion, not a mutation injected into the child CLI process.
  • Real CLI scenarios cover native-hook reinstall/update/user-scope transitions, byte/state preservation, pack/install/compile/audit, target widening/narrowing and mixed primitives.
  • Two existing conformance skips remain visible. An earlier install timeout and independent passing runs are retained without claiming a host-load cause.
  • Guardian round 2 reviewed 66ba9ea6: actual fold_and_ship, four scores 8/10. All 34 guardian artifacts remain byte-identical. Fifty previous composition artifacts remain identical to dc89f5fe; four changed faithfully with main: lockfile conformance tests, the owner matrix and generated CONFORMANCE.json/CONFORMANCE.md. Combined statements were regenerated and the suites rerun. The SHA256 crosswalk distinguishes unchanged identity from upstream deltas; no third guardian was run.
  • Earlier failed matrix qualifications remain recorded; the final full run supersedes, not erases, them.
  • Actual cumulative outer9 follows the user's continuous mergeability authorization and explicit conflict request. Copilot classification2, CI recovery1 and guardian2 are not reset. The immutable completion schema omits its unsupported optional iteration field; a hash-bound authorization sidecar records the true count. Accepted dc89f5fe and earlier packets remain immutable historical evidence, not rebound.
  • Docs-sync's classifier was tool-starved and is not claimed complete; actual docs and specification reviews are recorded separately.
  • The unchanged Mermaid diagram was rendered successfully with mmdc for this body. Local proof is macOS/editable-source CLI, not an all-platform shipping claim; actual CI platform execution is separately attributed.
  • No hooks were executed, no user configuration was automatically remediated, and no merge was performed.

Scenario Evidence

Scenario Principle Executed tests
Exclude unrelated transcripts Secure by default test_audit_primitive_contract.py::test_excluded_transcripts_are_not_walked_or_read
Command hooks visible/non-failing Governed by policy ::test_shared_settings_command_inventory_is_visible_and_non_failing, ::test_native_command_formats_are_inventory_not_prompt
Decoded fields without configuration writes Secure by default ::test_decoded_prompt_fields_fail_without_scanning_unrelated_settings
Unknown/unreadable content cannot claim success Governed by policy ::test_unknown_or_malformed_recognized_content_never_claims_success, ::test_unreadable_recognized_file_is_reported
Canonical patterns/global tracking Multi-harness support ::test_registered_prompt_patterns_are_discovered, ::test_user_scope_claims_keep_tracking_metadata
Non-prompt exclusion retains hash/ownership checks Secure by default ::test_non_prompt_scripts_keep_hash_and_ownership_checks
Symlink safety and state preservation Secure by default ::test_symlink_prompt_is_never_read, ::test_audit_and_failures_preserve_complete_workspace_snapshot
Reinstall/update/user-scope preservation Portability by manifest test_audit_unrecorded_unicode_lifecycle.py::test_native_hook_discovery_across_reinstall_update_and_user_scope
Precise ordinary/CI finding association Governed by policy tests/spec_conformance/test_policy_reqs.py::test_native_findings_identify_only_the_offending_field
Every owner guard has a meaningful mutation case Governed by policy Entire test_architecture_owner_rule_mutations.py, including registry equality and attributed violations
Global compile retains fresh native instructions without redundant Claude roots Multi-harness support test_generated_lifecycle_state_machine.py::test_generated_lifecycle_mandatory_replay, test_required_lifecycle_state_machine.py::test_required_global_audit_rule_matrix_for_external_roots, test_ownership_invariant_lifecycle.py::test_global_update_preserves_owned_external_skill_targets

Unqualified test filenames are under tests/integration/; abbreviated methods refer to test_audit_primitive_contract.py.

How to test

  • Run uv run --frozen --extra dev pytest -q tests/integration/test_audit_primitive_contract.py tests/integration/test_architecture_audit_discovery.py tests/integration/test_architecture_owner_rule_mutations.py; expect every case, including exhaustive equality, to pass.
  • Run APM_E2E_TESTS=1 APM_BINARY_PATH="$PWD/.venv/bin/apm" uv run --frozen --extra dev pytest -q tests/integration/test_audit_unrecorded_unicode_lifecycle.py; expect real CLI preservation cases to pass.
  • In an isolated Claude workspace, add a command-only hook to .claude/settings.json; apm audit --no-drift --format json should show non-applicable inventory without command execution.
  • Add "\u202e" to a documented nested prompt handler; expect exit1 and its exact pointer. Automatic --strip must refuse configuration changes.
  • Make the recognized configuration malformed; expect incomplete coverage and exit1, not a Unicode accusation or success report.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

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

Fix the target-scope resolution issue and clarify the lockfile union in both documentation pages.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR bounds audit scans to primitive deployment directories, avoiding unrelated session data while preserving lockfile-scoped coverage.

Changes:

  • Adds bounded governance partitioning.
  • Restricts deployed-tree walks and adds regression tests.
  • Documents the updated scan boundaries.
File summaries
File Summary
tests/unit/test_security_file_scanner.py Tests bounded scanning and transcript exclusion.
tests/unit/install/phases/test_lockfile_union.py Tests bounded governance partitioning and lockfile coverage.
src/apm_cli/security/file_scanner.py Restricts deployed-tree walks. Critical (2 votes): Uses project-scope resolution instead of supplied scope-resolved targets, potentially omitting global .copilot files.
src/apm_cli/install/manifest_reconcile.py Adds bounded governance partitioning.
docs/src/content/docs/reference/baseline-checks.md Documents scan scope. Nit (1 vote): Explicitly document the lockfile-scoped union.
docs/src/content/docs/enterprise/security.md Documents enterprise scan boundaries. Nit (1 vote): Explicitly document the lockfile-scoped union.
Review details

Suppressed comments (2)

docs/src/content/docs/enterprise/security.md:200

  • This new boundary description omits an important part of the implemented scope: the lockfile-scoped pass is still unioned with the bounded walk, so recorded paths outside primitive directories (for example .claude/settings.json and .claude/apm-hooks.json) are still checked for hidden Unicode. As written, readers could interpret the primitive-directory list as the complete content-integrity scope; please state the lockfile union explicitly.
A whole-project scan checks **every regular file under the deploy directories your targets govern** (primitive directories such as `.claude/skills/` and generated files, not the whole target root) for hidden Unicode, not only files recorded in `apm.lock.yaml`. Hash verification and positional `PACKAGE` scans remain lockfile-scoped because they need recorded ownership. Source content under `.apm/` is not added by the deploy-tree walk; install-time scanning owns that surface, while any `.apm/` path already recorded in the lockfile remains covered.

docs/src/content/docs/reference/baseline-checks.md:134

  • The reference page similarly describes the primitive-directory walk but does not say that the lockfile-scoped pass is unioned with it. That distinction matters for the documented examples such as a claimed .claude/settings.json: it remains hidden-Unicode scanned even though it is outside a primitive directory. Please make that union explicit rather than implying that only primitive-directory files are covered.
  1. Critical hidden Unicode (tag characters, bidi overrides, variation selectors 17-256, and similar steganographic markers), over **every file under the deploy directories the project's targets govern** (each target's primitive directories and generated files, such as `.claude/skills/` and `.claude/hooks/`, not the whole `.claude/` root). This signal needs no recorded baseline, so it is deliberately not limited to `deployed_files` -- otherwise a deployed file the lockfile omits would be exempt from scanning for as long as it stayed unrecorded. The rest of a target root (session transcripts, history, caches) is never deployed by APM and is not walked.
  • Files reviewed: 6/6 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 src/apm_cli/security/file_scanner.py Outdated
@Santoshkumarpuppala

Copy link
Copy Markdown

At f155f4e (these files unchanged at head 05b264d3) docs and code disagree on .claude/settings.json. baseline-checks.md:134 says "a claimed .claude/settings.json" is still scanned, and enterprise/security.md:200 says it "stays covered". But install doesn't record it (hook_integrator.py:1538, manifest_reconcile.py:205-207), and apm.lock.yaml has no settings.json entry but lists the .claude/rules files. The tree walk (file_scanner.py:157-158) now uses install_governance(scoped, bounded=True), which leaves it out, and the lockfile pass has nothing to add.

Run: apm install (claude target), then a user hook with U+202E in .claude/settings.json, then apm audit --ci. On main b914b1b (same walk result as the merge-base 6f3dd39) it exits 1 with "unicode: .claude/settings.json". At f155f4e it exits 0. Control: U+202E in .claude/skills/stray/SKILL.md exits 1 at both refs.

Drift still catches U+202E in APM's own hook entry and in .claude/apm-hooks.json (both run at head), so the loss I measured is the user-owned part of .claude/settings.json. install_governance also leaves out .gemini/settings.json and .cursor/hooks.json (read, not run).

If dropping user-owned content is the intent (as the PR does with transcripts), only the two doc sentences need to change. If not, adding merge_hook_config_paths(scoped) to the walk prefixes should restore it (read, not run). Happy to add the settings.json regression test for the code route.

@sergio-sisternes-epam

Sergio Sisternes (sergio-sisternes-epam) commented Sep 18, 2026 •

Copy link
Copy Markdown
Collaborator

PR triage recommendation

needs-issue

Recommendation only, not merge or scope approval. A responsible
human maintainer must approve. Labels, automated advice, and
silence are not approval.

Linked issue

#2961 (open; type/bug, area/cli, area/audit-policy, triage/recommended).
Not labelled status/accepted. Automated issue triage recommended accept;
that is not human scope approval.

Proposed classification

type/bug, area/cli, area/audit-policy

No existing human classification on this PR. Linked issue already carries
these labels; no conflicts.

Suggested next action

Wait for a maintainer to accept #2961 (status/accepted) before treating
this PR as in-scope work. Do not merge from this recommendation.

Thank you for contributing this pull request and for filing #2961 first.

APM starts with an issue, not an implementation
(https://github.com/microsoft/apm/blob/main/CONTRIBUTING.md).
#2961 is not labelled status/accepted yet, so a maintainer still needs to
review and accept the scope. This PR is labelled status/deferred until
that happens.

CODEOWNERS review requests already present: danielmeppiel,
sergio-sisternes-epam.

This is advisory classification only, not merge or scope approval.


Generated by autopilot-pr-triage-worker. This comment is AI-generated and may contain errors.

@sergio-sisternes-epam Sergio Sisternes (sergio-sisternes-epam) added triage/recommended Automated advice completed; not human scope approval. status/deferred Not invited for implementation now; no release commitment. type/bug Something does not work as documented. area/cli CLI command surface, flags, help text (cross-cutting). area/audit-policy apm-policy.yml schema, custom_checks, install-time enforcement. status/accepted Human scope approval; verify the issue's approval record and review contact before work. and removed status/deferred Not invited for implementation now; no release commitment. labels Sep 18, 2026
…rectories

install_governance() returns the whole root for dedicated targets, which is
right for ownership but wrong for a tree walk: at user scope ~/.claude and
~/.codex are mostly session transcripts APM never deploys. Scanning them cost
minutes of CPU per `apm audit --ci` and produced critical content-integrity
findings the audit cannot remediate.

Add a `bounded` keyword that partitions every root the way `.agents/` already
is, and use it from _scan_deployed_trees. Ownership and prune callers are
unchanged. Lockfile-claimed paths outside primitive directories stay covered
by the lockfile-scoped scan.

_scan_deployed_trees also walks the scope-resolved targets the caller
supplies rather than re-resolving at project scope, so a global audit's user
roots such as ~/.copilot are no longer skipped.

Fixes microsoft#2961

Claude-Session: https://claude.ai/code/session_01Nf7EcPHsefXRkPgvv3dAW3
@danielmeppiel

Copy link
Copy Markdown
Collaborator

lkshrk (@lkshrk), thanks for making the settings.json boundary an explicit maintainer decision. The approved scope is now recorded on #2961:

#2961 (comment)

The decision is neither "scan the entire settings file for hidden Unicode" nor "drop shared hook configuration from audit." Separate primitive discovery from the checks applicable to each surface.

Surface Discovery Hidden-Unicode prompt check
Known instruction, rule, agent, skill, prompt and context documents Include within the selected target and project/user scope, even without a lockfile entry Apply to recognized prompt-bearing content
Hook entries inside known hook files or shared native settings Discover through the documented hook containers, including untracked/manual entries Apply only to documented prompt-bearing fields, where the format has them
Hook command strings and executable scripts Recognize the hook definition; do not execute it or recursively chase commands Do not classify executable content as prompt text
Unrelated settings, transcripts, history and caches Exclude from automatic primitive discovery Do not scan as prompt surfaces

Please revise this PR against that contract before technical panel review:

  • Document and test an explicit per-target set of filenames/patterns, primitive kinds and structured fields. Reuse the existing target and hook registries as the authority, rather than adding a competing path list or scanning whole target roots.
  • Keep discovered untracked hooks visible as such. A missing applicable Unicode check must not make them disappear, but discovery does not imply unsafe content, APM ownership, or a verified hash.
  • Preserve the existing evidence requirements for ownership, hashes and drift. Apply the prompt/non-prompt distinction consistently to the lockfile and discovery paths; lockfile membership alone does not make a script a prompt.
  • Report unreadable or unknown recognized formats rather than implying successful coverage. Preserve path containment and symlink protections.
  • Add regressions for the tracked/untracked and prompt/non-prompt cases, shared settings with hook and unrelated fields, project/user target resolution, and excluded session data. Synchronize the maintained audit documentation and directly relevant apm-usage guidance with the actual boundary.

The current directory-bounding change is a useful foundation, but this approved contract is more than the earlier two-sentence documentation alternative. Please keep the work in this existing PR, link the approval in its scope section, and state any remaining gaps rather than marking the issue complete before the contract is met. This request does not authorize executing hooks, guessing runtime output, or introducing automatic edits/deletions of user configuration.

Review contact: Daniel Meppiel (@danielmeppiel). Scope acceptance does not certify the current implementation, test results, or merge readiness; technical review will follow the revision.

Apply the approved scope for microsoft#2961: inventory native hooks without executing commands, inspect documented prompt fields, fail incomplete coverage, and preserve configuration and ownership boundaries.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Pass user scope to canonical claim path validation and update the legacy scanner test seam.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Replace duplicated JSON examples with the native format authority and keep the integrator within the module-length guardrail.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@danielmeppiel

Copy link
Copy Markdown
Collaborator

APM Review Panel: needs_rework

PR #2962 unifies deployed-content discovery, but reproduced remediation, coverage and reporting errors need correction before its audit results can be trusted.

panel-mode=full; model=gpt-6-astra; seven specialists + apm-ceo

cc lkshrk (@lkshrk) Daniel Meppiel (@danielmeppiel) Sergio Sisternes (@sergio-sisternes-epam) -- a fresh advisory pass is ready for your review.

Reviewed local merged head cf0a1d98 (published revision 41f29e1 plus main d68f052). This is not a review of the unchanged original author head.

At cf0a1d9, the strongest signals are demonstrated safety failures: an external Cowork skill under HOME was rewritten by audit --strip because remediation authority was inferred from a display label; an unreadable parent containing a critical prompt produced exit 0, passed=true and no coverage diagnostic. These contradict the accepted external-root protection and incomplete-coverage contracts. CI renderer probes also show inventory disappearing for passing checks and becoming security errors when an unrelated finding fails the check. These are correctness defects, not optional presentation improvements.

The architecture and bounded-discovery assessments are positive, but do not negate those edge cases. Current-head lint guards passed. The orchestrator's 402-case suite recorded 401 passes and one install timeout under concurrent load; the test panel independently passed 205 targeted tests and four actual CLI lifecycle tests. Those results support the exercised paths, not the untested failure paths. The supplied findings do not attach failed automated-test evidence to the reproduced defects, so those reproductions must not be described as failing regression tests. The two explicit missing-test findings deserve priority over ordinary recommended refinements: mixed-content strip refusal must preserve every file and deployment record, and incomplete coverage must not produce successful-looking SARIF.

The remaining findings fit the accepted scope: discovery must use Cowork's actual deployed root; human-readable pointers must be escaped without changing machine pointers; decoded prompt coordinates must not masquerade as physical source locations; protected-field remediation must be actionable; documentation must disclose Copilot App filesystem coverage limits and complete the exit-code reference. Pre-read deduplication, per-claim directory-status reuse and the public result-type naming nit can be folded without expanding the feature. The parent contract calls for all in-scope recommendations to be folded and then a fresh delta panel; the curated ordering below is sequencing, not permission to omit lower-priority findings.

Dissent. CLI Logging classifies misleading decoded coordinates as blocking, while DevX classifies the same behavior as recommended. I side with CLI Logging on severity because a security diagnostic must not direct users to a fictitious source location; this remains advisory emphasis, not a merge gate.

Aligned with: External-root strip protection must come from explicit root eligibility, and untrusted field labels must be safe to display. Unreadable recognized content must report incomplete coverage, while inventory and actual policy violations must remain distinct in every CI format. Discovery must match Cowork's installer-selected root, and documentation must distinguish filesystem coverage from Copilot App's SQLite-backed workflows. Trustworthy locations, actionable refusal guidance, accurate exit-code documentation and avoiding repeated filesystem work make audit behavior predictable.

Panel summary

Persona B R N Takeaway

| python-architect | 0 | 0 | 1 | Canonical discovery and applicability routing are coherent; one public result-type naming nit. |

| cli-logging-expert | 3 | 0 | 0 | CI inventory, printable field labels, and decoded coordinates need correction. |

| supply-chain-security-expert | 2 | 1 | 0 | Isolated probes found external-root rewriting, false clean inaccessible roots, and missed Cowork skills. |

| devx-ux-expert | 0 | 2 | 0 | 58 contract tests passed; improve protected-field remediation and coordinate semantics. |

| doc-writer | 0 | 2 | 0 | Clarify App coverage and exit1 reference; 58 tests passed, 48 links resolve. |

| test-coverage-expert | 0 | 2 | 0 | 205 targeted and 4 installed lifecycle tests passed; add mixed-strip and SARIF failure traps. |

| performance-expert | 0 | 2 | 0 | Bounded discovery and linear merging are sound; remove redundant probes and content reads. |

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

Top 5 follow-ups

  1. [supply-chain-security-expert] Derive strip authority from explicit surface-root eligibility, never its display label. -- The isolated HOME reproduction rewrote an external Cowork skill that the accepted scope requires remediation to protect.

  2. [supply-chain-security-expert] Report inaccessible recognized roots as incomplete coverage rather than silently excluding them. -- A chmod(0) parent containing a critical prompt produced exit 0, passed=true and zero scanned files without a diagnostic; access failures must be distinguished from deliberate symlink exclusion.

  3. [cli-logging-expert] Separate hook inventory from CI violation details across passing and failing renderers. -- Current-head probes show passing text/SARIF hiding inventory and failed checks rendering not-applicable command inventory as security errors.

  4. [cli-logging-expert] Escape structured field labels at human-readable rendering boundaries while preserving exact machine pointers. -- Raw U+202E survives in text and Markdown descriptions; untrusted pointer content must not control the visual interpretation of security diagnostics.

  5. [cli-logging-expert] Use structured pointers as locations and explicitly qualify decoded-value coordinates in JSON, text and Markdown. -- CLI Logging and DevX both found decoded string offsets presented as source-file coordinates; SARIF already avoids the fictitious physical region.

Recommendation

Fold all in-scope panel recommendations, starting with external-root strip authority, unreadable-root false-clean behavior and CI inventory classification. Preserve the passing validation evidence, add the missing regression traps, rerun current-head validation and obtain a fresh delta panel after the fixes. The reproduced safety errors do not support ship_now; this is an advisory recommendation only, with no label, approval or merge gate.


Full per-persona findings

python-architect

  • [nit] Give the result returned by scan_project_result a public type name. at src/apm_cli/security/file_scanner.py
    The typed contract is consumed across modules; a public FileScanResult name clarifies it.

cli-logging-expert

  • [blocking] Separate hook inventory from CI violation details at src/apm_cli/policy/ci_checks.py
    CheckResult.details is hidden for passing text/SARIF checks and emitted as error results for failed checks. A not-applicable command inventory entry becomes a security error when another violation fails content-integrity. Read-only current-head renderer probes reproduced both cases.

  • [blocking] Sanitize structured field labels at human-readable rendering boundaries at src/apm_cli/security/file_scanner.py
    User-controlled JSON pointers are interpolated into descriptions. In-memory rendering preserved raw U+202E in text and Markdown. Preserve exact machine pointers, escape printable descriptions and Rich/Markdown display values.

  • [blocking] Distinguish decoded prompt coordinates from physical file coordinates at src/apm_cli/security/audit_report.py
    JSON, text and Markdown retain unqualified decoded string line/column values beside settings.json. SARIF already omits fictitious regions. Use the pointer as location and label any decoded-value coordinates explicitly.

supply-chain-security-expert

  • [blocking] Derive strip authority from the surface root, not its display label at src/apm_cli/security/file_scanner.py
    An external managed Cowork root under isolated HOME receives a HOME-relative label, so the target-prefix strip guard fails. A manual skill named skills was rewritten by audit --strip in an isolated reproduction. Carry explicit external-root eligibility.

  • [blocking] Report inaccessible recognized roots as incomplete instead of silently skipping them at src/apm_cli/security/primitive_discovery.py
    has_symlink_component returns true on OSError and containment suppresses OSError; discovery silently returns. Real chmod(0) parent fixture with a critical prompt produced exit0, passed=true, zero scanned and no coverage diagnostic. Distinguish unreadable access from deliberate symlink exclusion.

  • [recommended] Reuse the deployed skills-root contract for dynamic Cowork targets at src/apm_cli/security/primitive_discovery.py
    Cowork resolver already returns Documents/Cowork/skills; SkillIntegrator uses it directly. Discovery adds mapping.subdir again and searches skills/skills. Existing generated-pattern test seeds discovery's own incorrect path. Reuse canonical destination and test actual installer-selected root.

devx-ux-expert

  • [recommended] Make structured-field remediation guidance actionable and consistent at src/apm_cli/commands/audit.py
    Warning-only TOML prompt findings recommend --strip, which refuses generically without listing filename/pointer. Use protected-aware guidance for warnings and list every blocked field before refusal.

  • [recommended] Do not present decoded prompt offsets as source-file coordinates at src/apm_cli/security/audit_report.py
    A TOML prompt warning renders Location1:6 and JSON line1,column6 even though source column6 is part of the key. Match SARIF's explicit pointer/coordinate distinction.

doc-writer

  • [recommended] Do not imply filesystem discovery scans deployed Copilot App workflows at docs/src/content/docs/reference/cli/audit.md
    Actual App workflows are SQLite rows; the synthetic workflows/*.prompt.md mapping does not represent deployed files and no URI decoder scans them. Separate App from Cowork and disclose filesystem coverage limits.

  • [recommended] Add incomplete coverage and refused remediation to the exit-code reference at docs/src/content/docs/reference/cli/audit.md
    Dedicated exit1 table omits incomplete coverage and refused strip, despite the new behavior section. This can misclassify access errors as malicious content.

test-coverage-expert

  • [recommended] Extend lifecycle coverage to prove strip refusal preserves eligible files too at tests/integration/test_audit_unrecorded_unicode_lifecycle.py
    Existing fixtures test protected settings and successful regular stripping separately, not a dirty SKILL.md plus protected settings in one refusal. Extend current lifecycle, no new expensive module.

    Proof (missing): tests/integration/test_audit_unrecorded_unicode_lifecycle.py -- Refused mixed-content strip leaves all files and deployment records unchanged.

  • [recommended] Exercise incomplete coverage through the SARIF consumer contract at tests/integration/test_audit_primitive_contract.py
    Current malformed-format tests cover JSON/CI JSON; SARIF only tests Unicode. Assert exit1, executionSuccessful=false and distinct incomplete-coverage error at the settings file.

    Proof (missing): tests/integration/test_audit_primitive_contract.py -- Malformed recognized content cannot produce successful-looking SARIF.

performance-expert

  • [recommended] Compute directory status once per claim, not per surface at src/apm_cli/security/file_scanner.py
    Predicate calls is_dir before lexical relation for every nonmatching patterned surface. AST probe measured100000 calls for1000 claims and100 surfaces. Memoize one status per claim and put lexical guards first.

  • [recommended] Deduplicate scan work before reading and parsing overlapping claims and discovery at src/apm_cli/security/file_scanner.py
    Tracked recognized files are read in claims then discovery; overlapping directory/explicit claims repeat work. Linear result merging only discards duplicate evidence after I/O. Use invocation-local cache keyed by path and interpretation, preserving tracked metadata and safety.

This panel is advisory. The maintainer decides whether to ship.


Generated by autopilot-pr-review-worker. This comment is AI-generated and may contain errors.

Separate CI inventory from violations, preserve denied-access coverage, bind external protection to surfaces, and share the deployed skills destination. Qualify decoded offsets, harden human rendering, deduplicate scans and extend regression traps.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@danielmeppiel

Copy link
Copy Markdown
Collaborator

APM Review Panel: needs_rework

PR #2962 broadens audit coverage without executing hooks; rendered Markdown pointer safety still needs correction before the change is ready to ship.

panel-mode=delta; model=gpt-6-astra; seven specialists + apm-ceo

cc lkshrk (@lkshrk) Daniel Meppiel (@danielmeppiel) Sergio Sisternes (@sergio-sisternes-epam) -- a fresh advisory pass is ready for your review.

At df7e236, the actual seven-persona delta panel leaves three findings: one blocking-severity Markdown rendering issue and two recommended corrections. The logging probe reproduced HTML entity decoding that introduces directional and control characters into both report tables. This is manual evidence, not a failed automated test; it nevertheless identifies a concrete unresolved reporting-safety defect and a missing regression guard at the rendered-output boundary. Passing source-level sanitization checks do not establish rendered-output safety.

The architect's shared-skills finding is narrower than a general performance regression: historical catalog attribution and selected-profile discovery produce different cache identities for the same content interpretation, causing a duplicate read and retaining misleading target metadata. Correct interpretation caching and attribution together. The documentation finding also merits an in-scope fold: the exit-code reference must distinguish default advisory drift from failures promoted by security.audit.fail_on_drift, including failed checks versus advisory cache-miss skips. These are targeted completions of the existing work, not reasons to expand its scope.

Validation is substantial but bounded: the supplied exact-head record reports 983 focused tests, 4 installed-CLI tests, 63 quality tests, 11 killed mutants, and lint/architecture checks; the additional owner-lifecycle run reports 4 passed in 21.61 seconds. The security and coverage panelists separately report 80 and 158 passing bounded tests; these counts should not be summed as unique coverage. None supplies an automated rendered-entity regression assertion. Six exact-head fork workflows were approved under explicit user authorization, but actual CI remains pending; approval is not a green result.

Dissent. There is no explicit severity disagreement to arbitrate. The performance persona's clean delta assessment does not negate the architect's specific shared-root identity case, and the security persona's clean return does not establish safety at the Markdown rendering boundary.

Aligned with: Preserve non-executing hook discovery and protected remediation boundaries; prevent rendered report labels from reintroducing hidden controls. Document policy-promoted drift failures accurately so consumers can interpret exit status reliably. Shared skill roots require correct selected-target attribution independently of content interpretation caching. Complete the three bounded corrections without expanding audit into hook execution or user-configuration rewriting.

Panel summary

Persona B R N Takeaway

| python-architect | 0 | 1 | 0 | Public FileScanResult and skills routing are coherent; the new cache still misses shared-root claims versus discovery. Packet validation is observed; CI remains pending. |

| cli-logging-expert | 1 | 0 | 0 | Inventory separation, decoded coordinates, and protected guidance pass bounded probes. Markdown entity decoding still bypasses pointer sanitization. CI remains pending. |

| supply-chain-security-expert | 0 | 0 | 0 | No new substantive security findings. All 80 isolated contract tests passed, including real denied-parent coverage; mixed-strip and Cowork-inside-HOME probes also passed. |

| devx-ux-expert | 0 | 0 | 0 | Static review at df7e236 found no remaining concerns in protected warning guidance, strip refusal, or decoded coordinates. No tests executed. |

| test-coverage-expert | 0 | 0 | 0 | 158 bounded tests passed. Prior mixed-strip and SARIF gaps are addressed; inspected delta regression traps are substantive. No new findings. |

| performance-expert | 0 | 0 | 0 | Both prior performance findings are addressed in cf0a1d9..df7e236. No substantive remaining findings or material complexity regression in the owned delta. |

| doc-writer | 0 | 1 | 0 | App, Cowork, coverage inventory and decoded offsets match source; one coupled exit-code promise remains stale. Static review only; docs-sync success is not established. |

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

Top 3 follow-ups

  1. [cli-logging-expert] Escape HTML entities at the human-readable Markdown boundary and test rendered text in both tables. -- Named entities such as ‏, , and bypass raw-Unicode sanitization after rendering. Preserve exact machine pointers in JSON and SARIF; add automated assertions against parsed Markdown output.

  2. [python-architect] Separate interpretation-cache identity from attribution and prefer selected profiles over historical catalog fallbacks. -- The shared .agents/skills path can be read twice and retain Hermes attribution for a Copilot project. Add a regression proving one content read across claims/discovery and correct tracked target metadata.

  3. [doc-writer] Correct the drift behavior paragraph and exit-code reference for security.audit.fail_on_drift. -- The current unconditional advisory wording misstates legitimate policy failures. Distinguish failed or unavailable drift checks promoted to exit 1 from advisory cache-miss skips.

Recommendation

Recommend folding all three remaining findings in scope, starting with rendered Markdown safety, then verifying the revised head and observing actual CI results. Preserve the prohibition on hook execution and user-configuration rewriting. Continue within the existing outer-loop budget of four, currently at iteration two. This is advisory guidance, not a label, merge gate, or authorization expansion.


Full per-persona findings

python-architect

  • [recommended] Separate content-interpretation cache identity from target attribution at src/apm_cli/security/file_scanner.py:195
    The new cache key retains every PrimitiveSurface field, including target. For a Copilot project, _scan_claimed_files appends unselected catalog profiles, and primitive_surfaces uses last-wins deduplication by path/pattern: the shared .agents/skills surface is attributed to Hermes. Automatic discovery using only the selected Copilot profile produces the same path and skill_standard interpretation with target='copilot'. These keys differ, so a tracked SKILL.md is read and scanned twice despite the shared invocation cache. First-scope merging also retains the catalog-derived Hermes attribution. The metadata mismatch predates this delta, but prevents the new cache from fulfilling the prior deduplication recommendation on the normal shared-skills path.

cli-logging-expert

  • [blocking] Escape HTML entities in human-readable Markdown pointer cells at src/apm_cli/security/audit_report.py:38
    _markdown_cell sanitizes raw Unicode but leaves ampersands untouched. A JSON event name containing ‏ therefore reintroduces U+200F, newline, and tab when Markdown renders the pointer. An in-memory probe against findings_to_markdown reproduced these controls in both the findings Location cell and the primitive coverage cell. This leaves the prior human-readable field-label safety finding unresolved even though the Markdown source itself contains only printable ASCII.

supply-chain-security-expert

No findings.

devx-ux-expert

No findings.

test-coverage-expert

No findings.

performance-expert

No findings.

doc-writer

  • [recommended] Document policy-promoted drift failures in the revised exit-code reference. at docs/src/content/docs/reference/cli/audit.md:295
    The prior App-coverage and incomplete/refused-remediation findings are addressed. However, the surrounding exit-0 row still unconditionally classifies bare-audit drift as advisory, and the revised exit-1 row omits policy-promoted drift failures. At df7e236, src/apm_cli/commands/audit.py:1105-1115 promotes an otherwise clean run to exit 1 when the drift check fails and security.audit.fail_on_drift is enabled. This includes a drift check that could not run, while an advisory cache-miss skip remains non-failing. The unconditional promise also appears at audit.md:252-255. This retained mismatch can cause consumers to misclassify a legitimate policy failure.

This panel is advisory. The maintainer decides whether to ship.


Generated by autopilot-pr-review-worker. This comment is AI-generated and may contain errors.

…contract

Preserve canonical emitted command layouts, safe Markdown text, shared-skill attribution, and valid native ownership fixtures. Propose optional deployed-prompt audit requirements without claiming completed specification adoption.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@danielmeppiel

Copy link
Copy Markdown
Collaborator

APM Spec Guardian: next_brief

Scope: editorial-patch; diff = +66/-4 lines across 1 spec file. Shocked-meter avg: 7.5/10.

Round 1 for PR #2962 at local revision 58dafc0 has a shocked-meter average of 7.5 and one new blocking finding. The blocker rule therefore requires next_brief regardless of the three ship_with_followups assessments. The highest-priority correction is the ordinary user-scope remediation escape: deployment traversal containment must not confer mutation authority over user configuration. Apply the bounded folds, obtain behavioral evidence for protected user-scope refusal and CI field attribution, and return the corrected artifact and evidence for round 2 of the two-round editorial-patch budget. A prose clarification alone cannot close the user-scope finding. This is an advisory recommendation, not an instruction to change labels or merge state.

The remaining findings are direct correctness or declaration-boundary fixes for the newly optional audit contract, not reasons to expand the amendment into a registry, schema or broader security redesign. No finding needs architectural deferral, no reserved-slot work is invented, and none of the supplied proposed fixes requires rejection for forbidden affiliation language. Existing approval covers bounded folds only; specification adoption still requires the pending human approvals and the 14-day Section 9.3 public comment window. The reported 122-test result was not rerun in this synthesis and does not establish green CI. One panel round remains; persistent new blocking findings in round 2 require maintainer escalation rather than another automatic round.

Convergence

| Panel | Stance | Shocked | New B | New R | New N |

|---|---|---:|---:|---:|---:|

| spec-swagger-editor | ship_with_followups | 8/10 | 0 | 2 | 2 |

| spec-oci-editor | needs_revision | 6/10 | 1 | 1 | 1 |

| spec-pkgmgr-editor | ship_with_followups | 8/10 | 0 | 2 | 2 |

| spec-tag-architect | ship_with_followups | 8/10 | 0 | 2 | 2 |

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

Convergent themes (flagged by 2+ panels)

  • T1 -- CI reporting must preserve the association between a native prompt finding and its field, independently of coverage inventory. (supporting: sw-rec-r1-1, oci-rec-r1-1, pkg-rec-r1-1)

  • T2 -- The optional capability needs an explicit, substantive declaration boundary compatible with independently claimed Governance implementations. (supporting: sw-rec-r1-2, pkg-rec-r1-2, tag-rec-r1-1)

  • T3 -- The requirements manifest must retain dependency-policy notes under req-pl-018 rather than attaching them to remediation safety. (supporting: sw-nit-r1-1, oci-nit-r1-1, pkg-nit-r1-1, tag-nit-r1-1)

  • T4 -- The proposed amendment and its pending-adoption notice must remain inside the revision-history table. (supporting: sw-nit-r1-2, pkg-nit-r1-2, tag-nit-r1-2)

Fold now (9 items)

  1. [F1 / standalone] Section 6.8.1, req-pl-020 -- Address oci-blk-r1-1 by explicitly including user-scoped prompt content in the protected-content set for automatic remediation. State that using the user's home directory as the deployment traversal root does not remove that protection. Preserve refusal before writing any selected content and the existing separation between discovery and mutation authority. Do not redefine the separately documented explicit --file operation.
    Success criterion: req-pl-020 explicitly protects user-scoped content even when the home directory is the traversal root; the pre-write refusal requirement still applies to a selection containing protected content. Its requirement ID and conditional capability trigger remain unchanged.

  2. [F2 / standalone] req-pl-020 implementation evidence: primitive discovery and automatic remediation eligibility -- Address the implementation portion of oci-blk-r1-1 by retaining installation scope independently of path containment when constructing remediation eligibility. Automatically discovered user-scope content must remain non-strippable and enter the existing protected-content refusal path even when it is contained within the selected deployment root. Keep the change confined to this eligibility decision and preserve explicit --file behavior.
    Success criterion: An ordinary static Copilot user profile classifies ~/.copilot/copilot-instructions.md as protected and non-strippable without shared settings or an external-root override. Automatic remediation refuses before writes; existing explicit --file behavior remains covered and unchanged.

  3. [F3 / standalone] tests/spec_conformance/test_policy_reqs.py, req-pl-020 behavioral coverage -- Add the missing regression from oci-blk-r1-1 using an isolated home directory and an ordinary static user-scope deployment profile. Request automatic remediation of prompt content containing a removable finding. Do not rely on shared native settings or an external-root override to obtain protection. Retain the existing mixed protected/unprotected selection regression.
    Success criterion: The new requirement-bound test fails against the reviewed eligibility behavior and passes after F2, asserting nonzero remediation status, identification of the protected user-scope location, and byte-for-byte preservation. The existing mixed-selection test continues to prove refusal before any selected content is written.

  4. [F4 / T1] req-pl-019(d) implementation evidence: CI content-integrity reporting -- Address sw-rec-r1-1, oci-rec-r1-1 and pkg-rec-r1-1 without weakening req-pl-019(d). Preserve the affected file and native field pointer for each CI content finding rather than reducing findings to filenames. Carry that association through CI text, JSON and SARIF independently of coverage inventory. For SARIF content findings, use the affected prompt artifact rather than the generic apm.lock.yaml mapping. Keep decoded coordinates as decoded metadata, not physical source regions.
    Success criterion: For a native settings file with two checked prompt fields and a finding only in the second, CI text and JSON identify the second field as the finding location. CI SARIF identifies the settings artifact and offending pointer, does not attribute the finding to apm.lock.yaml, and does not serialize decoded offsets as physical regions.

  5. [F5 / T1] tests/spec_conformance/test_policy_reqs.py, req-pl-019 reporting coverage -- Add a requirement-bound native-finding attribution case with two prompt fields, only one containing a critical finding. Assert the finding-to-field association in default and CI reporting, including CI JSON and SARIF. Keep the existing malformed-content and incomplete-coverage cases; inventory entries alone must not satisfy the new assertions.
    Success criterion: Tests assert the exact offending pointer and actual affected artifact separately from checked-field inventory. Moving the sole finding between the two fields changes the reported finding location. CI reporting regressions described by the three supporting findings cause test failures.

  6. [F6 / T2] Section 6.8.1, introductory disclosure and req-pl-019 -- Combine sw-rec-r1-2, pkg-rec-r1-2 and tag-rec-r1-1 in the existing requirement. Put an explicit disclosure obligation under req-pl-019's conditional MUST: identify supported targets, project or user scopes, recognized primitive locations and native formats, prompt checks, and intentional exclusions in the conformance statement. Require a non-empty declared target/scope profile containing at least one prompt-bearing location or field and one specified content check. Distinguish this claim-level minimum from a workspace that legitimately contains no matching content. In clause (a), identify the Consumer deployment implementation whose location and format definitions are used; explicitly permit consuming those definitions from another implementation without implementing deployment or claiming the Consumer class. Keep concrete paths, native keys, algorithms and serialization implementation-defined.
    Success criterion: The existing req-pl-019 explicitly binds disclosure, non-empty profile and content-check requirements to capability claimants. The text permits a standalone Governance implementation using an identified Consumer's definitions and a complete empty result for a workspace with no matching content. Non-claimants acquire no new feature obligation, and no requirement ID or schema contract is added.

  7. [F7 / standalone] tests/spec_conformance/gen_statement.py, DEPLOYED_PROMPT_AUDIT disclosure -- Address tag-rec-r1-2 by replacing the claim that ContentScanner checks prompt-injection markers with an accurate description of hidden or suspicious Unicode code points that may conceal instructions. Explicitly exclude semantic prompt-injection detection and retain the warning that a clean result is not proof of safe prompt text. Regenerate CONFORMANCE.md and CONFORMANCE.json from the corrected source rather than editing their disclosure independently.
    Success criterion: The generator and both generated conformance statements consistently describe Unicode-based checks, explicitly exclude semantic prompt-injection detection, and retain the safety disclaimer. Their capability disclosure no longer claims a separate prompt-injection marker detector.

  8. [F8 / T3] docs/public/specs/manifests/openapm-v0.1.requirements.yml, req-pl-018 through req-pl-020 -- Move the existing repository-case and policy-operand notes property back beneath req-pl-018's conformance_class property, before the req-pl-019 entry. Preserve the notes text and all three requirement entries.
    Success criterion: Parsed YAML associates the original dependency-policy notes with req-pl-018 and not req-pl-020. Requirement IDs, conformance classes and entry counts remain unchanged.

  9. [F9 / T4] Appendix D, proposed 0.1.42 revision-history row -- Remove the blank line immediately before the proposed 0.1.42 row so it is contiguous with the existing revision-history table. Preserve the proposed designation, compatibility explanation, statement counts and pending Section 9.3 notice.
    Success criterion: Markdown parsing renders 0.1.41 and proposed 0.1.42 as rows in the same table. The proposed row still states that approvals and the public comment period remain pending and that the proposal is not evidence of adoption.

Linter notes (1 check(s) failed)

  • [11] Non-spec Python files changed; general review also required. Exact 58dafc0 canonical lint independently passed.

Linter handoff: Run the full Wave 5 checklist after the folds; no post-fold pass is asserted here. Verify printable ASCII and absence of forbidden artifact tokens in all replacement text. Keep req-pl-019 and req-pl-020 conditional and preserve all existing IDs. Reconcile 125 unique requirement anchors, both Governance enumerations, Appendix C, the requirements manifest and generated conformance records; retain agreement at 125 statements, 120 MUST and 5 SHOULD across the three count sites. Validate unchanged schemas and local references. Parse YAML to check notes ownership, not merely ID counts, and parse Markdown to confirm the proposed history row remains inside its table. Regenerate and compare both conformance statements after correcting the scanner disclosure. Verify pending-adoption language, the required human approvals and the 14-day Section 9.3 window remain distinct from permission to apply bounded folds. Mechanical lint cannot establish behavioral closure: separately require passing evidence for static user-root protection without shared settings or external-root overrides, refusal before any selected write, preserved explicit --file behavior, and exact native finding pointers plus correct artifacts in CI JSON and SARIF. Ensure decoded offsets do not become physical source regions. Carry the actual round-2 panel findings into the next decision; do not infer blocker closure from wording changes, average score, test collection or previously reported results.


Full per-panel findings

spec-swagger-editor -- shocked_meter 8/10, confidence high

Summary: The proposed capability at 58dafc0 has sound conditionality, stable requirement enumeration and appropriately qualified adoption language. The principal followups are preserving native finding attribution in CI reports and making scope disclosure explicitly normative, plus two surgical editorial corrections. This advisory assessment does not replace the pending human approvals or public comment period; reported test runs were not rerun during this read-only review.

New recommended findings (2)

  • [sw-rec-r1-1] req-pl-019(d); src/apm_cli/policy/ci_checks.py::_check_content_integrity; tests/spec_conformance/test_policy_reqs.py -- The new requirement says reports identify native prompt findings by field location, but CI reporting discards the finding pointer when reducing findings to critical_files. Its violation detail is only 'unicode: .claude/settings.json'. primitive_coverage lists all checked fields without identifying which produced a finding. An in-memory check confirmed that moving the sole critical finding between two prompt fields produces identical CheckResult output. The new requirement-bound tests verify incomplete exit behavior but do not assert CI native-finding attribution. Detection and failure remain intact, so this is a reporting/conformance improvement rather than a broken safety guarantee.
    Recommended fix: Retain the native field pointer with each CI content finding and expose that association in CI text, JSON and SARIF, separately from coverage inventory. Do not present decoded offsets as physical file coordinates. Add a req-pl-019 conformance case with two checked native prompt fields, only one containing a finding, and assert that CI output identifies the offending field rather than merely the file or both inventory entries.

  • [sw-rec-r1-2] Section 6.8.1 introductory paragraph and req-pl-019 -- The capability's interoperability boundary depends on a declared scope, but the sentence requiring disclosure is only descriptive: 'A claim of this capability identifies the supported targets ...'. Section 11.2 requires listing optional features, not specifically publishing this capability's targets, locations, formats and checks. Consequently the load-bearing disclosure obligation lacks an explicit normative keyword and requirement binding, even though the subsequent MUST clauses depend on that declaration.
    Recommended fix: Move the disclosure obligation into req-pl-019, for example: 'An implementation claiming this capability MUST identify its supported targets, project or user scopes, recognized primitive locations and native formats, and prompt checks in its conformance statement.' Keep concrete paths, native keys, algorithms and serialization implementation-defined. Retain the existing requirement ID and conditional capability trigger; no new blanket Governance feature obligation is needed.

New nit findings (2)

  • [sw-nit-r1-1] docs/public/specs/manifests/openapm-v0.1.requirements.yml: req-pl-018 through req-pl-020 -- Inserting req-pl-019 and req-pl-020 before the existing notes line reattaches req-pl-018's repository-case and policy-operand notes to req-pl-020. Parsed YAML now describes prompt-audit remediation using unrelated allow/deny matching semantics.
    Recommended fix: Move the existing notes line immediately beneath req-pl-018's conformance_class line, before the req-pl-019 entry.

  • [sw-nit-r1-2] Appendix D, proposed 0.1.42 revision entry -- A blank line between the 0.1.41 row and the new 0.1.42 row terminates the Markdown table. The proposed amendment renders as a pipe-delimited paragraph rather than a revision-history table row; Markdown parsing confirms this.
    Recommended fix: Remove the blank line immediately before the '| 0.1.42 (proposed)' row.

Preserved strengths confirmed

  • Both new requirements explicitly apply only to implementations claiming deployed-prompt-audit; implementations declining the capability acquire no blanket feature obligation.

  • The amendment preserves separation between discovery, prompt applicability, incomplete coverage, content findings, ownership, executable trust and drift policy. Command-only content is not automatically classified as unsafe.

  • The no-execution, no-symlink-following and protected-remediation boundaries are explicit. The requirement-bound remediation test checks preservation of shared configuration and otherwise eligible selected content on refusal; broader integration tests additionally observe excluded reads and traversal.

  • There are 125 unique actual requirement anchors, with only req-pl-019 and req-pl-020 added and no existing IDs removed. Both new IDs appear once in each Governance enumeration. Appendix C, the requirements manifest and generated statement agree on the new entries; the three count sites agree on 125 statements, 120 MUST and 5 SHOULD.

  • All six JSON schemas under docs/public/specs/schemas parse and pass Draft202012Validator.check_schema; their local references resolve. This amendment changes no schema contract. New-section local cross-references resolve.

  • The proposed revision and generated capability disclosure explicitly retain pending Section 9.3 adoption. The compatibility account distinguishes optional implementation uptake from behavior-neutral errata and identifies newly failing audit outcomes without claiming completed approval or green CI.

spec-oci-editor -- shocked_meter 6/10, confidence high

Summary: At 58dafc0, the proposed capability has sound conditionality and separates discovery, coverage and trust appropriately. However, ordinary user-scope documents can escape the promised remediation protection, and CI reports lose native finding-to-field attribution. These are bounded findings against the new capability and its evidence, not unrelated baseline debt.

New blocking findings (1)

  • [oci-blk-r1-1] req-pl-020; Section 6.8.1; CONFORMANCE optional deployed-prompt-audit disclosure -- The claimed user-scope implementation does not consistently enforce the protected-remediation guarantee. scan_project_result replaces the metadata workspace with the home deployment root before constructing surfaces (file_scanner.py:554-576). For ordinary user-scope profiles, primitive_surfaces then considers paths under that home root non-external (primitive_discovery.py:76,103,119). An in-memory probe using the actual Copilot user profile classified ~/.copilot/copilot-instructions.md as strippable=True with no protected_files entry. The strip path consequently permits remediation using the home deployment root (commands/audit.py:1005-1032). This turns a traversal anchor into mutation authority over user configuration, contrary to the bounded amendment's protection of user roots and the capability claim under req-pl-020. The added conformance test covers shared project settings, not this ordinary user-scope document case.
    Recommended fix: Clarify req-pl-020 with: 'User-scoped prompt content remains protected even when the user's home directory is used as the deployment traversal root.' Carry installation scope independently of containment into remediation eligibility, making all automatically discovered or claimed user-scope content non-strippable. Add a req-pl-020 regression using a static user-scope target such as Copilot, without shared settings or an external-root override, asserting nonzero remediation status and unchanged bytes. Preserve the separately documented explicit --file operation.

New recommended findings (1)

  • [oci-rec-r1-1] req-pl-019(d); tests/spec_conformance/test_policy_reqs.py; generated conformance evidence -- The field-location contract is demonstrated for default output but not preserved in CI reports. _check_content_integrity reduces native prompt findings to 'unicode: ' (ci_checks.py:625-626), while primitive_coverage records every inspected field as checked without identifying which field produced a finding. With two prompt handlers and only the second containing a critical character, an in-memory probe produced indistinguishable coverage entries and no finding-to-field association. CI SARIF additionally attaches that result to apm.lock.yaml through the generic artifact map. The new req-pl-019 test exercises CI incomplete-coverage failure, but does not assert CI field attribution for actual prompt findings.
    Recommended fix: Retain req-pl-019(d)'s field-location guarantee across default and CI reports. Preserve the affected file and prompt pointer on each CI content finding; carry decoded coordinates only as decoded metadata, never as physical source regions. Add a req-pl-019 test with two native prompt fields, only one containing a finding, asserting the exact offending pointer and correct artifact in CI JSON and SARIF.

New nit findings (1)

  • [oci-nit-r1-1] docs/public/specs/manifests/openapm-v0.1.requirements.yml:299-311 -- Insertion of req-pl-019 and req-pl-020 displaced the existing dependency-policy case-normalization notes from req-pl-018 onto req-pl-020. The informative requirement manifest now describes the remediation-safety requirement using unrelated dependency-matching semantics.
    Recommended fix: Move the existing notes line back under req-pl-018, before the req-pl-019 entry.

Preserved strengths confirmed

  • The capability is explicitly optional and conditional on a conformance claim. Non-claiming implementations acquire no new mandatory feature, and the amendment does not alter schema identities, lockfile versions, hash verification, executable trust or drift policy.

  • Appendix D and the generated capability disclosure explicitly preserve pending Section 9.3 approvals and the public comment period; bounded amendment authorization is not presented as adoption.

  • Discovery is defined independently of ownership, while command-only inventory is distinguished from prompt applicability. Unsupported or unreadable recognized content must fail coverage in default and CI modes even without policy.

  • The implementation and inspected tests provide substantive evidence for decoded native-field inspection, command non-execution, symlink exclusion, transcript traversal boundaries, shared-settings preservation and pre-write refusal of mixed protected remediation.

  • Default JSON and SARIF reporting distinguish decoded prompt coordinates from physical source locations; human-facing reporting includes literal rendering defenses.

  • Read-only verification confirmed agreement among all 125 actual requirement anchors, Appendix C entries, requirement-manifest entries and generated conformance records. The reported 122 passing focused tests were not rerun and are not treated as CI-green evidence.

spec-pkgmgr-editor -- shocked_meter 8/10, confidence high

Summary: Reviewed PR #2962's supplied full diff and complete specification at local revision 58dafc0. The bounded optional capability preserves the package-manager integrity and ownership boundaries; the substantive followups are CI finding-to-field fidelity and explicit support for standalone Governance implementations. Supplied test results were not rerun, no CI outcome is inferred, and this advisory assessment does not establish specification adoption.

New recommended findings (2)

  • [pkg-rec-r1-1] req-pl-019(d); tests/spec_conformance/test_policy_reqs.py -- The proposed field-location obligation is not fully supported by the claimed CI implementation. In src/apm_cli/policy/ci_checks.py, _check_content_integrity reduces critical prompt findings to filenames and emits only 'unicode: '. The separately reported inventory includes pointers for every checked field, but does not identify which field produced a finding. A shared settings file with two prompt handlers, only one containing a critical finding, therefore loses the required finding-to-field association in CI JSON, text and SARIF. The new req-pl-019 conformance test parameterizes CI only for malformed-content failure; it does not assert native prompt finding locations in CI. This is a conformance-evidence refinement, not a demonstrated false passing audit.
    Recommended fix: Retain req-pl-019(d). Preserve each critical native finding's field pointer when constructing CI violation details, independently of coverage inventory, without presenting decoded offsets as physical coordinates. Extend the req-pl-019 behavioral test with two native prompt fields, one clean and one failing, and assert that default and CI reports identify the failing field rather than merely listing both checked fields.

  • [pkg-rec-r1-2] req-pl-019(a); Section 11.1 -- The requirement tells a Governance implementation to use the same definitions 'as its deployment implementation', implicitly assuming that the claimant has a deployment implementation. Section 11.1 otherwise permits independently claimed classes. A standalone Governance auditor can consume a Consumer's declared deployment profile without implementing deployment, but the new wording does not clearly accommodate that arrangement. This is an avoidable ambiguity in the optional capability's conformance boundary.
    Recommended fix: Replace 'as its deployment implementation' with 'as the Consumer deployment implementation identified by its conformance statement'. Add: 'A Governance implementation may consume those definitions from another implementation; claiming this capability does not require implementing deployment or claiming the Consumer class.' Keep the declared-scope and independent-discovery obligations unchanged.

New nit findings (2)

  • [pkg-nit-r1-1] docs/public/specs/manifests/openapm-v0.1.requirements.yml: req-pl-018 through req-pl-020 -- Inserting req-pl-019 and req-pl-020 before the existing notes property moved the dependency-policy identity-casing note from req-pl-018 onto req-pl-020. The new remediation requirement consequently carries unrelated explanatory metadata, while req-pl-018 loses its note. ID-count alignment does not detect this association error.
    Recommended fix: Move the existing allow/deny/require identity-casing notes property back under req-pl-018, before the req-pl-019 list entry.

  • [pkg-nit-r1-2] Appendix D: proposed 0.1.42 row -- A blank line separates the proposed 0.1.42 row from the revision-history table. In ordinary Markdown table rendering, the amendment becomes a standalone pipe-delimited paragraph rather than a table row.
    Recommended fix: Remove the blank line immediately preceding the proposed 0.1.42 row.

Preserved strengths confirmed

  • Both new requirements are conditional on an explicit deployed-prompt-audit capability claim; the amendment does not impose prompt auditing on implementations that do not claim it.

  • Appendix D distinguishes optional specification adoption from APM's changed default and CI behavior, expressly naming newly failing findings, incomplete coverage and protected remediation refusal. It does not present the change as behavior-neutral errata.

  • Section 9.3 reviewer approvals and the public comment period remain explicitly pending; the conformance disclosure does not represent the proposal as adopted.

  • Discovery is independent of ownership records, while prompt applicability excludes commands and executable bodies. The implementation tests separately exercise non-prompt script hash verification, preserving the distinction between prompt checks and integrity checks.

  • Incomplete recognized content fails default and CI audits independently of policy, while unmanaged-file evaluation, source materialization scanning and drift policy remain separately defined.

  • The remediation contract grants no ownership or execution authority and requires refusing protected-content remediation before writing any selected content. The new behavioral test verifies preservation of a mixed protected-settings and ordinary-prompt selection.

  • The implementation-specific target, format and check scope is disclosed rather than presented as a universal native-format standard. No new manifest schema, lockfile schema, lockfile version or dependency-resolution rule is introduced.

spec-tag-architect -- shocked_meter 8/10, confidence high

Summary: Reviewed PR #2962 at 58dafc0, including the complete post-edit specification and changed conformance artifacts and behavioral tests; no new blocking architectural finding was established. Recommended followups make the optional claim non-vacuous and narrow its scanner disclosure to the implemented checks. This advisory assessment does not establish adoption or green CI: Section 9.3 human approvals and the 14-day comment period remain pending, and the supplied 122-test result was not independently rerun.

New recommended findings (2)

  • [tag-rec-r1-1] Section 6.8.1 introduction; req-pl-019(a) -- The declared-scope model usefully binds discovery to deployment definitions, but does not explicitly prohibit a vacuous capability claim. A claimant could declare no prompt-bearing locations or no prompt checks and satisfy the conditional discovery and completeness obligations without performing a prompt audit. APM's published profile is substantive; this is a third-party conformance-contract gap, not an observed APM omission.
    Recommended fix: Add a claim condition under req-pl-019 requiring a non-empty declared target/scope profile with at least one prompt-bearing location or field and one specified content check. Distinguish this implementation-level minimum from an individual workspace containing no matching files, which may legitimately produce a complete empty result. Require intentional exclusions to be disclosed rather than inferred from an empty report.

  • [tag-rec-r1-2] Section 6.8.1 capability disclosure; tests/spec_conformance/gen_statement.py DEPLOYED_PROMPT_AUDIT; CONFORMANCE.md and CONFORMANCE.json -- The new implementation disclosure says ContentScanner checks 'hidden controls and prompt-injection markers'. The actual scan_text implementation checks suspicious Unicode code points and returns immediately for all-ASCII content; it does not implement a separate prompt-injection marker detector. Although the following safety disclaimer is valuable, the description of the implemented check can still suggest broader coverage than the algorithm supplies.
    Recommended fix: Change the generator's disclosure to describe 'hidden or suspicious Unicode code points that may conceal instructions', explicitly excluding semantic prompt-injection detection, and regenerate both conformance statements. Retain the existing statement that a clean result is not proof that prompt text is safe.

New nit findings (2)

  • [tag-nit-r1-1] docs/public/specs/manifests/openapm-v0.1.requirements.yml, req-pl-018 through req-pl-020 -- Insertion of the two new entries leaves the existing dependency-policy case-normalization notes attached to req-pl-020 instead of req-pl-018. The machine-readable projection now describes protected remediation using unrelated identity-matching semantics.
    Recommended fix: Move the existing notes property back beneath req-pl-018. Optionally add separate notes identifying req-pl-019 and req-pl-020 as conditional on the deployed-prompt-audit capability.

  • [tag-nit-r1-2] Appendix D, proposed 0.1.42 row -- A blank line before the proposed 0.1.42 row terminates the existing Markdown table, leaving this load-bearing amendment and pending-adoption notice outside the rendered revision-history table.
    Recommended fix: Remove the blank line between the 0.1.41 and proposed 0.1.42 rows.

Preserved strengths confirmed

  • The new requirements are expressly conditional on claiming deployed-prompt-audit, rather than silently expanding every Governance implementation's mandatory feature set.

  • Discovery, prompt applicability, unmanaged-file policy, source-plan scanning, drift, ownership and executable authorization remain distinct contracts.

  • req-pl-019(c) requires incomplete recognized coverage to fail in default and CI modes even without policy; unsupported native formats are not converted into clean results.

  • req-pl-019(d) explicitly separates decoded prompt coordinates from physical file coordinates and requires literal human-facing rendering. Integration assertions exercise JSON coordinate fields, SARIF region omission, Markdown escaping and entity handling.

  • req-pl-020 denies execution and mutation authority from discovery, prohibits symlink traversal, and requires protected remediation refusal before any selected content is written. The behavioral conformance test checks preservation across a mixed protected/unprotected selection.

  • The conformance statement publishes a concrete target and format profile and disclaims safety proof; the implementation profile is not itself an empty-scope escape hatch.

  • Appendix D distinguishes the opt-in specification capability from APM's changed default audit outcomes and explicitly preserves the pending Section 9.3 approvals and public comment period.

  • The new requirement IDs appear in both Governance enumerations, Appendix C, the requirements manifest and generated conformance statements. Active test binding is not presented here as proof of green CI.

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


Generated by apm-spec-guardian. This comment is AI-generated and may contain errors.

Fold actual guardian findings without weakening conditional requirements, reporting guarantees, or pending specification adoption.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@danielmeppiel

Copy link
Copy Markdown
Collaborator

APM Spec Guardian: fold_and_ship

Scope: editorial-patch; diff = +76/-4 lines across 1 spec file. Shocked-meter avg: 8.0/10.

For PR #2962 at head 66ba9ea, recommend fold_and_ship with no additional folds. The four actual round-2 returns independently recommend ship at 8/10, yielding an arithmetic mean of 8.0 and zero new blocking, recommended or nit findings. The prior OCI blocker, oci-blk-r1-1, is explicitly closed through implementation and behavioral evidence, not wording alone: user-scope metadata survives discovery and recorded-file fallback independently of HOME containment, automatic stripping excludes protected content, and real CLI subprocess tests cover tracked and untracked user documents with refusal and byte-preservation assertions. The mixed-selection preservation test and killed user-scope mutant strengthen that closure. Reporting, capability disclosure and editorial follow-ups are also closed. No unresolved patch or architectural work is established by these returns, so neither folds nor deferrals are invented; there are no proposed fixes requiring forbidden-affiliation rejection.

This is the final editorial-patch round, 2 of 2, and its evidence supports ending this spec-review loop without another brief. The panels inspected exact-head evidence reporting 1379 passing tests, two pre-existing skips, 15 killed mutants and passing canonical lint/architecture checks; they did not rerun those suites, and this synthesis does not claim independent execution. The highest-signal remaining follow-up is the orchestrator's post-return eleven-check mechanical verification and the outcome of the six approved GitHub CI runs, currently pending rather than green. Section 9.3 human approvals and the 14-day public comment period remain pending; proposed specification changes are not adopted. This technical recommendation is advisory only, not merge authorization or a label decision, and does not replace the separate general CEO synthesis.

Convergence

| Panel | Stance | Shocked | New B | New R | New N |

|---|---|---:|---:|---:|---:|

| spec-swagger-editor | ship | 8/10 | 0 | 0 | 0 |

| spec-oci-editor | ship | 8/10 | 0 | 0 | 0 |

| spec-pkgmgr-editor | ship | 8/10 | 0 | 0 | 0 |

| spec-tag-architect | ship | 8/10 | 0 | 0 | 0 |

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

Convergent themes (flagged by 2+ panels)

  • T1 -- Closed reporting findings: default and CI outputs retain the offending pointer and actual artifact, while distinguishing decoded coordinates from physical source regions. (supporting: sw-rec-r1-1, oci-rec-r1-1, pkg-rec-r1-1)

  • T2 -- Closed capability-definition findings: substantive profile/check/exclusion disclosure is required, and standalone Governance implementations may use an identified Consumer's definitions without claiming deployment conformance. (supporting: sw-rec-r1-2, pkg-rec-r1-2, tag-rec-r1-1)

Linter notes (1 check(s) failed)

  • [11] Non-spec Python files changed; general review also required. Exact 66ba9ea canonical lint independently passed.

Linter handoff: Rerun all eleven mechanical checks after this return against head 66ba9ea; record actual pass, fail and skipped outcomes rather than treating panel inspection as fresh execution. Specifically verify ASCII and forbidden-token absence; schema validity and reference resolution; fixture parsing and requirement citations; 125 unique requirement anchors and agreement across all three count sites on 120 MUST plus 5 SHOULD; Governance enumeration of req-pl-019 and req-pl-020; local links; notes attached to req-pl-018; and the proposed 0.1.42 row remaining inside the revision-history table. Confirm both generated conformance statements match read-only regeneration, disclose Unicode checks and semantic prompt-injection exclusions, and preserve pending-adoption language. Handle Mermaid and CHANGELOG checks explicitly. For check 11, surface the mixed Python/spec scope and separate general review rather than asserting no Python changes. Keep reported exact-head lint/architecture success distinct from pending GitHub CI. Any mechanical failure must be surfaced as an advisory linter note without silently changing this ship_decision.


Full per-panel findings

spec-swagger-editor -- shocked_meter 8/10, confidence high

Summary: At 66ba9ea, the bounded corrections close my prior findings without introducing a new contract defect. I directly checked structural invariants and CI serializers and inspected execution evidence recording 1379 passes, two existing skips, lifecycle execution, 15 detected mutants and canonical lint success; I did not rerun those suites. This is an advisory technical ship assessment, not green CI or specification adoption: Section 9.3 human approvals and the 14-day public comment period remain pending.

Preserved strengths confirmed

  • Capability obligations remain conditional, with discovery, applicability, coverage, findings and mutation authority separated. Generator and both generated statements consistently disclose Unicode checks, explicitly exclude semantic prompt-injection detection, and retain pending-adoption language.

  • User-scope metadata survives discovered and recorded paths independently of HOME containment. The exact-head execution record includes both tracked and untracked Copilot subprocess refusal cases plus mixed-selection preservation; the user-scope mutation is detected.

  • There are 125 unique requirement anchors, with only req-pl-019 and req-pl-020 added and correctly enumerated as Governance. All three count sites agree on 120 MUST and 5 SHOULD; six schemas validate and their references resolve, local links resolve, and both generated statements match read-only regeneration.

spec-oci-editor -- shocked_meter 8/10, confidence high

Summary: The bounded round-2 corrections close the actual OCI blocker and reporting follow-up without introducing a new in-scope finding. I inspected source, tests and supplied exact-head evidence reporting 1379 passes, two existing skips, 15 killed mutants and passing lint/architecture checks; I did not rerun those suites or establish green CI. This assessment is advisory, not adoption or merge authorization: human approvals and the Section 9.3 14-day public comment period remain pending.

Preserved strengths confirmed

  • Discovery remains distinct from ownership, executable authorization and mutation authority; incomplete recognized coverage fails independently of policy, while valid command-only inventory remains non-failing.

  • The capability remains conditional and now requires substantive profile disclosure without forcing Governance implementations to claim Consumer conformance. Generated disclosures correctly limit checks to suspicious Unicode rather than semantic prompt-injection detection.

  • Existing integrity, mirror, extraction and credential-redaction contracts remain unchanged; Appendix D preserves proposed status and pending Section 9.3 adoption requirements.

spec-pkgmgr-editor -- shocked_meter 8/10, confidence high

Summary: Reviewed the complete specification and bounded correction evidence at 66ba9ea; this panel's prior findings are closed, with no new substantive issue in the optional contract. Supplied artifacts record 1379 passing tests, two unrelated conformance skips, 15 killed mutations and passing canonical lint/architecture checks; I inspected that evidence rather than rerunning it, and infer no green CI result. The technical ship recommendation is advisory, not adoption: Section 9.3 human approvals and the subsequent 14-day public comment period remain pending.

Preserved strengths confirmed

  • The capability remains OPTIONAL and independently claimable by Governance; req-pl-019 now requires substantive, non-empty scope/check disclosure and intentional exclusions without imposing deployment behavior on that class.

  • Discovery remains distinct from ownership, executable authorization, hash verification and drift policy. req-pl-020 explicitly protects user-scoped content even with HOME as traversal root; both tracked/untracked static-user tests and the mixed-selection preservation test pass in the supplied XML, and the user-scope mutation is killed.

  • No dependency-resolution rule, registry trust anchor, schema or lockfile-version contract changes. Appendix D distinguishes APM's changed audit behavior from optional specification adoption and retains the pending Section 9.3 process.

spec-tag-architect -- shocked_meter 8/10, confidence high

Summary: Reviewed the complete current specification and round-2 folds at 66ba9ea; no new in-scope architectural finding warrants another revision. Inspected supplied evidence records 1379 passes, two existing skips, lifecycle/conformance execution, 15 assertion-killed mutations and passing lint/architecture checks; these were not independently rerun. Technical ship advice is advisory only and establishes neither green GitHub CI nor adoption: Section 9.3 human approvals and the 14-day public comment period remain pending.

Preserved strengths confirmed

  • The capability remains conditional and architecturally separable: req-pl-019 identifies the Consumer definition provider while permitting standalone Governance implementations without deployment or a Consumer-class claim. Discovery, applicability, coverage, ownership and authorization remain distinct.

  • req-pl-020 now protects user-scoped content independently of HOME containment. PrimitiveSurface carries user_scope through discovery and recorded-file fallback; remediation eligibility excludes it. The supplied functional XML records passing real-CLI tracked and untracked static-user regressions asserting refusal and byte preservation.

  • req-pl-019(d)'s finding attribution survives default and CI reporting through shared serializers: JSON identifies the offending field and decoded coordinate space; SARIF identifies the actual prompt artifact and pointer without inventing a physical region. Executed two-field regressions move the sole finding between fields across text, JSON and SARIF.

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


Generated by apm-spec-guardian. This comment is AI-generated and may contain errors.

Register the authority and reject all four ordinary/CI JSON/SARIF consumer bypasses without changing runtime or specification behavior.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@danielmeppiel

Copy link
Copy Markdown
Collaborator

Advisory delta review: needs_rework

At 66ba9ea6ec4bb992a545042cd917f4c29d3aee26, the six-persona delta panel found one remaining in-scope recommendation: enforce the newly shared JSON/SARIF finding serializers as a registered architecture boundary. A source-override probe bypassed the CI serializer calls without a static violation. This is a regression-prevention gap, not a demonstrated runtime failure.

The actual CEO recommends folding that guard before the final reassessment, honoring the maintainer's request to address every in-scope recommendation. The fold is limited to authority metadata, the existing architecture rule and four consumer-bypass tests; no runtime or specification redesign is proposed.

The other five reviewers found no new substantive issue. The coverage reviewer independently ran 110 focused cases; the logging reviewer ran 19 reporting cases. Exact-head local evidence records 1,379 passes, two existing conformance skips and 15 assertion-killed runtime mutants, with canonical lint and architecture checks passing. These facts do not erase the missing static boundary.

The separate specification guardian completed its second and final round with fold_and_ship, four scores of 8/10 and no remaining specification folds. Its previously reported user-scope remediation finding is closed by implementation and real-CLI preservation evidence. Section 9.3 human approvals and the subsequent 14-day public comment period remain pending; this is not specification adoption.

At review time, Linux CI, lifecycle, specification, docs and CodeQL analysis jobs had passed; Windows Compatibility and Merge Gate were still running. Docs-sync's tool-starved classifier remains a disclosed orchestration limitation. This advisory does not authorize a merge or change reviewer requirements.


Generated by autopilot-pr-review-worker. This comment is AI-generated and may contain errors.

@danielmeppiel

Copy link
Copy Markdown
Collaborator

Final bounded delta: needs_rework

The actual final CEO recommendation is needs_rework, not ship_now. The last guard-only candidate is committed locally at d317fd2ffdbb622135f1f4e000c477a39fbfc0e2 but has not been pushed. The published PR remains at 66ba9ea6ec4bb992a545042cd917f4c29d3aee26.

The Python Architect confirmed that the new canonical serializer guard closes the preceding recommendation: one registered owner, four ordinary/CI JSON/SARIF call-routing checks, and four source-override bypass tests. Independent architect checks passed 31 tests and the architecture lint.

The Test Coverage reviewer found a real failure in a previously omitted suite:

tests/integration/test_architecture_owner_rule_mutations.py::
test_matrix_covers_every_registry_guard_exactly_once

The exhaustive matrix lacks both audit-finding-serialization and audit-primitive-discovery. The former is new in this final candidate; the latter was introduced earlier in this PR. The required bounded repair is to add meaningful mutation cases for both IDs in the existing matrix, mapped to the existing audit-primitive-discovery rule, retaining the dedicated bypass tests.

The local candidate's 1,435 passing tests, two existing conformance skips, 15 killed runtime mutants and full canonical lint/architecture success remain valid evidence, but do not include or override this failing matrix check. The coverage reviewer independently passed the 10 dedicated guard tests and 72 architecture-runner tests before reproducing the matrix failure.

The authorized four-iteration limit is exhausted. No fifth autonomous fold, push, or review has been started. This is unresolved work inside the approved scope, not an out-of-scope deferral. Copilot classification remains 2/2; CI recovery 1/3; specification guardian 2/2.

The published head's checks are terminal success/neutral/skipped and GitHub reports MERGEABLE/BLOCKED. They do not certify the unpublished candidate. The unchanged specification has the actual guardian recommendation fold_and_ship (8/10); Section 9.3 human approvals and the subsequent 14-day comment period remain pending. No merge, auto-merge, reviewer change, or user configuration mutation was performed.


Generated by autopilot-pr-review-worker. This comment is AI-generated and may contain errors.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Preserve main's owner matrix and conformance additions; regenerate combined conformance statements.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@danielmeppiel

Copy link
Copy Markdown
Collaborator

Authorized extra pass: audit gaps closed; qualification still fails

The actual final CEO recommendation remains needs_rework, for a different, inherited failure. Both previously reported audit matrix gaps are fixed by 04cea96ac4: meaningful cases for audit-primitive-discovery and audit-finding-serialization now live in the existing exhaustive matrix. All dedicated guards and the exhaustive registry equality assertion remain intact.

The maintainer authorized exactly one exceptional test-only pass, cumulative outer5, not a reset. Current main was then composed cleanly through d3c5cd9bd793fd430d6e0eb3371acbdcaedcd8d3, preserving upstream owner/mutation entries and regenerating combined conformance statements.

At resultant local 58bd983865dec617702a775b1be62aae0033ead4:

1 failed, 1871 passed, 2 skipped in 620.75s (0:10:20)

The sole failure is test_matrix_covers_every_registry_guard_exactly_once: main's newly registered transport-platform-marketplace-package-remote guard has no matrix case. Both the Architect and Test Coverage reviewers independently reproduced it. This omission comes from the required main composition (#3044), not the two audit additions.

The two authorized cases pass, and removing their respective detector requirements in memory causes their actual matrix assertions to fail; restoring those requirements makes them pass again. Full canonical lint/architecture checks and fifteen runtime mutation trials also pass. Those positive results do not override the failing exhaustive matrix.

No unauthorized third case or sixth iteration was started. The candidate remains unpublished. The original PR still points to 66ba9ea6ec4bb992a545042cd917f4c29d3aee26; its completed success/neutral/skipped checks cannot certify the local candidate. Further handling of the inherited matrix failure needs maintainer disposition.

The guardian-reviewed normative specification remains byte-identical to66. Generated conformance statements incorporate six upstream lockfile tests; no new specification review or normative extension occurred. The guardian recommendation remains fold_and_ship at8, distinct from outstanding Section9.3 human approvals and the subsequent14-day comment period.

Accounting is preserved: actual outer5, Copilot2, CI recovery1, guardian2. The canonical completion schema is unchanged; its optional iterations field is omitted, with the actual count and direct authorization bound in iteration-exception.json. No merge, auto-merge, reviewer change or user-configuration mutation was performed.


Generated by autopilot-pr-review-worker. This comment is AI-generated and may contain errors.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@danielmeppiel

Daniel Meppiel (danielmeppiel) commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

APM Review Panel: ship_now

TECHNICAL ONLY / Section 9.3 adoption HOLD. The prior actual CEO recommendation is retained with explicit attribution after mechanical conflict repair. This is not a new panel, unconditional readiness, or permission to merge or adopt the proposed specification.

panel-mode=delta; current-review=mechanical-conflict-repair; last-actual-persona=apm-ceo@dc89f5fe

The requested conflict repair composes 73d6d0e280d9d898bb9ab84c7ec28f4348691199 into prior head dc89f5fe56726a1c23b82f8d853e449843e91044, preserving both parents in merge 78b8b5a4e55b7f3ebe88e7b8fcab3f48241cd462. Its test/docs-only successor and current qualified head is 0cac8097a9bf3e4a4a1cc8a01cc2f303787642ad. Main includes #2876, #3119 and #3116. Both actual conflicts were additive, not competing implementations:

  • .apm/architecture/owners/hooks-integrations.json: retain both audit owners and the native-instruction projection owner.
  • src/apm_cli/compilation/root_context_protection.py: retain the audit filename helper and the protected-root/explicit-cleanup helpers.

Every changed upstream function body (40) and both prior audit function bodies in overlapping runtime files retain their parent ASTs. The merge had 80 upstream-identical paths and 11 faithful unions. The successor changes only three lifecycle test locations and a documentation paragraph; it has no runtime/spec changes. No function signature or invariant was reinterpreted, and no new specialist/CEO panel or guardian was run. The previous full/delta panel remains historical; the last actual narrow CEO ship_now remains attributed to dc89f5fe.

The merged-head run exposed seven stale test expectations across three lifecycle locations: fully native Claude instructions correctly no longer produce a redundant CLAUDE.md. The corrected tests require the exact installed native revision, absence of that redundant root and complete artifact snapshots. Mixed Claude/Hermes fixtures permit only Hermes AGENTS.md output and verify its revision freshness. No test selection or assertion was suppressed; unmatched fallback, policy scanning/refusal and protected cleanup remain covered.

Section 9.3 still requires a spec-labelled issue and at least two non-author approvals covering implementation and consumer/integrator experience, followed by a 14-day public comment period. The observed linked issue #2961 lacks the spec label and review history has no qualifying approvals. No qualifying timestamp or earliest adoption date is established. Neither AI advice nor the proposed September 30 history entry starts that clock.

Aligned with: one canonical owner per decision, preservation of both parents' protections and tests, and separate technical evidence from human adoption.

Panel summary

Persona B R N Takeaway
APM CEO (last actual pass: dc89f5fe) 0 0 0 Historical technical ship_now; no new persona execution is claimed for this mechanical repair.

B = blocking-severity findings, R = recommended, N = nits.
Counts are historical review signals, not adoption approvals.

Recommendation

Retain the prior technical ship_now recommendation after verified additive conflict repair, bounded test-model reconciliation and this result head's own validation. Keep the specification-adoption hold and normal repository approvals separate. No merge, auto-merge, review approval, queue action or policy waiver was performed.


Full per-persona findings

APM CEO - historical, not rerun

The actual prior composition assessment found no follow-ups at dc89f5fe. Current conflict resolution preserves the reviewed audit implementation while incorporating the already-landed upstream changes; fresh execution, not an invented review, qualifies the resulting tree.

Current-head execution

The unchanged combined functional selection executed at 0cac8097: 3,357 passed, 2 existing skips, 2 subtests passed in 1,256.77 seconds. JUnit contains 3,359 unique named outcomes. All seven current-main lint gates and the architecture boundary check passed. The intermediate 78b8b5a4 run's 3,350 passes, seven stale-oracle failures and two skips remain separately attributed.

The full run combines prior audit coverage with every incoming changed test module: HOME/APM_HOME aliases, ref freshness, Claude native deduplication and policy scanning, protected explicit cleanup, Cursor runtime/static values, real installed-CLI lifecycle snapshots, native evidence/provenance owners, complete owner mutation matrix and frozen registry, conformance and quality.

Nine physical on-disk guard/registry removals caused assertion failures and then passed after byte-exact restoration. They cover root prompt discovery, cleanup hash/current-content protection, both audit owner records plus the native projection owner, and native-eligibility removal against each of the three corrected lifecycle locations. Fifteen process-local audit mutations and three actual-matrix detector-removal controls were also rerun. No mutant was committed. These probes do not claim to mutate every incoming upstream test independently.

Strict terminal verification returns verified=true, terminal_evidence_required=true, 8 touched owners / 31 actual passing functional IDs. Incoming native/provenance owners are retained and exercised in the whole registry/matrix and lifecycle selection; they are not falsely classified as PR-specific changes.

All six identity-checked current-head workflow runs completed successfully: CI, spec conformance, merge gate, docs, NOTICE and CodeQL. The qualification snapshot observed 19 SUCCESS, 2 NEUTRAL, 1 SKIPPED, with GitHub MERGEABLE/BLOCKED. Only workflow execution was approved; no human review approval occurred.

Canonical completion SHA256: 56c63303cf0b34c4171d20ed81b32ff577997a179649d60621831a50407ee493. The matching iteration-exception.json, scenario-evidence.json, strict owner receipt and unchanged-spec crosswalk bind this head's execution and historical identity. The final terminal seal includes the live publication readback and current CI snapshot.

Specification and historical evidence

All 34 guardian-identity artifacts remain byte-identical to reviewed 66ba9ea6; no third guardian was run. Fifty prior composition-identity files remain identical to dc89f5fe. Four artifacts changed faithfully with main: the owner matrix, lockfile conformance tests and generated CONFORMANCE.json/CONFORMANCE.md. The generator reproduced the combined statements; these four are explicitly not claimed byte-identical to the prior packet.

The accepted dc89f5fe packet is preserved under qualified-dc89, including completion SHA256 79388c7f25b3c0b8a2b88120245b48342ca4bece9c3c0c45ea4ff47bc4e76b7a and seal db440c48b8568a60ffcd0e9e163e1ec4f44838aa7412143b7996496ba8b26f6c. It remains historical, not rebound to the repaired head.

Disclosure

Actual cumulative accounting is outer 9 / Copilot 2 / CI recovery 1 / guardian 2, under the explicit conflict request and continuous mergeability authorization; no reset. The unchanged canonical schema omits only its unsupported optional iteration field, with the true count and completion hash bound in iteration-exception.json. The seven failures from intermediate 78b8b5a4 remain archived under that SHA, not relabeled as current-head results.

Existing conformance skips, the earlier timeout with independent passing evidence, the incomplete tool-starved docs-sync classification and the preliminary-scanner boundary of the user-scope mutation remain disclosed. Local validation uses macOS and the editable source CLI; inherited native-lifecycle proof is not an all-platform shipping claim. Actual CI jobs provide separate platform evidence.

This panel is advisory. Human specification adoption and normal repository approval requirements remain separate and unchanged.


Generated by autopilot-pr-merge-worker. This comment is AI-generated and may contain errors.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Resolve additive owner-registry and root-context helper conflicts by retaining both parent contracts unchanged.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Preserve exact native revisions and no redundant Claude root, with complete snapshot assertions and isolated Hermes compilation writes after main composition.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@danielmeppiel
Daniel Meppiel (danielmeppiel) merged commit 4e916ec into microsoft:main Sep 30, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/audit-policy apm-policy.yml schema, custom_checks, install-time enforcement. area/cli CLI command surface, flags, help text (cross-cutting). status/accepted Human scope approval; verify the issue's approval record and review contact before work. triage/recommended Automated advice completed; not human scope approval. type/bug Something does not work as documented.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] audit --ci walks the whole target root (session transcripts under ~/.claude, ~/.codex): minutes of CPU and false content-integrity findings

5 participants