fix(gates): waive lockfile-exists for a package, which is not an install root #137

Claude · 2026-09-20T20:25:42Z

Claude commented

2026-09-20 20:25:42 +00:00

Why

plugins/onedev (merged in #136) 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 plugin package is not one — it is content to be installed somewhere else — so it has no green state:

Package state	`apm audit --ci` result
no `apm.lock.yaml`	`lockfile-exists` fails — "apm.yml declares dependencies but apm.lock.yaml is absent"
with `apm.lock.yaml`	`lockfile-exists` passes, arming the other nine; `drift` then fails demanding the dependency's 8 skills be deployed inside the package at `.agents/skills/<name>/SKILL.md`. `apm lock` also leaves an `apm_modules/` tree in there.

This blocked every push from the repo, not just OneDev work. #136 had to be pushed with --no-verify as a result.

What

scripts/apm-audit-ci.sh replaces the inline bash -c loop in .pre-commit-config.yaml and waives exactly one failure: lockfile-exists, for a non-root manifest. It 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 normally;
output apm does not produce in the recognised shape is a failure.

Dropping `--ci` for packages was considered and rejected

It was the smaller change — plain apm audit in a package 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:

Mode	Malformed dependency	Exit
plain `apm audit`	not caught	0
`apm audit --ci`	caught, names the field	1

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 cheap fix would have discarded the check precisely where it earns its keep.

Known weak point

The waiver matches on apm's stdout, so an apm upgrade rewording either line silently turns it off. That direction is safe — it fails the push rather than hiding a defect. Re-verify and update the two patterns rather than widening them.

Also in this commit

apm.lock.yaml gains the onedev entry. #136 could not carry it: the root lockfile cannot record plugins/onedev until apm can resolve it from the remote's main, and it could not be on main until #136 merged.

Verification

scripts/apm-audit-ci.sh — shellcheck clean
waiver fires: apm audit --ci: waived lockfile-exists in plugins/onedev/ (package, not an install root) → exit 0
fails closed: a malformed-dependency fixture dropped into plugins/_tmpbroken/ → exit 1 (fixture removed)
full suite: 19 passed, 0 skipped, 0 failed
pushed with hooks enabled — all 10 pre-push gates passed, unlike feat(onedev): redistribute TOD's agent skills through a plugin wrapper (#136)

Two tests caught real defects in the first draft and are fixed here:

test-no-pipefail-early-exit-grep.sh — printf ... | grep -q under set -o pipefail races: grep -q exits on first match, SIGPIPEs the writer, and the writer's death becomes the pipeline's status. Now uses here-strings.
test-adr0020-contract.sh — pins every repo-authored hook's entry:; updated for the new one.

Docs

ADR-0026 — a plugin package is not an install root
docs/spec/gates.md — apm-audit-ci section rewritten; "six plugin packages" → seven; the old "arms itself the moment one does not" prediction recorded as having come true

🤖 Generated with Claude Code

https://claude.ai/code/session_01NwD8Egs5r4ndqeFLmhusX2

## Why `plugins/onedev` (merged in #136) 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 plugin package is not one — it is content to be installed somewhere else — so it has no green state: | Package state | `apm audit --ci` result | |---|---| | **no** `apm.lock.yaml` | `lockfile-exists` fails — "apm.yml declares dependencies but apm.lock.yaml is absent" | | **with** `apm.lock.yaml` | `lockfile-exists` passes, **arming the other nine**; `drift` then fails demanding the dependency's 8 skills be deployed *inside* the package at `.agents/skills/<name>/SKILL.md`. `apm lock` also leaves an `apm_modules/` tree in there. | This blocked every push from the repo, not just OneDev work. #136 had to be pushed with `--no-verify` as a result. ## What `scripts/apm-audit-ci.sh` replaces the inline `bash -c` loop in `.pre-commit-config.yaml` and waives **exactly one** failure: `lockfile-exists`, for a non-root manifest. It fails closed on three axes: 1. the root manifest is **never** waived, whatever it reports; 2. 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 normally; 3. output apm does not produce in the recognised shape is a failure. ## Dropping `--ci` for packages was considered and rejected It was the smaller change — plain `apm audit` in a package 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: | Mode | Malformed dependency | Exit | |---|---|---| | plain `apm audit` | **not caught** | 0 | | `apm audit --ci` | caught, names the field | 1 | 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 cheap fix would have discarded the check precisely where it earns its keep. ## Known weak point The waiver matches on apm's stdout, so an apm upgrade rewording either line silently turns it off. That direction is safe — it fails the push rather than hiding a defect. Re-verify and update the two patterns rather than widening them. ## Also in this commit `apm.lock.yaml` gains the `onedev` entry. #136 could not carry it: the root lockfile cannot record `plugins/onedev` until apm can resolve it from the remote's `main`, and it could not be on `main` until #136 merged. ## Verification - `scripts/apm-audit-ci.sh` — shellcheck clean - waiver fires: `apm audit --ci: waived lockfile-exists in plugins/onedev/ (package, not an install root)` → exit 0 - fails closed: a malformed-dependency fixture dropped into `plugins/_tmpbroken/` → exit 1 (fixture removed) - full suite: **19 passed, 0 skipped, 0 failed** - **pushed with hooks enabled — all 10 pre-push gates passed**, unlike #136 Two tests caught real defects in the first draft and are fixed here: - `test-no-pipefail-early-exit-grep.sh` — `printf ... | grep -q` under `set -o pipefail` races: `grep -q` exits on first match, SIGPIPEs the writer, and the writer's death becomes the pipeline's status. Now uses here-strings. - `test-adr0020-contract.sh` — pins every repo-authored hook's `entry:`; updated for the new one. ## Docs - **ADR-0026** — a plugin package is not an install root - `docs/spec/gates.md` — `apm-audit-ci` section rewritten; "six plugin packages" → seven; the old "arms itself the moment one does not" prediction recorded as having come true 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01NwD8Egs5r4ndqeFLmhusX2

Claude added 1 commit 2026-09-20 20:25:42 +00:00

fix(gates): waive lockfile-exists for a package, which is not an install root fbd030c7ea

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

Defame1297 merged commit af008b6d37 into main

2026-09-20 20:26:39 +00:00

Defame1297 deleted branch fix/apm-audit-ci-package-lockfile

2026-09-20 20:26:40 +00:00

Defame1297 referenced this issue from a commit

2026-09-20 20:26:40 +00:00

Merge pull request 'fix(gates): waive lockfile-exists for a package, which is not an install root' (#137) from fix/apm-audit-ci-package-lockfile into main

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Defame1297/holocron#137