Skip to content

fix(git): preserve symlink capability config precedence - #3145

Merged
Daniel Meppiel (danielmeppiel) merged 11 commits into
mainfrom
danielmeppiel-issue-delivery-3137
Oct 6, 2026
Merged

Daniel Meppiel (danielmeppiel) merged 11 commits into
mainfrom
danielmeppiel-issue-delivery-3137

Conversation

@danielmeppiel

@danielmeppiel Daniel Meppiel (danielmeppiel) commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

Description

fix(git): preserve symlink capability config precedence

TL;DR

Keep Git's effective core.symlinks value when APM freezes configuration for a Git child. Repository-local capability detection now survives an inherited global/system value, while explicit command-scope overrides still win. Authentication, URL-rewrite isolation, other settings, and containment are unchanged.

Important

Native non-admin Windows acceptance is now verified at c5bd885a5dea65b0f5b8ab17a4987cc63673e921: baseline 29 passed, the old-precedence mutation failed as expected, and restoration passed. The hosted run retained actual token, capability, JUnit and cleanup evidence. This remains a draft: required Microsoft SDL analysis and protected merge policy are separate from that successful observation. Exact-head owner/functional qualification passes. The fresh engineering advisory is separate from protected merge permission; no review bypass or merge is claimed.

Problem (WHY)

  • On base 7dfc5dd74e, real Git resolves inherited core.symlinks=true plus repository-local false as local false. APM's materializer instead produces command true.
  • The clone probe already obtains Git's repository-local capability result, but the materializer drops non-network local entries while promoting inherited entries. The new real-checkout regression fails on that behavior.
  • [!] Retaining every repeated symlink value would still be wrong: existing first-occurrence deduplication can turn true, false, true into true, false, discarding explicit command intent.

Evidence includes real local Git and actual Windows execution, following the Agent Skills validation loop: "do the work, run a validator (a script, a reference checklist, or a self-check), fix any issues, and repeat until validation passes."

Approach (WHAT)

  1. Consume the existing validated configuration snapshot; retain the effective core.symlinks value once, keeping command intent above file entries even when parent and isolated-child snapshots are merged.
  2. Include local/worktree values for this setting only, preserving the capability result without extending unrelated local configuration replay.
  3. Leave auth fences, URL checks, configuration-file isolation, and containment paths untouched.
  4. Document that link-text fallback is not equivalent to real-symlink package semantics.

Implementation (HOW)

File Change
src/apm_cli/utils/git_env.py Retain one effective symlink value through the existing materializer and its pure selection helper; no new production config owner or persistent file.
tests/unit/cache/test_git_symlink_config.py 28 real-Git precedence, checkout and mixed-auth preservation cases, including isolated-child and repeated command intent.
tests/unit/cache/test_git_symlink_precedence.py Six pure helper-precedence cases; the removed-local-priority mutation produces two expected failures.
tests/integration/test_windows_native_symlink.py Real denied-capability standard-user Git/APM parity and causal in-memory mutation; no mocked Windows privilege result.
scripts/windows_native_standard_user_symlink_gate.ps1 Bounded disposable hosted account/registry/scratch setup with independent, failure-safe restoration.
scripts/windows_native_symlink_probe_entry.py Exact source/interpreter binding, real token/error-1314 qualification, baseline/mutation/restored phases and strict JUnit verification.
tests/unit/test_windows_native_symlink_probe.py, tests/unit/test_windows_native_symlink_workflow.py Fail-closed helper/workflow controls and real-Git checkout-EOL/config-isolation regression.
.github/workflows/ci.yml Exact-head Windows job, checkout-only LF normalization, frozen setup, guaranteed cleanup/artifact steps and helper lint coverage.
tests/integration/conftest.py, pyproject.toml One canonical native prerequisite marker; no dependency changes.
docs/src/content/docs/consumer/install-packages.md Troubleshooting guidance for capability detection and link-text fallback limits.
docs/src/content/docs/contributing/integration-testing.md Native marker, evidence and cleanup contract.
packages/apm-guide/.apm/skills/apm-usage/dependencies.md Matching guidance in the shipped agent skill resource.

Architecture classification: owner-extension for the canonical decision Git child-process repository location and URL rewrite safety, still owned by utils/git_env.py. Clone/network consumers retain that authority; no centralization, new owner or routing repair is introduced. Existing architecture boundaries pass; canonical exact-head terminal qualification is tracked separately from static lint.

Diagram

The dashed node is the only changed stage; the existing probe and isolation paths remain in place.

flowchart LR
    subgraph Probe["Existing target probe"]
        I["git_clone_env: git init"]
        S["Ordered Git config snapshot"]
        I --> S
    end
    subgraph Freeze["Existing config materializer"]
        E["Keep final core.symlinks value"]
        F["Unchanged auth and URL filtering"]
        S --> E
        S --> F
    end
    subgraph Execute["Git child"]
        C["Isolated environment"]
        G["git clone and checkout"]
        E --> C
        F --> C
        C --> G
    end
    classDef changed stroke-dasharray: 5 5;
    class E changed;
Loading

Trade-offs

  • Preserve the value already selected by Git, rather than forcing symlinks on/off or reopening global/system configuration after validation.
  • Limit the repair to core.symlinks; do not redesign precedence for unrelated multi-valued Git settings.
  • The portable regression injects the unavailable-capability result after real git init, then performs real checkout. It proves propagation, not Windows privilege detection. The separate native test mocks neither Git nor capability.
  • The native job temporarily changes only an owned disposable hosted-runner fixture. It does not change user machines or persistent Git configuration; both cleanup passes are evidenced. The first hosted attempt failed provenance before qualification and is not counted as acceptance.
  • No archive-symlink materialization, containment relaxation, README change, or release/changelog claim is included. Packages requiring genuine symlinks still need compatible sources or an appropriate environment.

Benefits

  1. Repository capability decisions survive both inherited system and global configuration.
  2. Explicit indexed and GIT_CONFIG_PARAMETERS command intent survives repeated values.
  3. A real checkout with the unavailable-capability result produces link text while preserving the actual target file.

Issue and approved scope

Issue: #3137

Human scope-approval comment: #3137 (comment)

This implements the bounded repair, regression coverage, guidance and observed native acceptance. The 2026-10-06 engineering run folds the three genuine documentation findings in c5bd885a: a portable shipped-skill link, the correct agent-source reference and focused recovery prose. No runtime or native-fixture code changed in that commit. Current-head engineering advice and owner qualification are recorded separately in the advisory. Legacy SDL analysis and applicable human/queue policy remain separate provider requirements, not waived by engineering completion. The PR stays draft and no automatic closing keyword is used.

The current unedited scope record supersedes the earlier nomination for this renewed run. Actual user delegation and coordinator-approved plans aa27770d/8aa39b7e, not the record alone, authorize the bounded disposable-runner work. Earlier receipts remain historical. Review contact: Daniel Meppiel (@danielmeppiel); existing CODEOWNERS and reviewer state are unchanged.

Type of change

  • Bug fix
  • New feature
  • Documentation
  • Maintenance / refactor

Testing

  • Tested locally
  • All existing tests pass
  • Added tests for new functionality (if applicable)

Validation

Evidence below is bound to head c5bd885a5dea65b0f5b8ab17a4987cc63673e921, with main 18c4c43c924ceae890fe0f2038806690e5b2d6c8 incorporated. CI run 37440135815, including native Windows acceptance and provider Lint, succeeded. All ordinary current-head checks completed successfully or with neutral/skipped outcomes, including gate, Spec conformance, both CodeQL analyses and docs build. Protected merge conditions remain separate; the historical missing legacy SDL analysis is not replaced by default CodeQL.

The previous body's "sole merge blocker" statement remains withdrawn. Earlier results and attempts are retained as history, not relabelled as current-head evidence. This fresh run reexecuted the local functional suite, full lint mirror and four causal mutations, then downloaded and verified a new native artifact at c5bd885a. The first new native job at e7dd8272 failed source provenance before capability qualification; both cleanup paths succeeded. Recovery 1 fixed checkout EOL/config-isolation compatibility without accepting dirty sources. No amend, force push, global Git change or lockfile churn was used.

Exact commands and results
Command Result
uv run --frozen --extra dev python -m pytest -p no:cacheprovider -q --basetemp /Users/danielmeppiel/.copilot/session-state/cf24cbf0-c318-4853-a131-4be723d1cea8/files/pr-3145-evidence/pytest-functional --junitxml /Users/danielmeppiel/.copilot/session-state/cf24cbf0-c318-4853-a131-4be723d1cea8/files/pr-3145-evidence/functional.xml tests/unit/test_windows_native_symlink_probe.py tests/unit/test_windows_native_symlink_workflow.py tests/unit/cache/test_git_symlink_precedence.py tests/unit/cache/test_git_symlink_config.py tests/unit/cache/test_git_env.py tests/unit/deps/test_git_auth_env.py tests/unit/deps/test_clone_engine_bearer_env.py tests/integration/test_git_hook_env_isolation.py tests/integration/test_generic_https_credential_env_e2e.py tests/integration/test_architecture_io_guards.py tests/integration/test_windows_native_symlink.py 225 passed, 1 skipped in 12.42s. The macOS native skip is not acceptance evidence.
uv run --frozen --extra dev ruff check src/ tests/ scripts/lint_architecture_boundaries.py scripts/architecture_linter/ scripts/windows_native_symlink_probe_entry.py Passed: All checks passed!
uv run --frozen --extra dev ruff format --check src/ tests/ scripts/lint_architecture_boundaries.py scripts/architecture_linter/ scripts/windows_native_symlink_probe_entry.py Passed: 1916 files already formatted
uv run --frozen --extra dev python -m pylint --disable=all --enable=R0801 --min-similarity-lines=10 --fail-on=R0801 src/apm_cli/ scripts/lint_architecture_boundaries.py scripts/architecture_linter/ scripts/windows_native_symlink_probe_entry.py Passed: exit 0.
bash scripts/lint-auth-signals.sh Passed: [+] auth-signal lint clean
bash scripts/lint-architecture-boundaries.sh Passed: exit 0.
cd docs && npm run test:links Passed: 14 tests, zero failures.
./scripts/windows_native_standard_user_symlink_gate.ps1 -ExpectedHead $env:APM_NATIVE_EXPECTED_HEAD on the hosted Windows runner Passed: exact-head baseline 29 cases, expected mutation exit 1, restored native case passed; no skipped/error cases.
./scripts/windows_native_standard_user_symlink_gate.ps1 -CleanupOnly Passed, independently after the primary cleanup.

YAML I/O, 2100-line and portable-path guards passed through equivalent Python regex/line checks on macOS, then the actual provider Lint job passed. The unchanged diagram passed installed mmdc -i body-diagram.mmd -o body-diagram.svg --quiet.

The native artifact windows-native-symlink-1 (ID 11400587845) records standard-user SID ending 1003, distinct from runner SID ending 500, no administrator group/token or symlink privilege, successful ordinary I/O and actual error 1314. All three native JUnit nodes record the same qualified context. The mutation records ["global","true","local","false",true,false] and the real APM Git clone's expected symlink failure; restoration passes. Primary and independent cleanup both report success with no failures.

The full local repository/integration suite was not run. The local command above and actual provider jobs are the evidence, not the unchecked blanket checklist claim.

Scenario Evidence

# Scenario (user promise) Principle(s) Test(s) proving it Type
1 An inherited symlink preference cannot override Git's repository capability decision. DevX (pragmatic as npm) tests/unit/cache/test_git_symlink_config.py::test_network_env_preserves_effective_symlink_setting (regression-trap for #3137) Integration with real local Git; pytest component
2 Explicit command overrides and unrelated compression settings still work; user config files remain unchanged. DevX (pragmatic as npm), Secure by default Same parametrized precedence test; tests/unit/cache/test_git_symlink_config.py::test_isolated_child_preserves_parent_command_intent Integration with real local Git
3 Unavailable-capability checkout writes link text, not substituted target contents. DevX (pragmatic as npm) tests/unit/cache/test_git_symlink_config.py::test_clone_respects_unavailable_symlink_capability Integration; capability result simulated, checkout real
4 Without Windows symlink rights, APM matches native Git's link-text fallback without modifying global settings. Vendor-neutral, DevX (pragmatic as npm) tests/integration/test_windows_native_symlink.py::test_native_standard_user_symlink_fallback (regression-trap for #3137) Actual hosted Windows integration; baseline/mutation/restored, non-skipped
5 Credential/URL isolation and hook-environment filtering remain intact. Secure by default tests/unit/cache/test_git_env.py, tests/integration/test_generic_https_credential_env_e2e.py, tests/integration/test_git_hook_env_isolation.py Existing unit/integration contracts

How to test

  • Run uv run --frozen --extra dev pytest -q -n0 tests/unit/cache/test_git_symlink_config.py; expect 28 passing cases.
  • Inspect the linked native job and retained artifact: require baseline 29 pass, one intentional mutation failure, restored pass, identical qualified context and both cleanup records. Do not enable the native marker manually or run the hosted fixture on a user machine.
  • Run the relevant-suite command above; expect the auth, URL, hook, and architecture contracts to remain green.
  • Review the documentation and usage resource: link-text fallback must not be presented as general support for real-symlink package semantics.

Spec conformance (OpenAPM v0.1)

If this PR changes behaviour that an OpenAPM v0.1 req-XXX covers,
confirm the three-step ritual in the
development guide:

  • Spec edit: docs/src/content/docs/specs/openapm-v0.1.md updated
    (new/changed <a id="req-XXX"></a> anchor + prose + Appendix C
    row).
  • Manifest edit: docs/src/content/docs/specs/manifests/openapm-v0.1.requirements.yml
    updated.
  • Test edit: a @pytest.mark.req("req-XXX") test under
    tests/spec_conformance/ added or extended.
  • CONFORMANCE.{md,json} regenerated via
    uv run --extra dev python -m tests.spec_conformance.gen_statement
    and committed.
  • N/A -- this PR does not change OpenAPM-observable behaviour.

This repairs Git configuration replay, not primitive admission, manifest formats, archive extraction, or containment requirements.

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

Retain the effective core.symlinks value, including repository capability detection and explicit command overrides, when freezing Git configuration. Add real-Git precedence and clone regressions and document link-text fallback limits.\n\nRefs #3137

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep an explicit parent command-scope symlink setting above child file-scope entries while preserving explicit child overrides.

Refs #3137

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

Daniel Meppiel (danielmeppiel) commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator Author

APM Review Panel: ship_now

Delta folds all three documentation findings cleanly; zero remaining in-scope engineering work at c5bd885 with verified new-head native acceptance, full CI and complete mutation evidence.

panel-mode=delta; personas=doc-writer,devx-ux-expert; synthesizer=apm-ceo

The delta commit c5bd885 folds exactly the three documentation improvements identified by the full panel: a deployment-independent URL replaces the boundary-escaping relative link in the shipped skill resource, the positional 'see the next bullet' reference now names 'Skipped symlinked agent source' explicitly, and the trailing archive-containment jargon is removed from the user-facing troubleshooting bullet. Both delta specialists -- doc-writer and devx-ux-expert -- independently verified all three closures and returned zero new findings. No production code, test code, CI configuration or native fixture changed between the prior panel head and this one.

Engineering evidence at c5bd885 is independently verified and complete: 225 local tests pass with one expected macOS native skip; four causal mutations (old-selection, first-command, auth-fence, checkout-eol) fail for their intended causes and 72 restored controls pass; the full lint mirror including provider Lint succeeds; owner detection confirms a single owner-extension of 'Git child-process repository location and URL rewrite safety' with 29 functional test IDs matched against retained JUnit; documentation link tests pass 14/14. A fresh hosted native artifact (ID 11400587845, job 112191586003) at exactly this head records standard-user SID ending 1003, no administrator group or symlink privilege, successful ordinary IO and actual Windows error 1314 -- baseline 29 pass, old-precedence mutation 1 expected fail, restored 1 pass, two successful cleanups. All 24 ordinary CI jobs completed with the same distinct job-name set as the prior head (21 SUCCESS, 2 NEUTRAL, 1 SKIPPED), including both CodeQL analyses and docs build. No CI reruns or recoveries were used.

The auxiliary docs-sync classifier aborted due to missing filesystem tool access in its session and never produced valid classifier output. This is an infrastructure limitation, not a documentation gap: the actual doc-writer and devx-ux-expert reviews executed successfully as full-panel specialists and again as delta specialists, providing more thorough coverage than the classifier alone would have. The python-architect's design-pattern nit from the full panel is explicitly interpretive with 'pragmatic suggestion: none' and does not represent outstanding engineering work. Protected merge conditions -- CODEOWNER review, legacy SDL analysis, merge queue and last-push approval -- remain separate provider requirements, excluded from this run's engineering targets per operator instruction. The PR correctly stays draft.

Dissent / factual qualifications. No inter-specialist disagreement in the delta. The three factual imprecisions noted by the full-panel CEO remain on the record and are unaffected by this delta: (1) symlink config is inserted before the auth fence block in the materializer loop, not after as one specialist prose claim stated -- isolation is preserved regardless since the entry carries only key name and value; (2) there are 28 real-Git preservation cases in test_git_symlink_config.py (16 precedence + 5 repeated-command + 1 auth-isolation + 4 isolated-child + 1 unavailable-capability + 1 native-match), not the 22 cited by one specialist; (3) the native Windows acceptance tier is integration-with-fixtures exercising clone_git_worktree and git_network_env consumers, not full CLI e2e as one specialist classified it. Additionally, the performance finding establishes unchanged O(1)/O(n) algorithmic complexity, not an empirical zero-latency measurement. The test-coverage-expert's '72' shorthand refers to a multifile test suite total, not a single-file count. None of these corrections alter any specialist's conclusions or the security posture of the change.

Aligned with: Auth isolation, credential helper suppression and extraheader scrubbing remain structurally unaffected; the delta touches only documentation prose, not the materializer or fence logic. Mutation-auth-fence evidence at c5bd885 proves the regression trap catches credential leaks when the fence is removed. The user-reported Windows checkout failure (#3137) is fixed with verified new-head native acceptance; documentation now gives clear symptom-cause-recovery guidance without positional ambiguity or contributor jargon. Fresh native Windows acceptance at c5bd885 on an actual denied-capability standard-user token (SID ending 1003, WinError 1314, no admin group); portable regression covers all platforms via real Git fixtures with isolated configuration.

Growth signal. The documentation cleanup in this delta directly improves the troubleshooting experience for Windows non-admin users encountering link-text files. The deployed skill resource now links to the published page instead of escaping its package boundary, so users consuming the skill outside the monorepo can follow recovery instructions without a broken path. This is the last friction point in the Windows enterprise adoption funnel for this specific scenario.

Panel summary

Persona B R N Takeaway
doc-writer 0 0 0 Both prior documentation findings are closed at c5bd885. No new substantive delta findings; exact-head hosted native proof and CI remain for orchestrator verification.
devx-ux-expert 0 0 0 All three full-panel doc fixes cleanly folded: explicit section ref replaces positional 'next bullet', trailing archive jargon removed, skill link now deployment-independent; zero remaining in-scope user-guidance issues; ship.

Counts are signal strength, not permission. The maintainer ships.

Recommendation

All three documentation follow-ups from the full panel are folded and independently verified closed by both delta specialists with zero new findings. Engineering evidence at c5bd885 is complete: local functional suite (225 pass, 1 native skip), four causal mutations fail as intended then 72 restored controls pass, full lint mirror passes, owner qualification with 29 functional test IDs verified, fresh native Windows acceptance (artifact 11400587845, baseline 29 pass, mutation 1 expected fail, restored 1 pass, same qualified standard-user context), and 24/24 CI jobs completed. No in-scope engineering work remains. Protected merge conditions (CODEOWNER review from sergio-sisternes-epam, legacy SDL analysis, merge queue, last-push approval) are provider policy gates separate from this engineering assessment; the PR stays draft until those human and machine conditions are satisfied.

Folded in this run

  • c5bd885a: replaced the shipped skill's package-escaping relative link with the published troubleshooting URL.
  • c5bd885a: replaced "the next bullet" with the precise "Skipped symlinked agent source" reference.
  • c5bd885a: removed the archive-containment aside from consumer recovery guidance.

The actual final delta specialists independently verified all three closures. The separate CEO received their complete raw returns and the full panel's returns, conversation and current-head evidence; its genuine recommendation is ship_now, with no follow-ups. No runtime, test or native-fixture code changed in the documentation fold. Earlier advisories below are historical, not the current recommendation.

Copilot signals reviewed

Both complete fetch rounds contained no submitted reviews or inline comments. The two-round cap is honored. No thread replies, resolutions or reviewer requests were necessary.

Current-head qualification

Exact head: c5bd885a5dea65b0f5b8ab17a4987cc63673e921. Incorporated main: 18c4c43c924ceae890fe0f2038806690e5b2d6c8.

  • Functional suite: 225 passed, 1 native prerequisite skip on macOS. This skip is not native acceptance.
  • Complete lint mirror: ruff check/format, pylint duplication, auth boundary, architecture boundary, YAML I/O, 2100-line and portable-path guards passed; actual provider Lint succeeded too.
  • Causal negative controls: old-selection, first-command, auth-fence and checkout-EOL mutations each failed for the intended cause. Fresh restored controls: 72 passed.
  • Canonical-owner evidence: deterministic owner-extension of Git child-process repository location and URL rewrite safety, still in utils/git_env.py; 28 real-Git preservation cases plus one actual native test. The unchanged verifier's full functional branch passed, with wrong-owner and stale-head negative controls rejected. Its blocked-status shortcut is not counted as functional evidence.
  • Documentation links: 14 passed; no new broken local links.
  • Fresh hosted native proof: Windows job 112191586003, artifact windows-native-symlink-1 ID 11400587845, downloaded and digest-verified at this exact head. Actual standard-user SID ending 1003, distinct runner SID ending 500, no administrator group/token or symlink privilege, successful ordinary I/O, and actual error 1314. Baseline 29 passed; causal old-precedence mutation 1 expected failure; restoration 1 passed. All phases retain the same qualified context; both cleanup records passed. This is real hosted Windows evidence, not a macOS simulation or a prior-head artifact.
  • Ordinary CI: 24 completed: 21 success, 2 neutral, 1 skipped. Both CodeQL analyses and docs build succeeded after runner queues cleared. The distinct job-name set matches the prior head; no missing, failed, cancelled or pending ordinary check is accepted as green. CI run 37440135815.

The auxiliary docs-sync classifier aborted because that task lacked filesystem tools; no successful docs-sync classification is claimed. The actual required full and delta documentation/DevX reviews ran and verified the fixes.

Deferred

None. No in-scope foldable item remains. The architect's interpretive design-pattern note requested no change; it is not an unresolved recommendation.

Provider state and run boundaries

PR Head CEO recommendation Iterations Folds Deferrals Copilot rounds CI Mergeable Merge state Notes
#3145 c5bd885 ship_now 2 3 0 2 green MERGEABLE BLOCKED draft and protected policy remain

This fresh run used 2 outer iterations, 2 Copilot fetch rounds, 0 CI recoveries, 1 normal fast-forward push, one full specialist cohort and one delta cohort. Previous recovery attempts remain historical and do not consume or reset this run's counters.

Engineering completion does not grant protected merge permission. The PR remains draft. Historical missing legacy SDL analysis, CODEOWNER/last-push approval and merge queue are separately applicable provider conditions, explicitly outside this run's engineering target. Default CodeQL success does not replace missing legacy SDL analysis. No draft promotion, approval, enqueue, merge, protection change, credential pursuit or unrelated label/reviewer change occurred. The maintainer decides the next protected action.


Full per-persona findings

doc-writer

No findings.

devx-ux-expert

No findings.

This panel is advisory. It does not grant merge permission.

Superseded advisory, retained as historical evidence

APM Review Panel: ship_with_followups

Preserve Git's native symlink capability detection in the APM config materializer, fixing non-admin Windows checkout failure (#3137) with 19 production lines, full native acceptance, and no new code defects.

panel-mode=full; personas=python-architect,test-coverage-expert,doc-writer,performance-expert,auth-expert,supply-chain-security-expert,devx-ux-expert; synthesizer=apm-ceo

This PR fixes a real user-reported bug (#3137) where APM's configuration materializer promoted an inherited global core.symlinks=true above Git's repository-local capability detection, causing checkout failures on non-admin Windows accounts. The fix is surgical: 19 added production lines in git_env.py introduce a pure _symlink_entry_wins predicate and a single-variable accumulator in the existing materializer loop. All seven panelists converge on zero blocking and zero recommended code findings. The auth-expert confirms credential/URL isolation is structurally unaffected with mutation-proven regression traps (test_network_env_symlink_precedence_preserves_auth_isolation at tests/unit/cache/test_git_symlink_config.py asserts ('credential.helper', '!stale-helper') not in entries). The supply-chain-security-expert verified read-only credentials, bounded cleanup, source provenance, and fail-closed evidence handling across the disposable hosted fixture. The performance-expert confirms zero measurable impact: O(1) predicate in an existing O(n) loop with no new subprocess or I/O. The three substantive findings are all documentation improvements.

Test evidence is comprehensive and load-bearing. The test-coverage-expert verified five scenario rows - all passing at exact head 5033ba4 with outcome: passed evidence blocks on every finding, satisfying the contract. Four local mutation modes each produced expected failures (old-selection 3, first-command 3, auth-fence 1, checkout-eol 1); restoration passed 72/72 clean. Native Windows acceptance (artifact 11359676500, digest sha256:957031ae...8836) confirms real denied-capability context: standard-user SID ending 1003, no admin group/token, no symlink privilege, ordinary I/O succeeds, actual Windows error 1314. Baseline 29 pass, intentional mutation 1 fail, restored 1 pass, both cleanup completions recorded with zero failures. CI is fully green: 24/24 rollups (21 success, 2 neutral, 1 skipped). The production change extends the existing canonical git_env.py owner without introducing parallel authority - an owner-extension classification confirmed by owner-touch-report.json.

The doc-writer identified two genuine in-scope issues: the shipped skill resource at packages/apm-guide/.apm/skills/apm-usage/dependencies.md:80 uses a five-level relative link that escapes the packages/apm-guide boundary, breaking in standalone consumption contexts; and docs/src/content/docs/consumer/install-packages.md:218 says 'see the next bullet' but the next bullet is 'Critical security finding', not the intended 'Skipped symlinked agent source' entry. The devx-ux-expert flagged trailing archive-containment jargon in the user-facing troubleshooting bullet at line 220. All three are clean in-PR folds that strengthen the documentation without touching production logic. The python-architect's design-pattern nit is explicitly interpretive ('the current shape is the simplest correct design at this scope' with 'pragmatic suggestion: none') and not an actionable defect. Provider-level requirements (CODEOWNER review, SDL analysis, merge queue) remain outside this engineering panel; the PR correctly stays draft.

Dissent / factual qualifications. No severity disagreements between panelists. Three factual imprecisions noted for the record: (1) the supply-chain-security-expert states symlinks are 'appended after the auth fence entries' but source code appends them before the auth fence block (retained.append(("core.symlinks", symlinks.value)) precedes the if auth_fence is not None: block) - isolation is preserved regardless since the entry carries only key name and value with no credential data; (2) the python-architect cites '22 real-Git cases' but the parametrized test_git_symlink_config.py contains 28 real-Git controls (16 precedence + 5 repeated-command + 1 auth-isolation + 4 isolated-child + 1 unavailable-capability + 1 native-match); (3) the test-coverage-expert labels the native Windows acceptance tier as 'e2e' but the test exercises clone_git_worktree and git_network_env consumers, not the full CLI entry point - correct tier is integration-with-fixtures. None of these imprecisions affect any panelist's conclusions or security posture.

Aligned with: Auth isolation, credential helper suppression, and extraheader scrubbing verified structurally unaffected by auth-expert and supply-chain-security-expert; mutation-auth-fence proves the regression trap catches credential leaks when the fence is removed. Directly fixes a real user-reported Windows checkout failure: APM now preserves Git's native capability detection instead of forcing inherited global values, matching the behavior users expect from a plain git clone. Native Windows acceptance on actual denied-capability standard-user token (SID *1003, error 1314, no admin group); portable regression covers all platforms via real Git fixtures with isolated configuration.

Growth signal. This is a concrete Windows-user conversion fix: a real user hit the symlink checkout failure and had to change their global Git configuration as a workaround. Removing that paper cut is a direct funnel improvement for Windows non-admin environments, which are common in enterprise settings where APM adoption matters most.

Panel summary

Persona B R N Takeaway
python-architect 0 0 1 Production _symlink_entry_wins predicate and materializer accumulator extend the canonical git_env owner correctly; no parallel authority, truth table verified, auth isolation confirmed.
test-coverage-expert 0 0 0 Five scenario rows verified; regression traps, auth-isolation, real-Git integration, 4 local mutations, native Windows acceptance all pass at exact head. No gap.
doc-writer 0 1 1 Fallback semantics and native prerequisites match the implementation and supplied proof. Make the shipped skill's troubleshooting link portable and fix one misplaced bullet reference.
performance-expert 0 0 0 Zero measurable impact: 19 added lines are pure O(1) predicate + single-variable tracker in an existing O(n) loop; no new subprocess, network, or I/O calls in production.
auth-expert 0 0 0 Symlink selection cleanly extracted from the credential materializer; auth fence, credential helper suppression, and extraheader scrubbing structurally unaffected with mutation-proven regression trap.
supply-chain-security-expert 0 0 0 Read-only credentials, bounded cleanup, source provenance, and auth isolation verified against actual native artifact; no security regressions.
devx-ux-expert 0 0 1 No CLI surface change; troubleshooting docs cover symptom-cause-recovery for link-text checkout; skill resource stays in sync; ship.

Counts are signal strength, not permission. The maintainer ships.

Top 3 follow-ups

  1. [doc-writer] Replace the five-level relative link in packages/apm-guide/.apm/skills/apm-usage/dependencies.md:80 with a deployment-independent URL (e.g. https://microsoft.github.io/apm/consumer/install-packages/#when-things-go-wrong). - The current link escapes the packages/apm-guide boundary and breaks in standalone skill consumption contexts - readers see the fallback warning but cannot follow the recovery instructions.
  2. [doc-writer] Replace 'see the next bullet' with 'see "Skipped symlinked agent source" below' in docs/src/content/docs/consumer/install-packages.md:218. - The next bullet is actually 'Critical security finding', not the intended agent-source symlink check; the positional reference misdirects troubleshooting readers.
  3. [devx-ux-expert] Remove trailing '; archive extraction and containment rules are unchanged' from the symlink-checkout-fallback troubleshooting bullet at install-packages.md:220. - The clause is contributor-facing context that dilutes the concrete symptom-cause-recovery message aimed at users who just discovered link-text files.

Architecture

classDiagram
    direction LR
    class GitConfigEntry {
        <<ValueObject>>
        +scope str
        +key str
        +value str
    }
    class _GitConfigSnapshot {
        <<ValueObject>>
        +entries tuple~GitConfigEntry~
        +rewrites tuple
        +http_headers tuple~GitConfigEntry~
    }
    class _GitAuthFence {
        <<ValueObject>>
        +remote_url str
        +suppress_helpers bool
        +safe_headers tuple
        +managed_header str
    }
    class git_network_env {
        <<IOBoundary>>
        +remote_url str
        +overrides dict
    }
    class _materialize_git_config_snapshot {
        <<IOBoundary>>
        +env dict
        +snapshot _GitConfigSnapshot
        +auth_fence _GitAuthFence
    }
    class _symlink_entry_wins {
        <<Pure>>
        +current GitConfigEntry or None
        +candidate GitConfigEntry
    }
    class _merge_parent_git_config_snapshot {
        <<Pure>>
        +parent _GitConfigSnapshot
        +child _GitConfigSnapshot
    }
    _GitConfigSnapshot *-- GitConfigEntry : contains
    git_network_env ..> _merge_parent_git_config_snapshot : merges snapshots
    git_network_env ..> _materialize_git_config_snapshot : freezes config
    _materialize_git_config_snapshot ..> _symlink_entry_wins : selects winner
    _materialize_git_config_snapshot ..> GitConfigEntry : iterates
    _materialize_git_config_snapshot ..> _GitAuthFence : applies fence
    note for _symlink_entry_wins "Accumulator predicate: command scope\nis sticky; otherwise last-entry-wins"
    note for _materialize_git_config_snapshot "Loop extracts core.symlinks via continue,\naccumulates winner, appends after main loop"
    class _symlink_entry_wins:::touched
    class _materialize_git_config_snapshot:::touched
    classDef touched fill:#fff3b0,stroke:#d47600
Loading
flowchart TD
    A["git_network_env url, overrides\nsrc/apm_cli/utils/git_env.py:1168"] --> B["_merge_parent_git_config_snapshot\nsrc/apm_cli/utils/git_env.py:969"]
    B --> C["_materialize_git_config_snapshot env, snapshot\nsrc/apm_cli/utils/git_env.py:998"]
    C --> D{{"for entry in snapshot.entries"}}
    D --> E{"normalized == core.symlinks?"}
    E -->|Yes| F{"_symlink_entry_wins symlinks, entry\nsrc/apm_cli/utils/git_env.py:988"}
    F -->|True: replace| G["symlinks = entry\ncontinue"]
    F -->|False: command retained| H["continue"]
    E -->|No| I{"scope in local, worktree\nand not network-config?"}
    I -->|skip| J["continue"]
    I -->|keep| K{"include.path / auth fence\n/ header filter"}
    K -->|pass| L["retained.append key, value"]
    K -->|filtered| M["continue"]
    G --> D
    H --> D
    J --> D
    L --> D
    M --> D
    D -->|exhausted| N{"symlinks is not None?"}
    N -->|Yes| O["retained.append\ncore.symlinks, symlinks.value"]
    N -->|No| P["no symlinks entry"]
    O --> Q["Append auth fence entries:\ncredential.helper, http.extraheader"]
    P --> Q
    Q --> R["retained = list dict.fromkeys retained\ndedup preserving order"]
    R --> S["[I/O] Write GIT_CONFIG_NOSYSTEM=1\nGIT_CONFIG_SYSTEM=devnull\nGIT_CONFIG_GLOBAL=devnull\nGIT_CONFIG_COUNT=N, KEY_i, VALUE_i"]
Loading

Recommendation

Fold the three documentation fixes (skill link portability, bullet reference, trailing jargon) and this PR is engineering-complete with zero code findings from seven specialists. The production change is minimal (19 lines), well-tested with four mutation modes and full local/native evidence, and Windows acceptance is verified at exact head. After folding, a delta review should confirm clean. Provider-level requirements (CODEOWNER approval from sergio-sisternes-epam, SDL analysis, merge queue) are the remaining path to merge - those are human and policy gates, not in-scope engineering work.

Fresh run, 2026-10-06: the three listed documentation follow-ups will be folded
in this PR. The architecture design-pattern note explicitly requests no change.
No new production-code defect was identified. No Copilot review or inline
comment exists in the fully paginated conversation.

At head 5033ba473d1a9661c366f6d6526656bd0cedb0f9, local functional checks:
225 passed, one native-prerequisite skip on macOS; four removed-guard negative
controls failed for their intended causes, and 72 restored controls passed.
The full lint mirror passed (ruff check/format, pylint duplication, YAML I/O,
2100-line, portable paths, auth and architecture boundaries). Main
18c4c43c924ceae890fe0f2038806690e5b2d6c8 is incorporated.

Native proof was independently downloaded and verified, not inferred from a
green badge: job 111875086870,
artifact 11359676500, archive digest matching the API. Same qualified standard
user: 29 baseline passes, one causal old-selection failure, restored pass,
two successful cleanups. This is real Git/APM clone integration, not a full CLI
end-to-end test; the 28 preservation controls and exact multi-file execution
logs take precedence over abbreviated specialist counts. Performance reasoning
establishes unchanged complexity, not a measured zero-latency claim.

All 24 ordinary CI rollups completed: 21 success, two neutral, one skipped.
The prior missing-native-proof text below is historical. Legacy SDL analysis,
CODEOWNER/last-push policy and the merge queue are separate provider conditions,
excluded from this run's engineering targets, not waived. The PR remains draft;
existing ownership (Daniel Meppiel (@danielmeppiel), Sergio Sisternes (@sergio-sisternes-epam)) is unchanged. No
promotion, reviewer request, approval, queue action or merge is authorized.


Full per-persona findings

python-architect

  • [nit] Design patterns assessment for the production change in git_env.py

Design patterns

  • Used in this PR: Pure Function + Accumulator -- _symlink_entry_wins (annotated <<Pure>> in class diagram) is a stateless predicate extracted from the _materialize_git_config_snapshot entry loop; the loop uses an accumulator (symlinks: GitConfigEntry | None) to track the winning core.symlinks entry across the full iteration before appending it after the main loop. This separation keeps the selection logic testable in isolation (truth-table unit test in test_git_symlink_precedence.py) while the real-Git integration tests in test_git_symlink_config.py exercise the accumulator end-to-end against actual Git subprocess config resolution.
  • Pragmatic suggestion: none -- the current shape is the simplest correct design at this scope. The predicate has exactly one call site, the accumulator is three lines of loop state, and a Strategy or Visitor abstraction would be over-engineering for a single config key. If future config keys require similar scope-aware selection (e.g. core.autocrlf or core.eol), extracting a ScopeAwareConfigSelector would pay off, but not at the current one-key scope.

test-coverage-expert

No findings.

doc-writer

  • [recommended] Use a deployment-independent troubleshooting link in the shipped skill.

The new five-level relative link resolves in the microsoft/apm source checkout, but its target lies outside packages/apm-guide. In a standalone package or deployed skill, it points into the consumer's surrounding filesystem rather than the published documentation. Readers receive the fallback warning but cannot follow its promised recovery instructions. Cross-references must remain usable in the artifact's intended consumption context, not only in the producer's monorepo.

Suggested: Link this shipped skill resource to https://microsoft.github.io/apm/consumer/install-packages/#when-things-go-wrong. Keep the full explanation canonical there and retain relative links within the documentation site.

  • [nit] Name the symlinked-agent-source bullet instead of saying 'the next bullet'.

The next bullet is 'Critical security finding', not the agent-source symlink check. 'Skipped symlinked agent source' appears later, after policy, authentication and drift entries. The positional reference misdirects readers and weakens the otherwise correct distinction between checkout fallback and agent-source validation.

Suggested: Replace 'see the next bullet' with 'see "Skipped symlinked agent source" below'.

performance-expert

No findings.

auth-expert

No findings.

supply-chain-security-expert

No findings.

devx-ux-expert

  • [nit] Trailing archive-containment clause is maintainer jargon in a user-facing troubleshooting bullet

The symlink-checkout-fallback bullet ends with 'archive extraction and containment rules are unchanged'. A user who just discovered link-text files instead of real content needs symptom, cause, and recovery -- all of which the preceding five sentences deliver well. The trailing boundary assurance about archive extraction is aimed at contributors wondering whether the archive pipeline changed, not at the troubleshooting reader. It dilutes the concrete recovery message ('Use a package that ships real source files, or a symlink-capable environment'). Drop or move to a contributor note.

Suggested: End the bullet after 'or a symlink-capable environment.' and remove '; archive extraction and containment rules are unchanged'.

This panel is advisory. It does not grant merge permission.

Superseded advisory, retained as historical evidence

APM Review Panel: ship_with_followups

Native Windows acceptance is now demonstrated; this is not an accepted ship_now. At 5033ba473d1a9661c366f6d6526656bd0cedb0f9, real standard-user Windows execution passes with inherited global symlinks enabled and native local capability disabled. Removing the precedence guard causes the expected real Git failure; restoring it passes. Required SDL analysis and protected merge requirements remain outstanding.

panel-mode=delta; personas=python-architect,test-coverage-expert,supply-chain-security-expert; synthesizer=apm-ceo

cc Daniel Meppiel (@danielmeppiel) -- the final engineering evidence is available for review. Existing CODEOWNERS and requested-reviewer state are unchanged.

Recommendation

The genuine final CEO recommends ship_with_followups, not ship_now. The three final specialists identified no new actionable defect. Both earlier coverage recommendations are folded: a six-cell helper truth table and a native mutation that proves the exact changed selection with the correct polarity. Native observation, current-head functional evidence and final review are qualified independently; none substitutes for required security analysis or human approval.

The no-bypass provider check still reports: code-owner review from sergio-sisternes-epam is needed, one CodeQL result is missing, and changes must pass through the merge queue. The missing legacy Microsoft SDL analysis is distinct from successful default CodeQL. Downloading the pinned SDL pack with the existing credential returned 403; no credential elevation, placeholder result, policy weakening or bypass was used. The PR stays draft until required delivery gates are met. No promotion, queue entry or merge was performed.

Panel summary

Persona Blocking Recommended Nits Takeaway
Python architect 0 0 0 Existing owner boundaries and strict source provenance remain intact.
Test coverage 0 3 0 These three entries are closure observations, not outstanding requests: causal mutation, helper truth table, and EOL regression protection are present.
Supply-chain security 0 0 0 Checkout-only EOL configuration remains process-scoped; no persistent Git configuration or credential changes.

Reservations carried from the accepted scope

The current scope for #3137 permits only the bounded existing disposable-runner experiment. Explicit command-scope intent, credential/URL isolation, unrelated settings and containment remain protected. Link-text checkout is not archive-symlink extraction or general support for packages requiring genuine symlinks. The approved operational plans, not an assertion that Daniel separately approved every command, authorized the temporary hosted fixture.

Folded in this run

  • 945515d4: canonical selection helper and concise symptom-first documentation.
  • 0d08f5bd: exact-head disposable standard-user Windows fixture, capability qualification, native regression and failure-safe cleanup.
  • e7dd8272: both coverage followups; exact changed-decision tuple and symlink-specific mutation failure.
  • 5033ba47: CI recovery 1, pinning checkout-only LF and distinguishing wrong-head from dirty-source errors. A real-Git control reproduces unpinned CRLF dirtiness after configuration isolation; the pinned checkout passes without weakening clean-source checks.

No Copilot inline findings or submitted reviews were present in the pagination-complete final conversation. No new out-of-scope implementation is deferred by this final panel.

Native and regression evidence

CI run 37343172728, native job 111875086870 succeeded. Retained artifact: windows-native-symlink-1, ID 11359676500.

Observation Actual result
Source/interpreter Exact 5033ba47 checkout and its provisioned Python/APM executable; tracked source clean before and after.
Native capability Standard-user SID ending 1003, distinct from runner SID 500; no administrator token/group or symlink privilege; ordinary I/O succeeds; actual Windows error 1314.
Baseline 29 passed, including the native case and 28 preservation controls; no skips/errors.
Mutation One expected failure in the native case; recorded global true -> local false selection changes from correct=True to mutant=False, followed by the specific real Git symlink failure.
Restoration Native case passes again under the same qualified context.
Cleanup Primary cleanup and independent always() cleanup both complete without failures; original registry state and owned account/profile/scratch restoration recorded.
Local supporting suite 225 passed, one native prerequisite skip on macOS. That skip is not native proof.

The initial hosted run at e7dd8272 failed provenance before native qualification; its failure and successful cleanups are retained, not counted as acceptance. The removed-checkout-binding workflow mutation fails the exact regression and passes after restoration. The helper truth-table mutation produced two expected failures with the old guard, then six passing cases after restoration.

Owner, lint and CI qualification

Fresh deterministic detection at base 18c4c43c924ceae890fe0f2038806690e5b2d6c8 / head 5033ba473d1a9661c366f6d6526656bd0cedb0f9 identifies the existing Git child-process repository location and URL rewrite safety owner. Version 2 evidence uses that exact decision name and actual passing test IDs matched to retained JUnit, including the native case. Classification is owner-extension, with no new owner or routing split.

The canonical full functional-evidence branch was independently exercised and passed, including a negative control rejecting an owner path in place of its decision name. The externally blocked completion also passes its schema, but its status-only verifier shortcut is explicitly not treated as readiness or functional proof.

Before the normal fast-forward push, current main was incorporated and the complete local lint chain passed: ruff check/format, pylint duplication, auth/architecture boundaries, YAML I/O, 2100-line and portable-path guards. Exact commands/results are in the corrected PR body. uv.lock and the checkout are clean.

All 24 current-head rollup entries completed: 21 success, 2 neutral, 1 skipped, including required gate, Spec conformance, native Windows and provider Lint. This green rollup does not supply the missing SDL analysis.

PR Head Actual CEO stance CI recovery count Rollups Mergeable Provider merge state
#3145 5033ba473d ship_with_followups 1 green MERGEABLE BLOCKED
Review provenance and factual qualifications

The final delta covers 0d08f5bd..5033ba47, not a no-op. Three genuine specialist tasks and a separate whole-context CEO task executed; every final JSON payload validates against the actual schema. Raw prefixes, original findings and earlier schema-correction history remain retained. The complete pre-publication conversation watermark is adbad4e470dbe75a49b1f0bf30cb4b5253c111a34a734439530515942e7212b1.

The CEO's prose contained two factual slips, corrected in this rendering without altering its retained raw return or stance: there are two Git discovery jobs (Python 3.10/3.11), not three; one coverage return contains three closure observations, not three separate coverage returns. The schema does not force nonempty findings. Its ship_with_followups recommendation does not establish protected merge readiness.

Only actual counters are reported: one full cohort, two later delta cohorts and one real CI recovery are retained. Optional outer/Copilot episode totals are not reconstructed from push counts or copied worker assertions. No new budget or reset is inferred.

Full final per-persona findings

python-architect

No findings.

supply-chain-security-expert

No findings.

test-coverage-expert

  • [recommended] Prior mutation cause-specificity followup is closed: native_precedence_guard now records and asserts the exact changed-decision tuple with correct polarity, and the symlink-specific stderr sentinel narrows failure cause.

The delta adds a six-field changes list in native_precedence_guard that captures (current.scope, current.value, candidate.scope, candidate.value, correct, mutant) for every decision where the mutant diverges from production. The fixture then asserts ('global','true','local','false',True,False) in changes -- proving the mutation actually rejected local-false over inherited global-true (correct=True means production accepts the candidate; mutant=False means the mutant rejects it). The prior CEO noted the original coverage suggestion inverted the polarity; the actual code uses the correct orientation. The native CI mutation.xml JUnit property confirms the tuple was recorded on real Windows: mutation_decisions=[["global","true","local","false",true,false]]. Additionally, the symlink-specific stderr check ('unable to create symlink AGENTS.md' not in error.stderr) ensures only the expected Git clone failure triggers MUTATION_FAILURE, not an unrelated CalledProcessError. Both strengthening measures are present and verified.

Evidence (passed, integration-with-fixtures): tests/integration/test_windows_native_symlink.py; Native CI run37343172728 job111875086870: baseline 29pass/9.6s, mutation 1fail/1.8s with mutation_decisions=[[global,true,local,false,true,false]], restored 1pass/1.6s. Actual WinError 1314, SID S-1-5-...-1003, non-admin.

  • [recommended] Prior truth-table unit test followup is closed: new test_git_symlink_precedence.py covers 6-cell _symlink_entry_wins truth table including the critical command-over-local rejection.

The delta adds tests/unit/cache/test_git_symlink_precedence.py with 6 parametrized cases covering: None->local(True), global->local(True), local->command(True), command->command(True), command->local(False), system->global(True). The critical case command->local(False) proves the function correctly rejects a lower-scope candidate when a command-scope entry already exists. All 6 cases pass in local-recovery1.xml (225 passed, 1 skipped). This complements the 28 integration controls in test_git_symlink_config.py that exercise the same function through real Git fixtures. The prior coverage suggestion proposed 4 cases; the actual implementation adds 6 covering additional scope transitions (system->global, global->local).

Evidence (passed, unit): tests/unit/cache/test_git_symlink_precedence.py; local-recovery1.xml: 6 parametrized cases of test_symlink_entry_wins_truth_table all passed in 225-test suite, 10.01s total.

  • [recommended] New CRLF config-isolation regression test and workflow env assertion provide regression-traps for the CI recovery that unblocked native proof; no new coverage gaps introduced.

The delta adds test_provenance_survives_config_isolation_only_with_bound_checkout_eol (2 parametrized cases) proving that without checkout-time core.autocrlf=false pinning, a Git clone under a parent autocrlf=true config produces CRLF working-tree content that becomes dirty when the parent config is removed -- exactly the failure mode that blocked e7dd native CI. The pin_checkout_lf=True case passes provenance; the pin_checkout_lf=False case correctly raises 'tracked acceptance source is dirty'. Both pass in local-recovery1.xml using real Git subprocess (config_env fixture). The workflow test (test_native_job_is_unconditional_bounded_and_read_only) now asserts checkout env has the three GIT_CONFIG keys; recovery1-workflow-mutation.xml proves this test fails with KeyError:'env' when the env block is absent, confirming it catches the regression. The split provenance error messages (SHA mismatch vs dirty paths) are covered by existing test_provenance_rejects_wrong_source[head] and the new CRLF test respectively. No production source changed; no new user-promise surfaces untested.

Evidence (passed, integration-with-fixtures): tests/unit/test_windows_native_symlink_probe.py; local-recovery1.xml: both parametrized cases (pin_checkout_lf=False,True) passed in 225-test suite. recovery1-workflow-mutation.xml: workflow env assertion correctly failed with KeyError:'env' proving regression-trap.

Superseded prior advisory, preserved as history (not current-head evidence)

Native Windows proof remains pending

Current head: 945515d49ffb07ad2d965a02a3a4732132b410b4, based on main 18c4c43c924ceae890fe0f2038806690e5b2d6c8. This PR remains a draft, not an accepted ship_now.

The renewed scope for #3137 and coordinator-approved execution plan include the bounded native Windows experiment on a disposable GitHub-hosted runner. That obligation was not removed. The required same-account non-admin/no-symlink-rights observation and real Git/APM fallback have not yet been produced. The inherited-global, native-local and explicit-command configuration cases must remain distinct.

The genuine renewed review produced the documentation and helper-extraction fold in 945515d4. Those changes do not replace native acceptance evidence. The required SDL CodeQL analysis is also absent; ordinary check results do not satisfy that separate machine rule. Human approval and protected merge-queue conditions remain separate, with no merge or bypass authorized.

This repairs an accidental literal-file-path overwrite of this same comment. The previous full advisory is preserved below as historical evidence, not as current-head validation. In particular, its statement that the Windows setup was unapproved belongs to the earlier closed attempt; it does not describe the renewed execution plan.

Historical attempt at e6f50a1 (superseded)

APM merge-readiness advisory: blocked

This PR remains a draft, not merge-ready. The required native non-admin Windows scenario has not been demonstrated: inherited core.symlinks=true, no symlink-creation rights, and actual Git/APM checkout fallback under that same account at the final head. Portable simulation, macOS execution, and passing Windows CI without the required account/capability evidence do not establish that result.

Current head: e6f50a178c0acdb18720d55e1dddcc42999cd1c3. Current base: 7562a0c9f2b816a9f4554dd52c704ec78161c02b.

Reservations carried from the approved scope

  • Preserve explicit Git configuration intent, credential/URL isolation, unrelated settings, and containment. Link-text fallback is not equivalent to genuine package symlink semantics.
  • No archive-symlink materialization, global Git setting changes, or universal symlink forcing.
  • The proposed additional Windows CI/account/registry setup remains unapproved and was not implemented. No merge, reviewer request, or issue-closing action is authorized by this advisory.

Folded in this run

  • Repeated command-scope precedence regression coverage and capability-based documentation headings: bcd4b7b2743cb325c54e61c3eaa1227c83db3fb2.
  • Mixed symlink-precedence and credential-helper/header isolation regression: e6f50a178c0acdb18720d55e1dddcc42999cd1c3.
  • PR description corrected to classify the detected git_env.py owner touch as owner-extension.

Copilot signals reviewed

No Copilot inline findings were recorded in the corrected worker receipt.

Deferred outside the approved scope

A new install-time fallback warning would introduce an additional user-visible mechanism. It remains a separate follow-up, not a substitute for the required Windows evidence.

Regression and lint evidence

The worker reports that removing the repeated-value selection behavior and the credential-helper suppression guard each caused its corresponding regression test to fail, then pass after restoration. At the final head it reports 28 symlink cases, 125 adjacent auth/hook/URL tests, and 284 cache tests passing.

The worker reports the full current-main CI lint mirror passed: ruff check/format and pylint R0801 across the covered source and architecture-linter paths, plus auth, architecture-boundary, YAML I/O, 2100-line, and portable-path guards. The coordinator independently confirmed the final worktree is clean.

CI and mergeability

Fresh coordinator verification found all 22 rollup entries completed with SUCCESS, NEUTRAL, or SKIPPED, including the required gate. This is CI evidence, not proof of the missing non-admin Windows scenario.

PR Head Worker stance CI Mergeable Merge state Notes
#3145 e6f50a178c ship_with_followups reported green MERGEABLE BLOCKED Draft; required native Windows evidence absent

Completion qualification

The corrected blocked receipt passes JSON schema validation. Its embedded owner report still uses the older base 7dfc5dd74e, rather than current base 7562a0c9f2. The semantic verifier skips terminal owner-evidence enforcement for a blocked status, so its successful exit does not establish current-base readiness. The receipt also records panel_mode=full, not the required terminal delta; that finalization is not verified.

These evidence qualifications must be resolved before any later readiness claim. The coordinator has ended this attempt as blocked, not accepted the worker's claim that all process gates are complete. No merge was performed.


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


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


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

…cedence

Add test_network_env_preserves_last_repeated_command_value to close a
review-panel-flagged gap: no existing test discriminated first-occurrence-wins
from last-value-wins for repeated command-scope core.symlinks entries. The
production code in _materialize_git_config_snapshot already implements
correct last-value-wins semantics; this adds the missing regression trap
(mutation-break verified: fails under a naive first-wins mutation).

Also retitle the "Windows symlink checkout" / "Windows Git symlinks" doc
headings to "Symlink checkout fallback" / "Git symlink checkout fallback",
clarifying that the capability check is not Windows-exclusive (it applies to
any account without symlink creation rights), while the common real-world
trigger remains non-admin Windows.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Addresses coordinator finding: the symlink-precedence materializer and
the credential/URL auth fence share one entry loop in
_materialize_git_config_snapshot. Adds a focused regression test that
interleaves a repeated command-scope core.symlinks sequence with a
command-scope credential.helper reset and a stale http.extraheader,
asserting both the symlinks resolution AND the auth fence hold
together. Mutation-break verified: removing the
auth_fence.suppress_helpers credential.helper guard makes this new
test fail (stale '!stale-helper' leaks through); restoring the guard
makes it pass again. No production code changed.

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

- doc-writer: dedupe near-verbatim symlink-fallback prose across
  install-packages.md and dependencies.md; dependencies.md now points to
  the canonical install-packages.md section.
- devx-ux-expert: lead the symlink checkout-fallback troubleshooting
  bullet with the user-visible symptom before the internal mechanism.
- python-architect: extract the core.symlinks precedence OR-condition
  into a named helper _symlink_entry_wins() for readability.

Fresh full review panel (python-architect, test-coverage-expert,
auth-expert, doc-writer, performance-expert, devx-ux-expert) spawned
this run against origin/main-refreshed head; auth-expert and
performance-expert found zero issues. All 28 tests in
test_git_symlink_config.py re-verified passing after folds.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Complete the approved #3137 native acceptance requirement on a disposable hosted runner. Qualify actual denied capability, exercise real Git/APM fallback with an in-memory negative control, and independently restore owned fixture state. Local supporting checks do not substitute for the pending native run.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Fold the genuine delta coverage recommendations: require a changed global-true/local-false decision and symlink-specific Git failure, and directly guard the extracted selection truth table. The in-memory removed-local-precedence control fails two cells and all six pass unmodified.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Recover the first native CI failure without weakening provenance. A real-Git control reproduces CRLF checkout becoming dirty after system-config isolation; command-scoped checkout autocrlf=false preserves clean source under the child's empty Git config. Report head mismatch and changed paths separately.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Fold the fresh PR #3145 panel doc-writer and DevX follow-ups: use the published troubleshooting page from the shipped skill, name the actual agent-source check, and keep recovery guidance focused. No runtime or native fixture changes.

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

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@danielmeppiel
Daniel Meppiel (danielmeppiel) marked this pull request as ready for review October 6, 2026 11:45
Copilot AI balanced review requested due to automatic review settings October 6, 2026 11:45
@danielmeppiel
Daniel Meppiel (danielmeppiel) merged commit 0c51257 into main Oct 6, 2026
25 checks passed
@danielmeppiel
Daniel Meppiel (danielmeppiel) deleted the danielmeppiel-issue-delivery-3137 branch October 6, 2026 11:45

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.

Copilot review overview

🟡 Changes recommended

The native Windows acceptance job is not included in the required merge-gate checks, leaving its regression coverage non-blocking.

Review effort: Balanced
Findings: 1 High severity

Open (1)
What changed in this PR

Preserves Git’s effective core.symlinks precedence so repository capability detection overrides inherited settings while explicit command overrides remain authoritative.

Changes:

  • Updates Git configuration materialization.
  • Adds portable and native Windows regression coverage.
  • Documents symlink fallback behavior and adds a dedicated CI job.
File Description
src/​apm_cli/​utils/​git_env.py Selects and retains the effective symlink setting.
tests/​unit/​cache/​test_git_symlink_precedence.py Tests precedence selection rules.
tests/​unit/​cache/​test_git_symlink_config.py Exercises real Git configuration and checkout behavior.
tests/​integration/​test_windows_native_symlink.py Verifies native non-admin Windows fallback.
tests/​integration/​conftest.py Adds the native Windows prerequisite gate.
tests/​unit/​test_windows_native_symlink_probe.py Tests provenance, qualification, mutation, and JUnit controls.
tests/​unit/​test_windows_native_symlink_workflow.py Checks the native workflow’s structure and cleanup.
scripts/​windows_native_symlink_probe_entry.py Runs and validates native acceptance phases.
scripts/​windows_native_standard_user_symlink_gate.ps1 Creates and cleans up the disposable Windows fixture.
.github/​workflows/​ci.yml Adds native Windows acceptance and script linting.
pyproject.toml Registers the new pytest prerequisite marker.
docs/​src/​content/​docs/​consumer/​install-packages.md Documents link-text fallback limitations.
docs/​src/​content/​docs/​contributing/​integration-testing.md Documents the native test contract.
packages/​apm-guide/​.apm/​skills/​apm-usage/​dependencies.md Updates shipped dependency guidance.

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

Comment thread .github/workflows/ci.yml
echo "::endgroup::"

windows-native-standard-user-symlink-gate:
name: Windows Native Symlink Acceptance
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants