docs: clarify lockfile transitive-graph determinism (refs #2007)#2010
Conversation
Issue #2007 reported different graphs between a lockfile replay and a fresh install after an upstream transitive manifest moved a dependency to devDependencies. The resolver already excludes transitive devDependencies and the lockfile correctly pins the graph as resolved. Document that contract instead of changing replay semantics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Docs-only update clarifying that apm.lock.yaml deterministically replays the full resolved dependency graph (including transitive manifests) until the lockfile is explicitly regenerated, addressing confusion raised in #2007 about differing graphs between lockfile replay and fresh resolution.
Changes:
- Clarify in the lockfile spec that the lockfile pins the full resolved graph and does not implicitly follow upstream transitive manifest moves.
- Add the same “lockfile replay determinism” contract to the
apm installCLI reference. - Explain the effect of upstream moves between
dependencies.apmanddevDependencies.apmin the consumer install guide.
Show a summary per file
| File | Description |
|---|---|
| docs/src/content/docs/reference/lockfile-spec.md | Defines lockfile determinism contract and explicitly calls out upstream transitive manifest changes. |
| docs/src/content/docs/reference/cli/install.md | Adds an install behavior bullet describing lockfile replay across the full resolved graph. |
| docs/src/content/docs/consumer/install-packages.md | Explains why transitive graph changes only appear after regenerating the lockfile. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Low
| package's `apm.yml` does not change installs that replay an existing lockfile; | ||
| APM keeps using the recorded commits until you run `apm update`, `apm lock | ||
| --update`, or otherwise regenerate the lockfile after changing `apm.yml`. |
Addresses apm-review-panel doc-writer and DevX follow-ups by naming the exact lockfile refresh command and surfacing the replay note in the consumer guide. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Addresses the final DevX panel nit by keeping the consumer guide's refresh options aligned with the lockfile reference while preserving the narrow documentation-only scope. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
APM Review Panel:
|
| Persona | B | R | N | Takeaway |
|---|---|---|---|---|
| Python Architect | 0 | 0 | 0 | Docs-only; no architectural surface touched. Documented contract matches existing resolver/lockfile replay behavior. |
| CLI Logging Expert | 0 | 0 | 0 | No CLI output surface touched; docs-only clarification of existing lockfile replay semantics. |
| DevX UX Expert | 0 | 0 | 0 | Docs-only; lockfile replay note names concrete recovery commands matching CLI reference. |
| Supply Chain Security Expert | 0 | 0 | 0 | Docs-only; no code, no integrity weakening. Clarification reinforces secure-by-default lockfile replay semantics. |
| OSS Growth Hacker | 0 | 0 | 1 | Three-surface docs clarification reinforces lockfile determinism trust signal; no conversion-surface regressions. |
| Doc Writer | 0 | 0 | 0 | Docs-only clarification stays narrow, aligns the consumer and reference contract, and does not alter lockfile replay semantics. |
B/R/N are signal-strength counts, not gates. The maintainer ships.
Top follow-ups
- [CLI Logging Expert] Add a verbose-mode breadcrumb when replay skips resolution -- reduces support volume for the same confusion pattern that spawned [BUG] apm install replays stale locked commits and produces a different dependency graph than a fresh install #2007, but requires a code change outside this PR's scope.
- [OSS Growth Hacker] Add a search-symptom sentence to the consumer guide note -- improves discoverability for users who search the symptom rather than the concept.
- [Doc Writer] Consolidate lockfile replay explanation in a broader docs architecture pass -- useful if the three surfaces drift later, but outside this narrow clarification.
Recommendation
Merge as-is. The deferred items are all net-positive but none reduce the immediate value of closing #2007 with a correct, user-facing explanation.
Reservations carried from strategic-alignment
- P6 Reliability over magic: keep the fix NARROW to the transitive devDeps-filter behaviour; do NOT touch lockfile replay determinism. The original reporter was themselves unsure of intended behaviour, and this PR resolved to a docs-clarification outcome. The panel weighed this and found the docs clarification to be the right resolution.
Folded in this run
- (panel) Name
apm lock --updateexplicitly in the consumer guide -- resolved in 93689f3. - (panel) Change PR body from
Refs #2007toCloses #2007-- resolved in PR body update after 93689f3. - (panel) Wrap the consumer guide lockfile replay paragraph in a note callout -- resolved in 93689f3.
- (panel) Align consumer guide refresh wording with the lockfile reference nuance -- resolved in 08ffa5c.
Deferred (out-of-scope follow-ups)
- (panel) Add a verbose-mode breadcrumb when install replays a locked transitive graph -- scope boundary: PR scope is docs clarification; this requires CLI logging/code behavior changes.
- (panel) Add a search-symptom sentence to the consumer guide note -- scope boundary: PR scope is the lockfile replay contract; this is search/conversion copy tuning.
- (panel) Consolidate the determinism contract into one canonical docs definition with cross-links -- scope boundary: PR scope is three-surface clarification for [BUG] apm install replays stale locked commits and produces a different dependency graph than a fresh install #2007; this is broader docs architecture work.
Lint contract
uv run --extra dev ruff check src/ tests/, uv run --extra dev ruff format --check src/ tests/, pylint R0801, and bash scripts/lint-auth-signals.sh all exited 0 before the final push.
CI
All PR checks are green on 08ffa5c35d65b724ef9d0cb4d30e644cc042c938 after 0 CI fix iteration(s). Evidence includes CI run https://github.com/microsoft/apm/actions/runs/28681807993 and merge gate https://github.com/microsoft/apm/actions/runs/28681808012.
Mergeability status
Captured from gh pr view 2010 --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 |
|---|---|---|---|---|---|---|---|---|---|---|
| #2010 | 08ffa5c |
ship_with_followups | 3 | 4 | 3 | 2 | green | MERGEABLE | BLOCKED | pending required review |
Convergence
3 outer iteration(s); 2 Copilot round(s). Final panel verdict: ship_with_followups.
Ready for maintainer review.
Full per-persona findings
Python Architect
No findings.
CLI Logging Expert
No findings.
DevX UX Expert
No findings.
Supply Chain Security Expert
No findings.
OSS Growth Hacker
- [nit] The callout could include a one-sentence symptom to help searchers land here at
docs/src/content/docs/consumer/install-packages.md:145
Users who hit [BUG] apm install replays stale locked commits and produces a different dependency graph than a fresh install #2007 may search for different graph after upstream change. A leading symptom sentence could improve discoverability, but this is a micro-conversion optimization beyond the narrow contract clarification.
Auth Expert -- inactive
PR #2010 touches only consumer/install-packages, reference/cli/install, and reference/lockfile-spec docs -- no auth, token, or AuthResolver surface.
Doc Writer
No findings.
Test Coverage Expert -- inactive
Pure documentation change; zero src/**/*.py touched.
Performance Expert -- inactive
Docs-only change; no install/update hot path code or performance claim.
This panel is advisory. It does not block merge. Re-apply the panel-review label after addressing feedback to re-run.
Add a hermetic end-to-end regression test that drives apm install through local git fixtures to prove lockfile replay keeps the locked transitive dependencies graph while a fresh install resolves current upstream manifests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
APM Review Panel:
|
| Persona | B | R | N | Takeaway |
|---|---|---|---|---|
| Python Architect | 0 | 0 | 0 | Docs + one hermetic E2E test only; no src/ changes at head. No architectural concern; ship. |
| CLI Logging Expert | 0 | 0 | 0 | No CLI output code touched; docs/tests only. Validation green. No logging-UX findings. |
| DevX UX Expert | 0 | 0 | 0 | Docs/tests-only change. Prior DevX concern confirmed folded. No CLI surface, help text, error wording, or first-run flow affected. |
| Supply Chain Security Expert | 0 | 0 | 0 | Docs/tests-only change with urlparse-based remote scheme guard covering https, http, ssh, git, and git@ protocols. No concerns. |
| OSS Growth Hacker | 0 | 0 | 0 | Prior growth concern resolved: consumer note now leads with user-benefit lockfile pinning and links the spec. |
| Doc Writer | 0 | 0 | 0 | No material documentation issue remains; consumer and CLI summaries link to the lockfile specification. |
| Test Coverage Expert | 0 | 0 | 0 | All critical surfaces exercised: regular replay, --frozen replay, fresh regeneration, hermetic local git, and network sentinel. |
B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.
Architecture
classDiagram
direction LR
class CliRunner {
<<TestHarness>>
+invoke(cli, args) Result
}
class GitHubPackageDownloader {
<<Strategy>>
+resolve_git_reference(dep_ref) ResolvedReference
+_clone_with_fallback(url, path) Repo
}
class ResolvedReference {
<<ValueObject>>
+original_ref str
+ref_type GitReferenceType
+resolved_commit str
}
class LockfilePhase {
<<Phase>>
+replay(lock_data) list
}
class test_lockfile_transitive_replay_e2e {
<<TestModule>>
+test_lockfile_replay_keeps_locked_transitive_dependencies_graph()
}
test_lockfile_transitive_replay_e2e ..> CliRunner : invokes install
test_lockfile_transitive_replay_e2e ..> GitHubPackageDownloader : monkeypatches
test_lockfile_transitive_replay_e2e ..> ResolvedReference : stubs return
GitHubPackageDownloader ..> ResolvedReference : returns
GitHubPackageDownloader ..> LockfilePhase : feeds
note for GitHubPackageDownloader "Strategy: swapped to local-clone\nin test via monkeypatch"
class test_lockfile_transitive_replay_e2e:::touched
classDef touched fill:#fff3b0,stroke:#d47600
flowchart TD
A["test entry: test_lockfile_replay_keeps_locked_transitive_dependencies_graph"] --> B["[FS] _init_repo: create local git fixtures"]
B --> C["[FS] _write_consumer: write apm.yml manifest"]
C --> D["_invoke_install: monkeypatch network boundary"]
D --> E{"subprocess.Popen guard"}
E -->|"remote URL detected"| F["AssertionError raised"]
E -->|"local only"| G["original_popen proceeds"]
D --> H["CliRunner.invoke cli install"]
H --> I["[FS] lockfile written: apm.lock.yaml"]
I --> J["Assert: replay preserves locked SHAs"]
J --> K["[FS] unlink lockfile + re-install"]
K --> L["Assert: fresh resolve picks new upstream graph"]
Recommendation
Merge as-is. Zero residual findings across all active panelists, validation evidence is comprehensive (E2E + mutation break + 10,155-test integration suite + lint), and all prior follow-ups confirmed folded. No post-merge action required.
Full per-persona findings
Python Architect
No findings.
CLI Logging Expert
No findings.
DevX UX Expert
No findings.
Supply Chain Security Expert
No findings.
OSS Growth Hacker
No findings.
Auth Expert -- inactive
PR #2010 touches only docs and integration tests; no production auth, token, credential, or host resolution code changed.
Doc Writer
No findings.
Test Coverage Expert
No findings.
Performance Expert -- inactive
PR #2010 touches only documentation and integration-test files. No production resolver, cache, download, or materialization code is changed, and the PR makes no performance claim.
This panel is advisory. It does not block merge. Re-apply the panel-review label after addressing feedback to re-run.
WHY
Issue #2007 reported different graphs between replaying an existing lockfile and doing a fresh install after an upstream transitive package moved an entry from dependencies.apm to devDependencies.apm.
Per the P6 Reliability over magic reservation, lockfile replay determinism is intentional: unchanged Git dependencies must keep using the recorded commits instead of silently following upstream manifest moves.
WHAT
HOW TO TEST
APM_E2E_TESTS=1 uv run pytest tests/integration/test_lockfile_transitive_replay_e2e.py -vAPM_E2E_TESTS=1 uv run pytest tests/integration -m "not requires_network_integration and not live" --ignore=tests/integration/test_runtime_smoke.py -p no:cacheprovider -quv run --extra dev ruff check src/ tests/ && uv run --extra dev ruff format --check src/ tests/ && uv run --extra dev python -m pylint --disable=all --enable=R0801 --min-similarity-lines=10 --fail-on=R0801 src/apm_cli/ && bash scripts/lint-auth-signals.shDeterminism note
This PR intentionally preserves lockfile replay semantics. The resolver already queues root dependencies plus root devDependencies, but transitively queues only dependencies.apm; transitive devDependencies.apm are not installed as a consumer graph leak.
Closes #2007