You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@magpie.apache.org by po...@apache.org on 2026/10/05 15:09:52 UTC
(magpie) branch main updated: fix(setup-preflight): read the local lock under its own keys (#1523)
This is an automated email from the ASF dual-hosted git repository.
potiuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/magpie.git
The following commit(s) were added to refs/heads/main by this push:
new c99d0e00 fix(setup-preflight): read the local lock under its own keys (#1523)
c99d0e00 is described below
commit c99d0e0061c4413e6dc8ffa8284b1eeb6a7eb204
Author: Davide Polato <dp...@apache.org>
AuthorDate: Mon Oct 5 17:09:46 2026 +0200
fix(setup-preflight): read the local lock under its own keys (#1523)
The pre-flight parsed `.apache-magpie.local.lock` with the committed
lock's parser, which accepts only `method`, `url`, `min_version`, `ref`,
`commit` and `source`. The local lock that `install.md` and `upgrade.md`
tell the agent to write uses the keys `locks.md` documents for it:
`source_method`, `source_url`, `source_ref`, `fetched_commit` and
`fetched_at`. Every snapshot install (git-branch, git-tag, svn-zip)
therefore got `snapshot-unreadable` and stopped at `step-2`.
`lockfile.parse_local` reads the local lock with that key set and still
rejects unknown keys. The drift check compares each committed key with
its local counterpart (`method`/`source_method`, `url`/`source_url`,
`ref`/`source_ref`, `commit`/`fetched_commit`), as `upgrade.md` Step 1
does. Finding codes, facts keys and sections are unchanged, and so is
the committed-lock parser.
The tests wrote the local lock with the committed lock's keys, which hid
the bug; they now write the documented format. The adoption-and-setup
spec names the local-lock keys the drift check reads.
Closes #1491
Signed-off-by: Davide Polato <dp...@apache.org>
---
.../skills/setup/setup_preflight/core.py | 20 ++++--
.../skills/setup/setup_preflight/lockfile.py | 36 +++++++++-
tools/setup-preflight/tests/test_core.py | 83 ++++++++++++++++++----
tools/spec-loop/specs/adoption-and-setup.md | 2 +
4 files changed, 119 insertions(+), 22 deletions(-)
diff --git a/plugins/magpie-setup/skills/setup/setup_preflight/core.py b/plugins/magpie-setup/skills/setup/setup_preflight/core.py
index c995c83a..b38a2d34 100644
--- a/plugins/magpie-setup/skills/setup/setup_preflight/core.py
+++ b/plugins/magpie-setup/skills/setup/setup_preflight/core.py
@@ -53,7 +53,7 @@ from datetime import date, datetime
from pathlib import Path
from . import sections
-from .lockfile import Lock, MalformedLock, load
+from .lockfile import Lock, MalformedLock, load, parse_local
from .version import InvalidVersion, below
LOCAL_DIR = ".apache-magpie-local"
@@ -65,6 +65,14 @@ CACHE_NAME = ".preflight-cache.json"
TRUSTED_MARKETPLACE = "apache/magpie"
SNAPSHOT_METHODS = frozenset({"svn-zip", "git-tag", "git-branch"})
+#: Each pinned key in the committed lock, and the local-lock key that records
+#: what this machine actually fetched for it (`locks.md`, `upgrade.md` Step 1).
+LOCAL_COUNTERPART = {
+ "method": "source_method",
+ "url": "source_url",
+ "ref": "source_ref",
+ "commit": "fetched_commit",
+}
#: The framework checkout linking its own in-repo `skills/` source. The
#: skills *are* the working tree, so there is no snapshot or floor to drift.
LOCAL_METHOD = "local"
@@ -125,15 +133,13 @@ def _snapshot_findings(lock: Lock, root: Path) -> list[Finding]:
)
]
try:
- from .lockfile import parse as parse_lock
-
- local_lock = parse_lock(local.read_text(encoding="utf-8"))
+ local_lock = parse_local(local.read_text(encoding="utf-8"))
except MalformedLock as exc:
return [Finding("project", "snapshot-unreadable", "step-2", {"error": str(exc)})]
drift = {
- key: {"project": getattr(lock, key), "machine": getattr(local_lock, key)}
- for key in ("method", "url", "ref", "commit")
- if getattr(lock, key) is not None and getattr(lock, key) != getattr(local_lock, key)
+ key: {"project": getattr(lock, key), "machine": getattr(local_lock, local_key)}
+ for key, local_key in LOCAL_COUNTERPART.items()
+ if getattr(lock, key) is not None and getattr(lock, key) != getattr(local_lock, local_key)
}
if drift:
return [Finding("project", "snapshot-drift", "step-2", {"differs": drift})]
diff --git a/plugins/magpie-setup/skills/setup/setup_preflight/lockfile.py b/plugins/magpie-setup/skills/setup/setup_preflight/lockfile.py
index d306afac..77809480 100644
--- a/plugins/magpie-setup/skills/setup/setup_preflight/lockfile.py
+++ b/plugins/magpie-setup/skills/setup/setup_preflight/lockfile.py
@@ -15,13 +15,15 @@
# specific language governing permissions and limitations
# under the License.
-"""Read `.apache-magpie.lock` — the one shape, not general YAML.
+"""Read `.apache-magpie.lock` and `.apache-magpie.local.lock` — fixed shapes, not general YAML.
The lock is written by `setup`, never by hand, and its grammar is fixed by
[`locks.md`]: `key: value` scalars at column 0, a `plugins:` sequence of
`- name`, and a `reconciled:` mapping whose `skills:` child maps a skill's
frontmatter `name:` to its `surface_hash`. Comments and blank lines are
-ignored.
+ignored. The gitignored `.apache-magpie.local.lock` is flat `key: value`
+lines under its own keys (`source_method`, `source_url`, `source_ref`,
+`fetched_commit`, `fetched_at`) and is read by `parse_local`.
A real YAML parser is the obvious alternative and is rejected for one
reason: this module is copied into an adopter's gitignored
@@ -37,7 +39,7 @@ never actually saw.
from __future__ import annotations
-from dataclasses import dataclass, field
+from dataclasses import dataclass, field, fields
from pathlib import Path
@@ -64,6 +66,17 @@ class Lock:
reconciled: Reconciled | None = None
+@dataclass
+class LocalLock:
+ """`.apache-magpie.local.lock`: what this machine fetched, per `locks.md`."""
+
+ source_method: str | None = None
+ source_url: str | None = None
+ source_ref: str | None = None
+ fetched_commit: str | None = None
+ fetched_at: str | None = None
+
+
def _strip_comment(line: str) -> str:
return line.split("#", 1)[0].rstrip()
@@ -131,6 +144,23 @@ def parse(text: str) -> Lock:
return lock
+def parse_local(text: str) -> LocalLock:
+ """Parse the local lock: `key: value` lines only, under its own keys."""
+ local = LocalLock()
+ for raw in text.splitlines():
+ line = _strip_comment(raw)
+ if not line.strip():
+ continue
+ if line[0] == " " or ":" not in line:
+ raise MalformedLock(f"not a key: value line: {raw!r}")
+ key, _, value = line.partition(":")
+ key = key.rstrip()
+ if key not in {f.name for f in fields(LocalLock)}:
+ raise MalformedLock(f"unknown key: {key!r}")
+ setattr(local, key, value.strip())
+ return local
+
+
def load(path: Path) -> Lock | None:
"""Parse the lock at `path`, or `None` when there is no lock.
diff --git a/tools/setup-preflight/tests/test_core.py b/tools/setup-preflight/tests/test_core.py
index d6e6988c..1638dad3 100644
--- a/tools/setup-preflight/tests/test_core.py
+++ b/tools/setup-preflight/tests/test_core.py
@@ -114,28 +114,87 @@ def test_a_snapshot_method_without_a_local_lock_was_never_fetched(project: Path)
assert codes(project_findings(project, None)) == ["snapshot-never-fetched"]
+GIT_TAG_PIN = """\
+method: git-tag
+url: https://github.com/apache/magpie.git
+ref: v0.2.0
+commit: 1111111111111111111111111111111111111111
+"""
+
+FETCHED = """\
+# .apache-magpie.local.lock — gitignored; per-machine.
+
+source_method: {method}
+source_url: {url}
+source_ref: {ref}
+fetched_commit: {commit}
+fetched_at: 2026-10-05T12:00:00Z
+"""
+
+
+def write_local_lock(
+ root: Path,
+ *,
+ method: str = "git-tag",
+ url: str = "https://github.com/apache/magpie.git",
+ ref: str = "v0.2.0",
+ commit: str = "1111111111111111111111111111111111111111",
+) -> None:
+ """The local lock in the shape `install.md` and `upgrade.md` write it."""
+ (root / ".apache-magpie.local.lock").write_text(
+ FETCHED.format(method=method, url=url, ref=ref, commit=commit), encoding="utf-8"
+ )
+
+
+def test_a_local_lock_matching_the_pin_is_silent(project: Path) -> None:
+ """The local lock records what was fetched under its own keys
+ (`source_method`, `source_url`, `source_ref`, `fetched_commit`), and each
+ is compared with the committed key it records."""
+ write_lock(project, GIT_TAG_PIN)
+ write_local_lock(project)
+ assert project_findings(project, None) == []
+
+
+def test_a_local_lock_with_an_unknown_key_is_unreadable(project: Path) -> None:
+ write_lock(project, GIT_TAG_PIN)
+ (project / ".apache-magpie.local.lock").write_text("method: git-tag\nref: v0.2.0\n", encoding="utf-8")
+ found = project_findings(project, None)
+ assert codes(found) == ["snapshot-unreadable"]
+ assert found[0].facts == {"error": "unknown key: 'method'"}
+
+
def test_a_snapshot_ref_mismatch_is_drift(project: Path) -> None:
- write_lock(project, "method: git-tag\nref: v0.2.0\n")
- (project / ".apache-magpie.local.lock").write_text("method: git-tag\nref: v0.1.0\n")
+ write_lock(project, GIT_TAG_PIN)
+ write_local_lock(project, ref="v0.1.0")
found = project_findings(project, None)
assert codes(found) == ["snapshot-drift"]
- differs = found[0].facts["differs"]
- assert isinstance(differs, dict)
- assert differs["ref"] == {"project": "v0.2.0", "machine": "v0.1.0"}
+ assert found[0].facts["differs"] == {"ref": {"project": "v0.2.0", "machine": "v0.1.0"}}
+
+
+def test_a_fetched_commit_other_than_the_pinned_one_is_drift(project: Path) -> None:
+ write_lock(project, GIT_TAG_PIN)
+ write_local_lock(project, commit="2222222222222222222222222222222222222222")
+ found = project_findings(project, None)
+ assert codes(found) == ["snapshot-drift"]
+ assert found[0].facts["differs"] == {
+ "commit": {
+ "project": "1111111111111111111111111111111111111111",
+ "machine": "2222222222222222222222222222222222222222",
+ }
+ }
def test_a_snapshot_method_or_url_mismatch_is_drift(project: Path) -> None:
"""A different fetch method or source needs a re-install, not an upgrade;
the facts carry which key differs so the rule can say which."""
- write_lock(project, "method: git-tag\nurl: https://a.example/magpie\nref: v0.2.0\n")
- (project / ".apache-magpie.local.lock").write_text(
- "method: git-branch\nurl: https://b.example/magpie\nref: v0.2.0\n"
- )
+ write_lock(project, GIT_TAG_PIN)
+ write_local_lock(project, method="git-branch", url="https://b.example/magpie")
found = project_findings(project, None)
assert codes(found) == ["snapshot-drift"]
- differs = found[0].facts["differs"]
- assert isinstance(differs, dict)
- assert set(differs) == {"method", "url"}
+ assert found[0].facts["differs"] == {
+ "method": {"project": "git-tag", "machine": "git-branch"},
+ "url": {"project": "https://github.com/apache/magpie.git", "machine": "https://b.example/magpie"},
+ }
LOCAL = """\
diff --git a/tools/spec-loop/specs/adoption-and-setup.md b/tools/spec-loop/specs/adoption-and-setup.md
index d451081f..422b73aa 100644
--- a/tools/spec-loop/specs/adoption-and-setup.md
+++ b/tools/spec-loop/specs/adoption-and-setup.md
@@ -330,6 +330,8 @@ committed version with drift detection.
because an upgrade cannot fix it. `setup_preflight` compares all four
keys (`method` and `url` since #1435), and its `step-2` rules section
carries the three remedies.
+ Each committed key is compared with the local lock's own key for it:
+ `source_method`, `source_url`, `source_ref`, `fetched_commit` (#1491).
5. Override files can be discovered and surfaced to skills without
editing upstream skill bodies, and override text cannot weaken the
safety/confidentiality baseline.