fix(ado): preserve lock coordinates for outdated and update

[danielmeppiel]

fix(ado): reconstruct lock consumers from generic repository identity

TL;DR

Azure DevOps dependencies now survive lock reload, apm outdated, bounded
apm update, audit, and convergence without provider-specific lockfile fields.
The lock persists the same generic host and repo_url identity used for
GitHub, GitLab, and other Git providers; DependencyReference derives transient
ADO transport coordinates when reconstructing the consumer.

Important

Exact head 3697a6e42a43d850a266cd71fd0c1cea1f88cdcd is frozen for human review. This PR must not be auto-merged.

Problem (WHY)

• Closes [BUG] apm outdated / apm update report unknown for Azure DevOps deps — LockedDependency.to_dependency_ref() drops ado_* fields (distinct from #2150 / #2119) #2197: showed that lock reconstruction dropped transient ADO
  coordinates, causing canonical URL construction to fail and apm outdated
  to report unknown.
• apm update could not resolve an ADO semver range after lock reload, even
  though direct apm view ... versions and git ls-remote worked for the same
  repository.
• Persisting ado_organization, ado_project, and ado_repo would repair
  the symptom but duplicate information already present in
  host + repo_url.
• [!] A provider-specific lock projection would diverge from GitHub, GitLab,
  and generic-host behavior and create two values that must remain consistent.

The implementation follows the existing-owner rule rather than adding another
parser: "Synthesize from existing project artifacts".

Approach (WHAT)

1. Keep LockedDependency and apm.lock.yaml provider-neutral.
2. Reconstruct a generic DependencyReference, then derive transient ADO
   organization/project/repository coordinates from canonical host + repo_url.
3. Validate those coordinates before producing the ADO /_git/ transport URL.
4. Drop retired experimental ADO lock keys on read while preserving unrelated
   unknown fields for forward compatibility.
5. Guard the ownership boundary with AC14 and a real installed-binary lifecycle.

Implementation (HOW)

Surface	Change
src/apm_cli/deps/lockfile.py	Removes all ado_* dataclass, YAML, and reconstruction fields. Generic lock state is converted through the DependencyReference model boundary.
src/apm_cli/models/dependency/reference.py	Retains canonical ADO path validation and URL construction; exposes provider-coordinate derivation through the model interface.
src/apm_cli/models/dependency/provider_coordinates.py	Holds transient coordinate derivation/validation outside the large parser module while remaining inherited behavior of DependencyReference.
src/apm_cli/install/helpers/ref_reuse.py, src/apm_cli/deps/git_semver_resolver.py, src/apm_cli/marketplace/ref_resolver.py	Preserve the merged #2229 transport architecture and pass the canonical ADO HTTPS URL only after transport selection.
tests/unit/deps/test_lockfile_consumer_contract.py	Proves generic YAML round-trip, transient derivation, retired-key removal, unrelated unknown-field preservation, and non-ADO behavior.
tests/integration/test_ado_lock_consumer_contract.py	Exercises real install, lock reload, outdated, bounded update, audit, materialized bytes, and convergence while asserting no ADO lock keys exist.
scripts/lint-architecture-boundaries.sh, tests/integration/test_architecture_authorities.py	AC14 rejects provider fields or local ADO parsing in the lock owner.
tests/quality/critical_suite.toml, tests/quality/test_test_taxonomy.py	Keep predecessor 15 plus one ADO lifecycle row, for 16 total and TM002=16.
docs/src/content/docs/reference/lockfile-spec.md, CHANGELOG.md	Document provider-neutral persistence and the restored consumer behavior.
Resolver/outdated unit tests	Verify the reconstructed ADO URL, cache identity, auth-safe remote validation, and update wiring without provider-specific lock fixtures.

Diagrams

Legend: persisted state stays generic; only the in-memory reference derives the
ADO transport shape consumed by outdated and update.

flowchart LR
    subgraph Persist["Generic lock state"]
        I["host + repo_url"]
        Y["apm.lock.yaml"]
        I --> Y
    end
    subgraph Reconstruct["Canonical model boundary"]
        L[LockedDependency]
        D[DependencyReference]
        C["Transient ADO coordinates"]
        Y --> L
        L --> D
        D --> C
    end
    subgraph Consume["Real consumers"]
        U["Canonical /_git/ URL"]
        O["apm outdated"]
        P["apm update"]
        C --> U
        U --> O
        U --> P
    end
    classDef new stroke-dasharray: 5 5;
    class C,U new;

Loading

Trade-offs

• Derive instead of persist. Chose one canonical generic identity; rejected
  redundant provider fields and their mismatch states.
• Transient model fields remain. The existing ADO downloader APIs still
  consume named coordinates, so this PR derives them in memory rather than
  widening the change into a downloader rewrite.
• Retired keys are dropped deliberately. The provider fields never shipped
  on main; preserving them as unknown metadata would violate the final schema.
  Other unknown fields continue to round-trip.
• One small model module. Extracted provider-coordinate behavior because the
  canonical parser must remain below the 2,100-line CI guardrail.

Benefits

1. Fresh and rewritten ADO lock entries contain zero ado_* fields.
2. Generic host + repo_url reconstructs all 3 transient ADO coordinates.
3. A lock pinned to v1.0.0 reports v1.1.0, outdated, and git tags.
4. A bounded ^1.0.0 update changes the commit, ref, and deployed bytes once,
   then converges byte-for-byte.
5. GitHub, GitLab, generic Git, and unknown-field behavior remain unchanged.

Validation

APM_BINARY_PATH=dist/apm-darwin-arm64/apm APM_E2E_TESTS=1 APM_TIERED_RESOLVER=0 pytest <Codex+SSH+ADO>:

6 passed in 40.27s

pytest tests/unit tests/test_console.py tests/red_team -n 2 --dist worksteal:

19170 passed, 2 skipped, 21 xfailed, 86 subtests passed in 88.45s

pytest tests/quality tests/spec_conformance tests/integration/test_architecture_authorities.py:

202 passed, 2 skipped in 159.94s

apm audit --ci --no-policy --format json:

{"passed": true, "summary": {"total": 9, "passed": 9, "failed": 0}}

Exact-head and mutation evidence

Head: 3697a6e42a43d850a266cd71fd0c1cea1f88cdcd
Shepherd fold: test_validate_provider_coordinates_rejects_mismatched_ado_fields
Mutation: remove the supplied != canonical guard
Result: test_validate_provider_coordinates_rejects_mismatched_ado_fields FAILED
Restored result: 1 passed
Targeted ADO suite: 127 passed in 1.38s
Boundary lint: AC1-AC14 clean

Hosted exact-head runs are green with 0 red and 0 pending:
CI 29479999500, CodeQL 29479999474, Merge Gate 29479999532,
Spec 29479999465, Docs 29479999462, NOTICE 29479999478.

Scenario Evidence

#	Scenario (user promise)	Principle(s)	Test(s) proving it	Type
1	Reload an ADO lock and see the real latest tag instead of unknown.	DevX, Vendor-neutral	tests/integration/test_ado_lock_consumer_contract.py::test_ado_lock_replay_drives_outdated_update_audit_and_convergence	e2e
2	Update an ADO semver range once and converge without lock churn.	Governed by policy	tests/integration/test_ado_lock_consumer_contract.py::test_ado_lock_replay_drives_outdated_update_audit_and_convergence	e2e
3	Keep provider-specific ADO coordinates out of the lock schema.	Vendor-neutral	tests/unit/deps/test_lockfile_consumer_contract.py::test_ado_coordinates_are_derived_after_generic_lock_round_trip	unit
4	Remove retired ADO keys without dropping unrelated future metadata.	Secure by default	tests/unit/deps/test_lockfile_consumer_contract.py::test_retired_ado_lock_fields_are_dropped_without_losing_unknown_fields	unit
5	Prevent a second ADO parser or provider field projection in lock code.	Governed by policy	tests/integration/test_architecture_authorities.py::test_ado_lock_coordinates_have_single_owner	integration

How to test

• Run the ADO binary lifecycle; expect one pass and no ado_* keys in its generated lock.
• Run tests/unit/deps/test_lockfile_consumer_contract.py; expect 8 passes.
• Run the combined Codex+SSH+ADO binary contract; expect 6 passes.
• Run bash scripts/lint-architecture-boundaries.sh; expect AC1-AC14 clean.
• Run apm audit --ci --no-policy --format json; expect 9/9 checks.

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

[Copilot]

Pull request overview

This PR restores full Azure DevOps (ADO) dependency coordinate fidelity across lockfile write/read/reconstruction, and threads the canonical ADO remote URL into semver ref enumeration so apm outdated / bounded apm update can resolve and materialize real tags instead of degrading to unknown.

Changes:

• Persist ado_organization / ado_project / ado_repo in LockedDependency, validate them fail-closed, and reconstruct legacy locks via DependencyReference.canonical_ado_coordinates().
• Thread a canonical remote_url through _maybe_resolve_git_semver -> GitSemverResolver -> RefResolver, with safety checks and cache-keying by effective remote identity.
• Add unit + real-binary integration contracts, plus an architecture-boundary lint guard, to prevent lock reconstruction from reintroducing local ADO parsing.

Show a summary per file

File	Description
src/apm_cli/models/dependency/reference.py	Adds canonical ADO coordinate derivation and enforces coordinate/path consistency when building ADO URLs.
src/apm_cli/deps/lockfile.py	Persists ADO coordinates, validates partial/mismatched states, and reconstructs ADO refs deterministically.
src/apm_cli/install/phases/resolve.py	Passes canonical ADO remote URL into semver resolution.
src/apm_cli/deps/git_semver_resolver.py	Accepts optional remote_url and forwards it to the ref resolver when present.
src/apm_cli/marketplace/ref_resolver.py	Accepts an optional canonical ADO URL, validates it, and includes it in cache identity + bearer retry path.
scripts/lint-architecture-boundaries.sh	Adds a static guard enforcing lock reconstruction routes through DependencyReference and forbids repo_url.split(...) in lockfile reconstruction.
tests/unit/deps/test_lockfile_consumer_contract.py	Extends lock consumer contracts to cover ADO fields and URL-consumer projection.
tests/unit/install/test_git_semver_wiring.py	Verifies ADO semver wiring passes the canonical ADO URL.
tests/unit/install/phases/test_resolve_ref_resolver_reuse.py	Uses a production-shaped ADO semver dep fixture for auth/cache wiring reuse.
tests/unit/marketplace/test_ref_resolver.py	Adds tests for canonical ADO URL usage, unsafe URL rejection, and cache identity.
tests/unit/test_outdated_command.py	Regression test: lock coordinates must reach the real remote-ref consumer and form the expected parsed ADO URL.
tests/integration/test_ado_lock_consumer_contract.py	New real-binary end-to-end ADO lock/outdated/update/audit/convergence contract using hermetic local Git + insteadOf rewrite.
tests/integration/test_architecture_authorities.py	Ensures the ADO coordinate canonical owner is used and guarded by static lint.
tests/quality/critical_suite.toml	Adds the new ADO integration contract to the finite critical suite.
tests/quality/test_test_taxonomy.py	Updates expected finite manifest module count.
docs/src/content/docs/reference/lockfile-spec.md	Documents the three additive persisted ADO-only lock fields.
CHANGELOG.md	Adds an Unreleased/Fixed entry for restored ADO outdated/bounded-update behavior.

Review details

• Files reviewed: 17/17 changed files
• Comments generated: 4
• Review effort level: Low

[danielmeppiel]

APM Review Panel: needs_rework

Blocking CI regression: parse() refactoring silently drops non-ADO validation (virtual-ext rejection, charset checks). Ship after restoring _validate_final_repo_fields and resolving spec-conformance gap.

cc @danielmeppiel -- a fresh advisory pass is ready for your review.

All eight panelists converge on the same root cause: replacing _validate_final_repo_fields with canonical_ado_coordinates in parse() Phase 3 collapses the validation surface for non-ADO hosts to zero. CI confirms it -- test_ssh_nested_group_with_virtual_ext_rejected fails on shard 2. This is not a judgment call; the test is irrefutable evidence that a user-facing promise (rejecting virtual extensions in nested groups) no longer holds on this commit. The fix is surgical and well-understood: call _validate_final_repo_fields for the full validation contract, then extract ADO coordinates separately via canonical_ado_coordinates. The spec-conformance failure (51 uncited lines on critical paths) is a second blocking gate -- Mode B detection requires spec anchors before merge per our conformance contract. On the positive side, the ADO lock-coordinate threading is architecturally sound. The DependencyReference.canonical_ado_coordinates property is the right abstraction; the problem is purely in how parse() was rewired to use it. Once the two blocking items are resolved, this PR is a clean ship.

Dissent. CL-1 and DX-1 both flag error-message actionability as "required/recommended" (gray zone). Python-architect and supply-chain-security did not elevate these to blocking. Sided with "recommended": the error messages are suboptimal but fail-closed -- users hit them only on misconfiguration and can recover by re-running apm install. They should be fixed in a fast follow, not gate this PR after the regression is resolved.

Aligned with: Secure by default (VIOLATED -- restore _validate_final_repo_fields), Governed by policy (VIOLATED -- spec citation needed), Portability by manifest (UPHELD -- ADO lock round-trips correctly).

Growth signal. This fix unblocks enterprise ADO teams who hit silent lock-coordinate drift on apm update. Once the regression is patched, the CHANGELOG line ("ADO lock coordinates now survive outdated + update") is a strong enterprise-trust signal. Consider leading the next release note with the pain statement: "Teams on Azure DevOps no longer lose lock fidelity on routine updates."

Panel summary

Persona	B	R	N	Takeaway
Python Architect	1	2	2	Regression: canonical_ado_coordinates bypasses non-ADO validation; dead isinstance; hostname case gap
CLI Logging Expert	0	1	1	OfflineMissError reports wrong identity; error hint not actionable
DevX UX Expert	0	1	2	Upgrade path seamless; mismatch error needs host/value context
Supply Chain Security Expert	0	0	2	Fail-closed design is sound; no exploit vectors
OSS Growth Hacker	0	0	2	ADO fix is a strong enterprise-trust signal; CHANGELOG solid
Auth Expert	0	0	2	Bearer path correct; hostname nit; path scope advisory
Doc Writer	0	0	1	CHANGELOG and lockfile-spec.md are ship-ready
Test Coverage Expert	1	0	2	CI regression blocking; ADO integration contract is well-covered
Performance Expert	0	0	3	All costs O(1), negligible vs subprocess; cache semantics correct

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

Top 5 follow-ups

1. [Python Architect] (blocking-severity) Restore _validate_final_repo_fields in parse() for non-ADO hosts -- CI shard 2 FAILED: test_ssh_nested_group_with_virtual_ext_rejected. Call _validate_final_repo_fields unconditionally in parse() Phase 3; use canonical_ado_coordinates only for ADO coordinate extraction after validation passes.
2. [Test Coverage Expert] (blocking-severity) Address spec-conformance failure: 51 uncited critical-path lines -- CI spec-conformance job FAILED. Add spec anchors + manifest rows + conformance tests for each cited line, or file explicit waivers per CONFORMANCE.md.
3. [Python Architect] Normalize hostname comparison to case-insensitive in ref_resolver.py:215 -- Consensus across 4 panelists. Fails closed but violates RFC 4343. Fix: .lower() on both sides.
4. [CLI Logging Expert] Make error messages actionable: include actual vs expected values -- Users cannot act on "Rebuild the dependency reference from its declared source" without knowing which reference or what mismatched.
5. [Python Architect] Remove dead isinstance guard in lockfile.py:551 -- dep_ref is always DependencyReference at that point; else branch is unreachable dead code.

Per-persona findings detail

Python Architect

• PA-1 (blocking) reference.py:1960: Non-ADO validation silently dropped -- canonical_ado_coordinates returns (None,None,None) for non-ADO hosts without running _validate_final_repo_fields. CI-confirmed.
• PA-2 (recommended) lockfile.py:551: Dead isinstance guard -- dep_ref is always DependencyReference; else branch is unreachable.
• PA-3 (recommended) ref_resolver.py:216: Hostname comparison case-sensitive -- urlparse lowercases but self._host may not. Use .lower().
• PA-4 (nit) ref_resolver.py:291: OfflineMissError.remote uses owner_repo but cache_key may be remote_url.
• PA-5 (nit) reference.py:2055: canonical_ado_coordinates re-invoked in to_github_url -- O(1) cost, acceptable guard.

Auth Expert

• (nit) ref_resolver.py:215: Hostname case sensitivity -- same as PA-3. Fails closed.
• (nit) ref_resolver.py:226: remote_url path not validated against owner_repo -- safe today, future risk if callers bypass to_github_url.

Test Coverage Expert

• (blocking) CI regression: test_ssh_nested_group_with_virtual_ext_rejected FAILS -- root cause is PA-1.
• (nit) No test for mixed-case hostname comparison in ref_resolver.
• (nit) No test for offline miss with remote_url reporting canonical URL.

Supply Chain Security Expert

• (nit) Hostname case sensitivity -- same as PA-3. Fails closed, not exploitable.
• (nit) remote_url path not validated vs owner_repo -- same as Auth-2. Future defense.

CLI Logging Expert

• (recommended) OfflineMissError remote=owner_repo when cache_key is remote_url -- user action points to wrong identity.
• (nit) GitLsRemoteError hint "Rebuild the dependency reference from its declared source" is not actionable.

DevX UX Expert

• (recommended) Error message "The canonical remote URL does not match the configured host." gives no values to diff -- include parsed hostname and self._host in message.
• (nit) OfflineMissError identity inconsistency.
• (nit) Partial-coordinates error lacks repair guidance.

OSS Growth Hacker

• (nit) CHANGELOG entry could lead with the pain ("ADO packages previously showed unknown") before the fix.
• (nit) Future ADO quickstart section opportunity -- not this PR.

Doc Writer

• CHANGELOG format is consistent and correct -- (closes [BUG] apm outdated / apm update report unknown for Azure DevOps deps — LockedDependency.to_dependency_ref() drops ado_* fields (distinct from #2150 / #2119) #2197; fix(ado): preserve lock coordinates for outdated and update #2226) matches established pattern.
• lockfile-spec.md ado_project and ado_repo Notes could add persistence rationale -- optional, non-bloat.

Performance Expert

• canonical_ado_coordinates in to_github_url: ~1.4us per call, amortized by cache, noise vs subprocess.
• urlparse in _git_url_and_env: ~1.9us per call, guarded by cache.
• Cache key divergence (remote_url vs owner_repo): correct semantic split; no double-fetch today.
• No O(n^2) patterns introduced.

[danielmeppiel]

APM Review Panel: ship_with_followups

PR #2226 lands first-class Azure DevOps lock coordinates through the canonical reference owner, all hosted checks green and the exact-head lifecycle clean; only required human approval remains.

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

This PR extends APM's dependency-reference layer to carry Azure DevOps lock coordinates end-to-end through the single canonical owner in reference.py, rather than adding host-specific parsing across consumers. Every earlier needs-rework finding was folded on this head: full parse validation is restored, the spec waiver is accepted, hostname normalization is in place, error actionability and offline-cache identity are fixed, and the dead branch is gone.

The panel converges cleanly. Exact head f430787c0aa5bb3888c5306c2bea7d413ee65e6f has a green hosted check set, a passing real Consume lifecycle, and a field-drop mutation that fails with Partial Azure DevOps lock coordinates. Backward compatibility is deliberately asymmetric: an all-absent legacy ADO coordinate set is reconstructed, while any partial or mismatched set fails closed.

Dissent. No substantive dissent. Two schema-retry artifacts were discarded: an auth summary contradicted its own grounded clean evidence, and a docs finding referenced an unrelated page outside this diff.

Aligned with: Portable by manifest: ADO lock coordinates now round-trip like other Git dependencies. Secure by default: AuthResolver remains the credential owner and hostile URLs fail closed. Multi-host: ADO routing stays behind one reference owner. Pragmatic as npm: outdated and bounded update now work through persisted state.

Growth signal. First-class ADO coordinate support is a concrete enterprise adoption unlock and is worth naming directly in the next release note.

Panel summary

Persona	B	R	N	Takeaway
Python Architect	0	1	1	Single-owner rule, lock validation, and AC4 guard are correct; track the 2099-line module ceiling.
CLI Logging Expert	0	0	2	Current errors are actionable; two hypothetical wording nits remain.
DevX UX Expert	0	0	0	The real ADO install, outdated, update, audit, and convergence promises are restored.
Supply Chain Security Expert	0	0	2	Fail-closed design is sound; two defense-in-depth test nits remain.
OSS Growth Hacker	0	0	0	The fix is a clear enterprise ADO trust signal.
Auth Expert	0	0	0	AuthResolver ownership, token isolation, and PAT/bearer fallback remain intact.
Doc Writer	0	0	1	Lock schema and changelog are accurate; one touched-page ordering nit is optional.
Test Coverage Expert	0	1	1	Critical surfaces meet the evidence floor; the PR body can map one more URL scenario.
Performance Expert	0	1	2	No runtime regression; document the canonical URL cache-key discipline for future callers.

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

Top 3 follow-ups

1. [Test Coverage Expert] Add a PR-body scenario row for the explicit ADO semver/outdated URL handoff -- coverage is already green, but the mapping improves reviewer and contributor discoverability.
2. [Python Architect] Track a future extraction for reference.py, now 2099/2100 lines -- planned separation is preferable to an emergency split.
3. [Performance Expert] Document that ADO callers must supply remote_url to share the canonical cache slot -- no current caller regresses, but the API invariant should be explicit.

Architecture

classDiagram
    direction LR
    class DependencyReference {
        +ado_organization str
        +ado_project str
        +ado_repo str
        +canonical_ado_coordinates host repo_url
        +to_github_url
        +parse
    }
    class LockedDependency {
        +ado_organization str
        +ado_project str
        +ado_repo str
        +from_dependency_ref
        +to_dependency_ref
        +to_dict
        +from_dict
    }
    class RefResolver {
        +list_remote_refs
        -_git_url_and_env
        -_retry_rejected_ado_pat
    }
    class GitSemverResolver {
        +resolve
    }
    class LockFile {
        +read
        +write
    }
    LockedDependency ..> DependencyReference : reconstructs through canonical owner
    GitSemverResolver ..> RefResolver : passes canonical remote URL
    LockFile *-- LockedDependency : persists

Loading

flowchart TD
    L[LockFile.read] --> D[LockedDependency.to_dependency_ref]
    D --> C{ADO coordinates}
    C -->|all absent| B[Derive through DependencyReference]
    C -->|all present| V[Validate against canonical owner]
    C -->|partial or mismatched| F[Fail closed]
    B --> U[DependencyReference.to_github_url]
    V --> U
    U --> O[apm outdated]
    U --> S[GitSemverResolver]
    S --> R[RefResolver]
    R --> X[apm update]

Loading

Recommendation

Recommend shipping once required human approval lands. Exact head f430787c0aa5bb3888c5306c2bea7d413ee65e6f is green, mergeable, and protected by real lifecycle plus field-drop mutation evidence. None of the three follow-ups needs to land in this PR.

---

Full per-persona findings

Python Architect

• [recommended] reference.py is 2099/2100 lines. Track a follow-up extraction of the ADO-coordinate subsystem; no change is needed for this PR.
• [nit] The all-fields round-trip fixture intentionally pairs an ADO host with host_type="gitlab"; a comment could make that independence test clearer.

CLI Logging Expert

• [nit] The ADO-only repair hint could show the concrete production URL shape instead of <source>.
• [nit] A malformed future caller could render got 'None'; the current canonical caller cannot produce that state.

DevX UX Expert

No findings.

Supply Chain Security Expert

• [nit] The canonical supplied URL guard does not separately reject a non-default port; current DependencyReference validation owns that input.
• [nit] A generic-host RefResolver receiving remote_url lacks a direct defense-in-depth unit test; production only supplies it for ADO.

OSS Growth Hacker

No findings.

Auth Expert

No findings. AuthResolver remains the credential owner; no token reaches a URL; PAT and bearer fallback use the same canonical URL.

Doc Writer

• [nit] In the touched lockfile reference table, the claude_skill version exception could follow the git-semver case for slightly smoother type progression. The schema claims themselves are verified.

Test Coverage Expert

• [recommended] Add one Scenario Evidence row for the explicit ADO semver and outdated URL handoff. The tests already pass at unit and E2E tiers.
• [nit] No action: the YAML reload to URL assertion is already explicit in the unit contract and repeated in the real lifecycle test.

Performance Expert

• [recommended] Document the remote_url cache-key invariant for future ADO callers.
• [nit] Canonical coordinate revalidation is linear in a short repository path and negligible next to Git subprocess work.
• [nit] Non-ADO lock reconstruction performs one constant-time host classification per dependency.

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

[danielmeppiel]

APM Review Panel: needs_discussion

PR #2226 is a green Layer 3 preparation over accepted #2186 and #2229. The code is prepared; predecessor ordering is the remaining decision boundary.

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

Exact head 6ef4bcd8ebcd603679718dcc62843bf4ba438eca merges current main 4b646c2d66cbb197e03faf20774d136a46a5e24a, accepted #2186 head a3ec80aa3d56c4cee32e9c0b2a89277d0e7de12a, and accepted #2229 head a7c143d4c632e1d1610e5cb6d60effecd0438ec6. Hosted CI is green. Local evidence includes 19,121 full-corpus passes, 6 stacked critical lifecycle passes, 155 focused resolver/lock tests, 62 quality/architecture passes, 140 spec passes with 2 skips, a clean audit, and AC1-AC14.

The semantic merge preserves each owner: main's marketplace parser and repository-cache identity remain AC10/AC11; Codex printable diagnostics are AC12; TransportSelector -> RefResolver is AC13; and ADO lock projection through DependencyReference is AC14. The consensus UX folds are applied: the default header now says agent conversion warning, the ADO SSH hint names accepted coordinate shapes, and the SSH CHANGELOG entry uses the surrounding declarative style.

Dissent. The performance persona repeatedly missed the return schema. Its grounded analysis found transport/cache behavior correct and raised only a hypothetical sequential network path whose common case is lock-seeded dictionary lookup. The panel keeps that as a separate predecessor-level performance question, not a signal against this preparation.

Aligned with: Secure by default: SSH clears HTTP auth and ADO HTTPS validates a credential-free canonical URL. Governed by policy: taxonomy is exactly 16 and AC1-AC14 are green. Multi-host: SSH and ADO share one semver owner without collapsing transport identity. Pragmatic as npm: lock replay, outdated, bounded update, and actionable diagnostics converge.

Growth signal. Once the stack lands in order, the release story is enterprise source reliability across ADO, SSH, GitLab, and Codex conversion rather than three isolated implementation fixes.

Panel summary

Persona	B	R	N	Takeaway
Python Architect	0	1	2	Canonical owners are preserved; diagnostic category consolidation is a separate semantic choice.
CLI Logging Expert	0	2	1	Both consensus wording recommendations are folded.
DevX UX Expert	0	1	2	Default agent-conversion wording is now package-manager friendly.
Supply Chain Security Expert	0	0	3	No exploit or auth bypass; three defense-in-depth nits remain.
OSS Growth Hacker	0	0	1	CHANGELOG style is folded; enterprise reliability is the release angle.
Auth Expert	0	0	2	SSH/HTTPS credential boundaries remain correct.
Doc Writer	0	0	0	Docs are accurate, in-place resolved, and have no README drift.
Test Coverage Expert	0	0	0	Every critical user promise has evidence at or above its floor.
Performance Expert	0	0	0	Schema failure; grounded pre-return analysis found no current regression.

B = high-severity findings, R = recommended, N = nits.
Counts are advisory signal. The maintainer ships.

Follow-ups

1. [Coordinator] Merge fix(cli): warn when Codex agent tool scope is dropped #2186 and fix(deps): honor SSH for semver ref resolution #2229 before reconsidering fix(ado): preserve lock coordinates for outdated and update #2226, then resolve only residual tree differences.
2. [Python Architect] Weigh diagnostic-category consolidation separately; it changes semantics rather than wording.
3. [Performance Expert] Re-run the persona after its schema issue is fixed and evaluate annotate_update_plan_refs on the predecessor branch.
4. [Supply Chain Security] Consider eliminating the ADO SSH coordinate split parser when the transport owner can receive validated coordinates directly.

Recommendation

Freeze as prepared-waiting-on-#2186/#2229. Do not treat the green hosted state as independent landing permission. After both predecessors merge, fetch exact main, preserve all 16 critical rows and AC1-AC14, resolve only residual differences, and rerun hosted CI plus the two panels.

---

Evidence and deferred findings

• ADO legacy all-absent coordinates reconstruct; any partial coordinate set fails closed.
• Dropping ado_repo on the stacked head fails the round-trip regression test.
• SSH and HTTPS resolver instances use distinct transport-aware cache identities.
• ADO PAT/bearer fallback remains disabled for SSH and preserves the canonical URL for HTTPS.
• Broad category, parallelization, and owner-extraction refactors are deferred to predecessor-specific work.

_{This panel is advisory. Re-apply the panel-review label after predecessor integration to re-run.}

[danielmeppiel]

APM Spec Guardian: fold_and_ship

Scope: inherited editorial-patch; Layer 3 predecessor-relative spec diff = +0/-0 lines. The cumulative PR carries the independently verified #2186/#2229 spec union.

Exact head d3a9e987eda473952bc8bef92762866ad692d5f9 is byte-identical to immutable predecessor 7d231599bb647871717cfe9cdbdb68ae515e7ac9 for the OpenAPM spec, requirements manifest, and generated conformance artifacts. No ADO-layer normative statement, anchor, count, or fixture is added. The cumulative artifact remains at 100 requirements (95 MUST, 5 SHOULD), and hosted spec conformance is green.

Evidence

• git diff 7d231599...HEAD -- docs/src/content/docs/specs/openapm-v0.1.md is empty.
• tests/spec_conformance: 140 passed, 2 skipped.
• Hosted Spec conformance gate: SUCCESS.
• Requirement anchors, requirements-manifest IDs, CONFORMANCE rows, and JSON requirements remain inherited from the immutable predecessor.
• The ADO layer uses an explicit apm-spec-waiver: because it composes existing resolver/lock requirements without adding normative semantics.

Recommendation

Preserve the predecessor artifact exactly. After #2229 lands, re-probe the tree; any non-empty residual spec diff should trigger a fresh guardian run before #2226 is reconsidered.

_{This panel is advisory. Re-apply the spec-review label only if predecessor integration creates a residual spec diff.}

[danielmeppiel]

APM Review Panel: needs_discussion

PR #2226 is released against immutable #2229 and green. The remaining discussion is predecessor landing order, not implementation rework.

cc @sergio-sisternes-epam -- exact-head release evidence is ready for review.

Exact head d3a9e987eda473952bc8bef92762866ad692d5f9 has current main 4b646c2d66cbb197e03faf20774d136a46a5e24a and immutable #2229 7d231599bb647871717cfe9cdbdb68ae515e7ac9 as ancestors; #2229 already contains #2186 3d2828c88aa59880258ed0b38e9ca61e6c13daa6. GitHub's cumulative files API reports 37 inherited files. The predecessor-relative net diff is 18 files containing ADO lock/URL/tests/docs plus the necessary HTTPS handoff in the finalized semver owner.

The layering contract is exact: predecessor critical_suite.toml has 15 rows; #2226 adds only test_ado_lock_consumer_contract.py for 16, and TM002 asserts 16. Main AC10 marketplace and AC11 repository-cache, predecessor AC12 diagnostics and AC13 Git ref transport, and ADO AC14 are all present. Predecessor-owned Codex, SSH, marketplace/cache, and OpenAPM files are byte-preserved relative to 7d231599....

Hosted CI is green with 0 failed and 0 pending checks. Local evidence: 170 focused tests, 62 quality/architecture tests, 140 spec passes with 2 skips, 6 critical lifecycles, 19,122 full-corpus passes, clean audit, exact AC1-AC14 lint, and a field-drop mutation that fails closed when ado_repo forwarding is removed.

Aligned with: Secure by default: partial coordinates fail closed and credentials stay outside URLs. Governed by policy: 15->16 and AC10->AC14 are mechanically guarded. Multi-host: TransportSelector remains the single transport decision owner; ADO contributes only the canonical HTTPS /_git/ URL. Pragmatic as npm: install, persisted lock replay, outdated, bounded update, audit, and convergence are covered through public commands.

Panel summary

Lens	Result
Architecture	Canonical owner union preserved; predecessor-relative diff contains no duplicate transport/cache/coordinate authority.
Auth and supply chain	SSH clears HTTP auth; ADO PAT/bearer retry remains HTTPS-only; hostile canonical URLs fail closed.
DevX and logging	ADO status/update behavior and actionable errors remain covered; predecessor diagnostics are unchanged.
Docs	Lockfile reference is the only non-predecessor docs delta; README has no drift.
Test coverage	Every ADO promise has unit plus real-binary lifecycle evidence; mutation receipt is current.
Performance	Resolver/cache topology matches immutable #2229; ADO adds no extra SSH round-trip.

Recommendation

Freeze as predecessor-wait. Do not merge #2226 before #2229. Once #2229 lands, fetch exact main and resolve only residual tree differences; preserve the 15->16 proof and AC1-AC14, then rerun exact-head hosted CI.

_{This panel is advisory. Re-apply the panel-review label after predecessor landing if residual differences remain.}

[danielmeppiel]

APM Review Panel: ship_now

Fixes ADO lock coordinate preservation so outdated-and-update cycles no longer silently corrupt lockfile coordinates, closing a real ADO user regression.

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

All panel signals converged: the fix is correct, bounded, and non-breaking. This shepherd pass folded the actionable follow-ups in-PR: ADO SSH coordinate extraction now routes through DependencyReference.canonical_ado_coordinates, supplied ADO HTTPS remote_url values are validated against the same canonical coordinates before git ls-remote, ADO HTTPS auth is documented as credential-free URL plus http.extraheader, and corrupt ADO coordinate errors now include recovery guidance. CI is green on 017fca7.

Aligned with: Portable by manifest: ADO lock entries remain a faithful portable record; Secure by default: supplied URLs must match canonical coordinates; Multi-host support: ADO SSH and HTTPS share the same coordinate owner; Pragmatic as npm: recovery guidance replaces bare coordinate errors.

Panel summary

Persona	B	R	N	Takeaway
Python Architect	0	2	1	Canonical ADO coordinate authority is now used across lock and ref resolution.
CLI Logging Expert	0	0	2	Error messages now include actionable ADO lock recovery guidance.
DevX UX Expert	0	1	1	ADO outdated/update user promise is restored; corrupt-lock guidance improved.
Supply Chain Security Expert	0	0	2	ADO remote URLs are host/path validated and credential-free.
OSS Growth Hacker	0	0	1	ADO enterprise support story is stronger; docs are adequate.
Auth Expert	0	1	2	ADO auth remains delegated to AuthResolver/git env; no token leakage.
Doc Writer	0	0	1	Lockfile spec and changelog are accurate and scoped.
Test Coverage Expert	0	0	3	Critical ADO lifecycle and URL-safety surfaces are covered.
Performance Expert	0	1	3	No measured perf regression; one future-proofing refactor can wait.

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

Top follow-ups

1. [Performance Expert] Thread TransportPlan once per resolution pass instead of rebuilding per dependency -- future-proofing only; no measurable regression today.
2. [Docs / Growth] Optional changelog voice polish to lead with the user-facing ADO outdated / update outcome -- cosmetic only.

Folded in this run

• (panel) Routed ADO SSH coordinate extraction through DependencyReference.canonical_ado_coordinates and extended AC14 static/architecture guards -- resolved in 32daed93418a6e348e19ce573a0fc673dce6bb73.
• (panel) Validated supplied ADO HTTPS remote_url paths against canonical dependency coordinates before git ls-remote -- resolved in 32daed93418a6e348e19ce573a0fc673dce6bb73.
• (panel) Documented that ADO HTTPS keeps credentials out of the URL and uses git http.extraheader auth -- resolved in 32daed93418a6e348e19ce573a0fc673dce6bb73.
• (panel) Made corrupted ADO lock/reference coordinate errors include concrete recovery guidance -- resolved in 32daed93418a6e348e19ce573a0fc673dce6bb73.
• (panel) Kept the dependency reference module within the CI file-length guard after the shepherd fold -- resolved in 017fca75aa6e1a5776e1f7243606567f242777b9.

Copilot signals reviewed

No inline comments from copilot-pull-request-reviewer[bot] were present when fetched during this run.

Deferred (out-of-scope follow-ups)

• (panel) Thread TransportPlan once per resolution pass -- scope boundary: PR scope is ADO lock coordinate correctness; this is a future-proofing performance refactor with no measured regression.
• (panel) Polish changelog wording -- scope boundary: current changelog entry is accurate; this is release-note voice polish outside the correctness fix.

Regression-trap evidence (mutation-break gate)

• tests/unit/marketplace/test_ref_resolver.py::TestRefResolver::test_ado_remote_url_rejects_unsafe_variants -- deleted parsed_remote.path != expected_ado_path; test FAILED as expected; guard restored.

Lint contract

uv run --extra dev ruff check src/ tests/ and uv run --extra dev ruff format --check src/ tests/ both silent. Pylint R0801, auth-signal lint, and scripts/lint-architecture-boundaries.sh also passed.

CI

gh pr checks 2226 --repo microsoft/apm reports all checks passing on 017fca75aa6e1a5776e1f7243606567f242777b9 after 1 CI recovery iteration (initial file-length guard failure fixed by 017fca75).

Mergeability status

Captured from gh pr view 2226 --repo microsoft/apm --json number,headRefOid,mergeable,mergeStateStatus,statusCheckRollup immediately after the last push of this run.

PR	head SHA	CEO stance	iters	folds	defers	Copilot rounds	CI	mergeable	mergeStateStatus	notes
#2226	017fca7	ship_now	1	5	2	2	green	MERGEABLE	BLOCKED	pending required review

Convergence

1 outer iteration; 2 Copilot rounds. Final panel stance: ship_now.

Ready for maintainer review.

_{This panel is advisory. It does not block merge.}

[danielmeppiel]

APM Review Panel: ship_now

Provider-neutral ADO lock replay restores outdated and update behavior with fail-closed URL validation and no measurable performance regression.

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

Nine specialist lenses converged with zero blocking-severity findings. The architecture keeps LockedDependency and YAML provider-neutral, derives transient transport coordinates through the DependencyReference model boundary, and protects that routing with AC14 plus unit, integration, and real-binary coverage.

The three actionable in-scope findings were folded before this comment: ADO resolver errors now name the repository, the provider-coordinate error includes repo_url, and the duplicate inherited #2229 changelog line is removed. The remaining items are bounded follow-ups or nits.

Aligned with: portable by manifest - generic host + repo_url lock identity works across providers; secure by default - canonical URL checks remain fail closed and credentials stay outside URLs; multi-harness/multi-host - ADO now follows the same lock consumer contract as other Git hosts.

Growth signal. Provider-neutral lock identity is a useful release-note proof point: one generic lock schema supports multiple forges without provider metadata.

Panel summary

Persona	B	R	N	Takeaway
Python Architect	0	1	2	Clean single-owner extraction; mixin is bounded and AC14 supplies the dual guardrail.
CLI Logging Expert	0	1	1	ADO errors should name the dependency; folded at exact head.
DevX UX Expert	0	0	2	Provider-neutral lock semantics match package-manager expectations.
Supply Chain Security Expert	0	0	1	Generic identity, URL validation, credential handling, and commit integrity remain sound.
OSS Growth Hacker	0	0	2	The provider-neutral story is accurate and concise.
Auth Expert	0	0	2	Bearer/basic fallback and credential-free URL handling remain intact.
Doc Writer	0	1	0	Documentation is accurate; duplicate inherited changelog entry was folded.
Test Coverage Expert	0	1	1	Unit, architecture, and real-binary tiers cover all stated promises.
Performance Expert	0	0	0	New work is O(1) per dependency and network-neutral.

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

Top 4 follow-ups

1. [OSS Growth Hacker] Lead release communication with the restored user outcome -- this is a useful multi-forge proof point.
2. [Python Architect] Split maybe_resolve_git_semver only if it gains callers or complexity -- avoid speculative refactoring now.
3. [Test Coverage Expert] Consider a committed mutation fixture -- the current regression assertion and manual mutation already catch the exact omission.
4. [Supply Chain Security Expert] Quote the ADO organization segment for symmetry if its accepted character set ever widens -- current validation makes it URL-safe.

Architecture

classDiagram
    direction LR
    class ProviderCoordinateMixin {
        +validate_provider_coordinates()
        +is_transient_provider_field(field_name)
        +with_derived_provider_coordinates()
    }
    class DependencyReference {
        +repo_url
        +host
        +canonical_ado_coordinates(host, repo_url)
        +to_github_url()
    }
    class LockedDependency {
        +repo_url
        +host
        +to_dependency_ref()
        +from_dict(data)
    }
    DependencyReference --|> ProviderCoordinateMixin : inherits
    LockedDependency ..> DependencyReference : reconstructs

Loading

flowchart TD
    Y["apm.lock.yaml: host + repo_url"] --> L[LockedDependency]
    L --> D[DependencyReference]
    D --> C["derive transient ADO coordinates"]
    C --> V["validate canonical identity"]
    V --> U["build credential-free /_git/ URL"]
    U --> O["outdated and update"]

Loading

Recommendation

All panel lenses found the provider-neutral redesign sound, and the actionable review items were folded at exact head 8f820cc496. Ship after exact-head CI completes; track only the bounded follow-ups that earn separate scope.

---

Full per-persona findings

Python Architect

• [recommended] Split maybe_resolve_git_semver if it grows or gains another caller. The current function is tested and has one caller, so no speculative split is needed in this PR.
• [nit] The mixin uses NotImplementedError instead of an ABC/Protocol. One concrete inheritor makes this acceptable today.
• [nit] Provider coordinates are validated at lock creation and URL rendering. These protect different transitions and remain O(1).

CLI Logging Expert

• [recommended, folded] Newly added ADO GitLsRemoteError paths used an empty package identity. Exact head now passes owner_repo.
• [nit, folded] Provider-coordinate errors should name repo_url. Exact head now does.

DevX UX Expert

• [nit, folded] Name the dependency in provider-coordinate recovery guidance.
• [nit] Existing lockfile-spec link repairs are unrelated housekeeping inherited by the cumulative branch.

Supply Chain Security Expert

• [nit] The organization path segment is not quoted while project and repository are. Current validation restricts it to URL-safe characters.

OSS Growth Hacker

• [nit] Release communication can lead with the user outcome rather than transport implementation.
• [nit] A short portability hook may help readers connect the lock design to multi-forge use.

Auth Expert

• [nit] A non-ADO caller supplying remote_url receives an ADO-specific rejection message; it still fails closed.
• [nit] Quote the organization path segment for symmetry if validation ever widens.

Doc Writer

• [recommended, folded] Remove the duplicate inherited fix(deps): honor SSH for semver ref resolution #2229 changelog entry.

Test Coverage Expert

• [recommended] The real-binary lifecycle satisfies the integration floor for lock determinism; no coverage gap remains.
• [nit] A committed mutation fixture could make the manual mutation receipt self-running, but the exact regression assertion already fails when derivation is removed.

Performance Expert

No findings.

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

[danielmeppiel]

APM Review Panel: ship_now

Restores ADO lock round-trip fidelity so apm outdated and bounded apm update resolve real Azure DevOps versions without provider-specific lock fields.

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

The panel converged on ship: the PR keeps apm.lock.yaml provider-neutral, reconstructs transient ADO coordinates through the dependency-reference model boundary, and preserves auth-safe ADO transport selection through http.extraheader. Shepherding folded the in-scope follow-ups from the first pass: direct mismatch coverage, ABC enforcement on the mixin contract, command-shaped recovery text, RefResolver cache-contract docs, changelog wording, and refreshed exact-head PR evidence.

Aligned with: portable lockfiles stay provider-neutral; secure transport rejects credential-bearing or mismatched ADO URLs; multi-host support now treats ADO as first-class alongside GitHub/GitLab; DevX restores expected outdated / update behavior without new flags.

Panel summary

Persona	B	R	N	Takeaway
Python Architect	0	0	2	Single-owner ADO coordinate derivation is clean and guarded.
CLI Logging Expert	0	0	1	Recovery text is now concrete and actionable.
DevX UX Expert	0	0	1	No CLI surface change; ADO users get expected behavior back.
Supply Chain Security Expert	0	0	1	URL validation remains fail-closed and credential-safe.
OSS Growth Hacker	0	0	1	ADO parity strengthens the vendor-neutral lockfile story.
Auth Expert	0	0	1	ADO credential injection remains on the existing safe path.
Doc Writer	0	0	0	Docs, changelog, and PR body now align with the current head.
Test Coverage Expert	0	0	0	Lockfile, resolver, architecture, and mismatch paths are covered.
Performance Expert	0	0	1	No material hot-path regression; added checks are O(1) per dep.

Folded in this run

• (panel) Add unit regression test for validate_provider_coordinates() mismatched ADO fields -- resolved in 3697a6e42a43d850a266cd71fd0c1cea1f88cdcd.
• (panel) Promote ProviderCoordinateMixin.canonical_ado_coordinates to abc.abstractmethod -- resolved in 3697a6e42a43d850a266cd71fd0c1cea1f88cdcd.
• (panel) Make provider-coordinate mismatch recovery text include an apm install command -- resolved in 3697a6e42a43d850a266cd71fd0c1cea1f88cdcd.
• (panel) Sharpen the CHANGELOG ADO entry around apm outdated / apm update user benefit -- resolved in 3697a6e42a43d850a266cd71fd0c1cea1f88cdcd.
• (panel) Document the RefResolver remote_url cache-identity consistency contract -- resolved in 3697a6e42a43d850a266cd71fd0c1cea1f88cdcd.
• (panel) Refresh the PR body exact-head notice and hosted-run evidence after shepherd folds -- resolved in 3697a6e42a43d850a266cd71fd0c1cea1f88cdcd.

Copilot signals reviewed

• review-4705343839 -- NOT-LEGIT: overview-only review was stale after folds; no inline Copilot comments were returned by the GitHub API.

Regression-trap evidence (mutation-break gate)

• tests/unit/deps/test_lockfile_consumer_contract.py::test_validate_provider_coordinates_rejects_mismatched_ado_fields -- deleted provider_coordinates.py mismatch guard; test FAILED as expected; guard restored.

Lint contract

uv run --extra dev ruff check src/ tests/ and uv run --extra dev ruff format --check src/ tests/ both silent. The pylint R0801, auth-signal, remaining grep-equivalent guards, and architecture boundary lint also passed.

CI

gh pr checks 2226 --repo microsoft/apm is green on head 3697a6e42a43d850a266cd71fd0c1cea1f88cdcd (CI 29479999500, CodeQL 29479999474, Merge Gate 29479999532, Spec 29479999465, Docs 29479999462, NOTICE 29479999478) after 0 CI fix iterations.

Mergeability status

Captured from gh pr view 2226 --json mergeable,mergeStateStatus,statusCheckRollup immediately after the last push of this run.

PR	head SHA	CEO stance	iters	folds	defers	Copilot rounds	CI	mergeable	mergeStateStatus	notes
#2226	3697a6e	ship_now	1	6	0	2	green	MERGEABLE	BLOCKED	pending required review

Convergence

1 outer iteration; 2 Copilot rounds. Final panel stance: ship_now.

Ready for maintainer review.