fix(gates): waive lockfile-exists for a package, which is not an install root
plugins/onedev is the first plugin package to declare a real dependency, and that arms a check every previous plugin left vacuous. apm treats any directory holding both apm.yml and apm.lock.yaml as an install root; a package is not one, so there is no green state for it. Without a package lockfile, lockfile-exists fails outright. With one, it passes and thereby arms the other nine checks, where drift then demands the dependency's skills be deployed inside the package and apm lock leaves an apm_modules/ tree behind. scripts/apm-audit-ci.sh replaces the inline bash -c loop and waives that single check for a non-root manifest. It fails closed on three axes: the root is never waived; the failing check must be lockfile-exists and no other, asserted by matching "1 of 1 check(s) failed"; and unrecognised output fails. Dropping --ci for package directories was the smaller change and is wrong. Verified on apm 0.28.0 against a scratch package whose dependency entry carried no git/path/registry field: apm audit --ci exits 1 naming it, while plain apm audit exits 0 and says nothing. Malformed-dependency detection is the reason gates.md gives for auditing packages at all, and a package with dependencies is the only kind that can carry a malformed dependency entry. The waiver matches on apm's stdout, so an apm upgrade rewording either line turns it off. That fails the push rather than hiding a defect. Also records the onedev entry in apm.lock.yaml, which PR #136 could not carry because the plugin was not yet resolvable from the remote's main. ADR: 0026 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NwD8Egs5r4ndqeFLmhusX2
This commit is contained in:
60
docs/adr/0026-a-plugin-package-is-not-an-install-root.md
Normal file
60
docs/adr/0026-a-plugin-package-is-not-an-install-root.md
Normal file
@@ -0,0 +1,60 @@
|
||||
# A plugin package is not an install root — `apm-audit-ci` waives `lockfile-exists` for one
|
||||
|
||||
**Status:** Accepted (2026-09-20)
|
||||
|
||||
`plugins/onedev` is the first plugin package in this repo to declare a real dependency. It pins
|
||||
`code.onedev.io/onedev/tod#v4.3.4` so that a consumer installing `onedev` from the holocron
|
||||
marketplace picks up OneDev's eight TOD skills transitively — a `marketplace.packages` entry takes a
|
||||
local `source:` path, so a third-party repo cannot be listed for redistribution on its own, and the
|
||||
wrapper is the only mechanism that carries it.
|
||||
|
||||
That arms a check every previous plugin left vacuous, and leaves the package with no green state.
|
||||
|
||||
`apm audit --ci` in a plugin directory runs one check, `lockfile-exists`. While every plugin
|
||||
`apm.yml` declared `dependencies: {apm: [], mcp: []}` it reported `No dependencies declared --
|
||||
lockfile not required` and passed. `plugins/onedev` declares dependencies, so (verified against apm
|
||||
0.28.0):
|
||||
|
||||
- **without** a package `apm.lock.yaml` it fails — `apm.yml declares dependencies but apm.lock.yaml
|
||||
is absent`, reported as `1 of 1 check(s) failed`
|
||||
- **with** one it passes, and passing arms the other nine checks. `drift` then fails reporting eight
|
||||
unintegrated files at `.agents/skills/<name>/SKILL.md` — it wants the dependency's skills
|
||||
*deployed inside the package*. Generating the lockfile with `apm lock` also creates an
|
||||
`apm_modules/` tree in there.
|
||||
|
||||
The cause is that apm treats any directory holding both `apm.yml` and `apm.lock.yaml` as an **install
|
||||
root**. A plugin package is not one: it is content to be installed somewhere else. The second state
|
||||
is not a stricter version of the first, it is a category error — a package has no deployment target
|
||||
of its own, so there is nothing for a drift check to be right about.
|
||||
|
||||
The hook therefore waives `lockfile-exists`, and only that, for a non-root manifest.
|
||||
`scripts/apm-audit-ci.sh` replaces the inline `bash -c` loop that `.pre-commit-config.yaml` carried.
|
||||
The waiver fails closed on three axes: the root manifest is never waived whatever it reports; the
|
||||
failing check must be `lockfile-exists` and no other, asserted by matching `1 of 1 check(s) failed`,
|
||||
so any second failing check changes the count and fails the push normally; and output apm does not
|
||||
produce in the recognised shape is a failure.
|
||||
|
||||
**Dropping `--ci` for package directories was rejected.** It was the smaller change — plain
|
||||
`apm audit` in a plugin directory reports `No apm.lock.yaml found -- nothing to scan` and exits 0, so
|
||||
the loop would have gone green with a one-word edit. It is wrong. Verified on apm 0.28.0 against a
|
||||
scratch package whose dependency entry carried no `git`/`path`/`registry` field: `apm audit --ci`
|
||||
exits 1 naming the missing field, while plain `apm audit` exits 0 and says nothing. Malformed-
|
||||
dependency detection is the reason `docs/spec/gates.md` gives for auditing packages at all, and a
|
||||
package *with* dependencies is the only kind that can carry a malformed dependency entry — so the
|
||||
cheap fix would have discarded the check precisely where it earns its keep, in the one package that
|
||||
newly needs it.
|
||||
|
||||
Two alternatives were rejected for making the wrapper pointless or the repo fragile. Dropping the
|
||||
dependency from `plugins/onedev` turns the gate green immediately, but a consumer installing
|
||||
`onedev` from the marketplace then receives an empty package, which removes the only reason the
|
||||
wrapper exists. Committing a package lockfile and running `apm install` inside the package satisfies
|
||||
`drift` on a machine that has done so, but makes `deployed-files-present` a fresh-clone failure and
|
||||
commits this repo to maintaining a nested install root per package.
|
||||
|
||||
The weak point is stated rather than designed away: the waiver matches on apm's stdout, so an apm
|
||||
upgrade that rewords either line silently turns it off. That direction is safe — it fails the push
|
||||
rather than hiding a defect. Re-verify against the new output and update the two patterns rather
|
||||
than widening them.
|
||||
|
||||
This changes shared enforcement, which is why it is recorded here rather than left as a comment.
|
||||
`docs/spec/gates.md`'s `apm-audit-ci` section carries the operative detail.
|
||||
Reference in New Issue
Block a user