feat(kyberforge): add apm-install/apm-workflow/apm-orchestrate, ADR-0015 #91
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: Defame1297/holocron#91
Reference in New Issue
Block a user
Delete Branch "feat/88-marketplace-apm-conversion"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Builds the operational tooling for Microsoft APM (Agent Package Manager) in
kyberforge, scoped by issue #88 and grilled/refined viagrill-with-docs.What's here
plugins/kyberforge/docs/research/docs/microsoft-apm/— research reference set (overview, installation, configuration, cli-reference, examples, troubleshooting, testing-and-validation, marketplace-and-registries, monorepo-and-repo-shapes)apm.yml+.apm/) outright, superseding ADR-0001;plugins/<name>/monorepo-hybrid layout survives,.claude-plugin/marketplace.json/plugin.jsonbecome compiled output viaapm compile/apm packapm-install(skill) —apmbinary install +apm runtime setup|list|status|removeapm-workflow(skill) — dispatch skill covering configure/scaffold, marketplace, compile/pack/publish, audit/validateapm-orchestrate(agent + Copilot pair) — deterministic counterpart toapm-workflowfor future subagent-dispatched multi-plugin workAll three new artifacts passed
skill-audit/agent-audittwice — once inline during authoring, once via an independent clean-context recheck — with zero findings both times.Explicitly out of scope (tracked separately)
plugin-author/marketplace-author/skill-author/agent-author/forge's routing to author.apm/-native contentapm.yml+.apm/and run the real conversionCloses #88.
Test plan
bash tests/run-tests.shpassed via pre-push hook (full suite + manifest/marketplace validation + Vale sync)skill-audit/agent-auditclean on all three new artifacts, verified twice (inline + independent recheck)apmCLI smoke test — deferred to #89/#90 when these skills are actually usedFull review via
/code-review high(multi-agent, independently converged across 3+ passes then re-verified against the actual files). This diff is all-markdown (ADR-0015, apm-orchestrate agent pair, apm-install/apm-workflow skills, research docs, 2 plugin.json bumps) — no executable code, so findings are documentation self-contradictions and interface gaps rather than crashes. 10 inline findings below, grouped: (1) ADR-0015 internally disagrees with itself on when ADR-0001 is superseded and on its own stated scope-rationale; (2) apm-orchestrate's operation vocabulary (compile-manifest-check, add-package, add-marketplace) isn't grounded in apm-workflow's actual CLI verbs anywhere; (3) a Hard Rule in apm-orchestrate.md can never fire given the agent's declared tools; (4) apm-orchestrate.md and apm-orchestrate.agent.md have already drifted on the Output contract; (5) apm-workflow's reference docs duplicate the research docs and one recommends raw git shell instead of this repo's own git-commits/git-remotes skills; (6) CONTEXT.md's glossary and ADR-0001 get no forward-pointer to ADR-0015 despite both being living documents this ADR claims are affected. None of these are blocking code bugs, but several undermine the PR's own acceptance criteria ("internally consistent with the microsoft-apm research docs") and one (the dead Hard Rule) is a safety-relevant correctness issue in an agent definition. Requesting changes rather than approving outright given the ADR self-contradiction and the undocumented orchestrator operations — both are cheap to fix and load-bearing for issues #89/#90 that build on this.@@ -0,0 +28,4 @@and per-provider `plugin.json` files become **compiled output** via `apm compile`/`apm pack`,generated from `apm.yml` + `.apm/` per plugin, extensible to other `apm runtime`-supportedproviders without hand-maintaining a separate manifest per provider.- **This directly supersedes ADR-0001** ("Skills are distributed via plugins... each pluginThis Decision bullet states 'This directly supersedes ADR-0001' as a present-tense fact, but the Consequences section (line 63) says 'ADR-0001 is superseded once issue #90 executes' — a future, conditional event. The same document licenses opposite conclusions depending which section a reader lands on. Pick one framing (recommend: conditional, matching Consequences) and make the Decision bullet match it.
@@ -0,0 +40,4 @@- Actually translating the existing plugins into `apm.yml` + `.apm/` and running the realconversion is deferred to issue #90(https://git.dev.rkdr.net/Defame1297/holocron/issues/90).- `CONTEXT.md`'s "Plugin"/"Skill"/"Plugin marketplace" glossary entries remain accurate asThis says CONTEXT.md's Plugin/Skill/Plugin-marketplace glossary 'remains accurate as written until issue #90', but neither CONTEXT.md nor ADR-0001 gets any status marker or forward-pointer to this ADR. CONTEXT.md is mandated reading 'at the start of every session' per AGENTS.md, and this repo's own convention elsewhere (e.g. ADR-0007/ADR-0010) is to add an inline 'Superseded by' / 'Update' note on the older doc. Without that, an agent consulting CONTEXT.md or ADR-0001 directly has no signal a foundational change is pending and will keep hand-authoring plugin.json/marketplace.json exactly as this ADR says is being phased out.
@@ -0,0 +54,4 @@specifically because Vale tooling is generic and repo-agnostic, not holocron-marketplace-specific(see `CONTEXT.md`'s "lint plugin" entry) — the same argument applies to a generic `apm` CLIwrapper. Rejected anyway, in favor of `kyberforge`, because this tooling's scope is specificallyconverting *this* repo's marketplace, not standing up a reusable generic apm toolkit for otherThe rationale for rejecting a standalone
plugins/apm/plugin claims this tooling's scope is 'specifically converting this repo's marketplace, not standing up a reusable generic apm toolkit' — but the shipped apm-install/apm-workflow SKILL.md files are fully generic, repo-agnostic APM CLI documentation with essentially no holocron-specific content. The stated justification doesn't match what was actually built, which will make the placement decision look post-hoc to a future reviewer.agree that the created files are agnostic, but they still fit the kyberforge purpose.
@@ -0,0 +64,4 @@- ADR-0006 (plugin-version-parity) will need a third file, `apm.yml`, folded into its paritycheck once #90 lands — not resolved by this ADR.- ADR-0010 (agent sources relocated outside agents dir) needs revisiting once agents move under`.apm/agents/` with the `.agent.md` extension — not resolved by this ADR.Attributes the
.agent.mdextension to the future APM conversion ('needs revisiting once agents move under .apm/agents/ with the .agent.md extension'), but.agent.mdis this repo's pre-existing Copilot-pairing convention (ADR-0005/ADR-0010), already in use by this very diff's own apm-orchestrate.agent.md. Whoever executes #90 could read this as license to redesign an already-settled, orthogonal naming convention that has nothing to do with the APM move.@@ -0,0 +58,4 @@7. Catch and handle apm errors: retry once for a dependency-not-yet-scaffolded failure after the caller confirms the dependency exists; otherwise return error structure with diagnostics8. Aggregate all outputs and return as structured JSON## Outputapm-orchestrate.md and apm-orchestrate.agent.md duplicate ~95% of their content verbatim and have already drifted on the Output section: the .md version gives an explicit JSON schema with enumerated error codes (not_confirmed | apm_unavailable | manifest_invalid | dependency_unresolved | publish_failed); this .agent.md version has only a one-sentence prose summary that omits the error-code enum entirely. A future edit to one contract has nothing forcing the sibling to follow.
@@ -0,0 +20,4 @@These are non-negotiable regardless of `confirm` or any skill-local override:- `apm publish` claims a version on a registry — treat it as irreversible. Refuse without explicit `confirm: true`; always dispatch with `--dry-run -v` first and surface that output to the caller before the real publish, even when `confirm: true` was given.- MCP server secrets in any `apm.yml` content this orchestrator writes or edits must use `${VAR}` indirection — never a literal value.This Hard Rule governs 'any apm.yml content this orchestrator writes or edits', but the agent declares only
tools: Bash, Read(line 6) and its own Scope (line 13) states it 'does not decide manifest content yourself, you delegate' — it has no Edit/Write tool and never touches apm.yml directly. The rule it exists to enforce has no code path that ever fires; it reads as an active safety gate but is dead text. Either grant the capability this rule assumes, or move the rule to wherever manifest content actually gets written.@@ -0,0 +38,4 @@## Inputs- **operation:** string, one of:- configure: init-package, compile-manifest-check (does `apm.yml` parse and match `type:`)The
compile-manifest-checkoperation has no corresponding apm CLI command anywhere in apm-workflow's references or the shipped microsoft-apm research docs (verified by grep across both). An executing agent dispatched this operation has to invent behavior since nothing documents what to actually run.@@ -0,0 +39,4 @@- **operation:** string, one of:- configure: init-package, compile-manifest-check (does `apm.yml` parse and match `type:`)- marketplace: init-marketplace, check-marketplace, add-package, add-marketplaceadd-packageandadd-marketplaceare never mapped to their underlyingapm marketplace package add/apm marketplace addcommands anywhere in this file, despite this same file's Hard Rule insisting these two directions must 'never guess from context alone.' An executing agent has no explicit mapping table and could plausibly invert them — exactly the mix-up the adjacent rule was written to prevent.@@ -0,0 +1,127 @@---This and the other three apm-workflow reference files (compile.md, audit.md, marketplace.md) restate their sibling research docs under docs/research/docs/microsoft-apm/ nearly verbatim (same command lists, same caveats, same CI YAML block in audit.md) — two sources of truth for the same facts. An upstream apm flag/behavior change applied to the research doc (the stated source of truth) can be missed here, leaving the actionable skill guidance silently stale while looking equally authoritative. Consider having these reference files point at the research docs rather than duplicating their content.
This is/should be done through the sources.md. Not sure this finding is correct
@@ -0,0 +11,4 @@$EDITOR apm.yml # 2. describe each packageapm marketplace check # 3. validate refs resolveapm pack # 4. build marketplace artifactsgit add apm.yml .claude-plugin/marketplace.jsonDocuments a raw
git add/git commit/git tag/git push --tagschain as the literal release step. AGENTS.md's 'Prefer plugin skills over raw shell' section states commits/remotes are owned bygit:git-commits/git:git-remotesand to fall back to raw shell 'only when no skill covers it' — an agent following this reference verbatim shells out directly in a repo that already has dedicated skills for exactly this operation.Full review findings (10 inline comments) — mainly the ADR-0015 self-contradiction (Decision vs. Consequences on when ADR-0001 is superseded) and the undocumented apm-orchestrate operations (compile-manifest-check, add-package/add-marketplace) that aren't grounded in apm-workflow's actual CLI verbs anywhere. Submitted as COMMENT rather than REQUEST_CHANGES — Gitea disallows self-review rejection since author and reviewer are the same account here. Treat the ADR contradiction and the dead Hard Rule (apm-orchestrate.md line 23) as should-fix before merge; the rest are lower-severity consistency/maintainability notes. Details inline.
@@ -0,0 +28,4 @@and per-provider `plugin.json` files become **compiled output** via `apm compile`/`apm pack`,generated from `apm.yml` + `.apm/` per plugin, extensible to other `apm runtime`-supportedproviders without hand-maintaining a separate manifest per provider.- **This directly supersedes ADR-0001** ("Skills are distributed via plugins... each pluginThis Decision bullet states 'This directly supersedes ADR-0001' as a present-tense fact, but the Consequences section (line 63) says 'ADR-0001 is superseded once issue #90 executes' — a future, conditional event. The same document licenses opposite conclusions depending which section a reader lands on. Pick one framing (recommend: conditional, matching Consequences) and make the Decision bullet match it.
@@ -0,0 +40,4 @@- Actually translating the existing plugins into `apm.yml` + `.apm/` and running the realconversion is deferred to issue #90(https://git.dev.rkdr.net/Defame1297/holocron/issues/90).- `CONTEXT.md`'s "Plugin"/"Skill"/"Plugin marketplace" glossary entries remain accurate asThis says CONTEXT.md's Plugin/Skill/Plugin-marketplace glossary 'remains accurate as written until issue #90', but neither CONTEXT.md nor ADR-0001 gets any status marker or forward-pointer to this ADR. CONTEXT.md is mandated reading 'at the start of every session' per AGENTS.md, and this repo's own convention elsewhere (e.g. ADR-0007/ADR-0010) is to add an inline 'Superseded by' / 'Update' note on the older doc. Without that, an agent consulting CONTEXT.md or ADR-0001 directly has no signal a foundational change is pending and will keep hand-authoring plugin.json/marketplace.json exactly as this ADR says is being phased out.
@@ -0,0 +54,4 @@specifically because Vale tooling is generic and repo-agnostic, not holocron-marketplace-specific(see `CONTEXT.md`'s "lint plugin" entry) — the same argument applies to a generic `apm` CLIwrapper. Rejected anyway, in favor of `kyberforge`, because this tooling's scope is specificallyconverting *this* repo's marketplace, not standing up a reusable generic apm toolkit for otherThe rationale for rejecting a standalone
plugins/apm/plugin claims this tooling's scope is 'specifically converting this repo's marketplace, not standing up a reusable generic apm toolkit' — but the shipped apm-install/apm-workflow SKILL.md files are fully generic, repo-agnostic APM CLI documentation with essentially no holocron-specific content. The stated justification doesn't match what was actually built, which will make the placement decision look post-hoc to a future reviewer.agree that the created files are agnostic, but they still fit the kyberforge purpose.
@@ -0,0 +64,4 @@- ADR-0006 (plugin-version-parity) will need a third file, `apm.yml`, folded into its paritycheck once #90 lands — not resolved by this ADR.- ADR-0010 (agent sources relocated outside agents dir) needs revisiting once agents move under`.apm/agents/` with the `.agent.md` extension — not resolved by this ADR.Attributes the
.agent.mdextension to the future APM conversion ('needs revisiting once agents move under .apm/agents/ with the .agent.md extension'), but.agent.mdis this repo's pre-existing Copilot-pairing convention (ADR-0005/ADR-0010), already in use by this very diff's own apm-orchestrate.agent.md. Whoever executes #90 could read this as license to redesign an already-settled, orthogonal naming convention that has nothing to do with the APM move.@@ -0,0 +58,4 @@7. Catch and handle apm errors: retry once for a dependency-not-yet-scaffolded failure after the caller confirms the dependency exists; otherwise return error structure with diagnostics8. Aggregate all outputs and return as structured JSON## Outputapm-orchestrate.md and apm-orchestrate.agent.md duplicate ~95% of their content verbatim and have already drifted on the Output section: the .md version gives an explicit JSON schema with enumerated error codes (not_confirmed | apm_unavailable | manifest_invalid | dependency_unresolved | publish_failed); this .agent.md version has only a one-sentence prose summary that omits the error-code enum entirely. A future edit to one contract has nothing forcing the sibling to follow.
@@ -0,0 +20,4 @@These are non-negotiable regardless of `confirm` or any skill-local override:- `apm publish` claims a version on a registry — treat it as irreversible. Refuse without explicit `confirm: true`; always dispatch with `--dry-run -v` first and surface that output to the caller before the real publish, even when `confirm: true` was given.- MCP server secrets in any `apm.yml` content this orchestrator writes or edits must use `${VAR}` indirection — never a literal value.This Hard Rule governs 'any apm.yml content this orchestrator writes or edits', but the agent declares only
tools: Bash, Read(line 6) and its own Scope (line 13) states it 'does not decide manifest content yourself, you delegate' — it has no Edit/Write tool and never touches apm.yml directly. The rule it exists to enforce has no code path that ever fires; it reads as an active safety gate but is dead text. Either grant the capability this rule assumes, or move the rule to wherever manifest content actually gets written.@@ -0,0 +38,4 @@## Inputs- **operation:** string, one of:- configure: init-package, compile-manifest-check (does `apm.yml` parse and match `type:`)The
compile-manifest-checkoperation has no corresponding apm CLI command anywhere in apm-workflow's references or the shipped microsoft-apm research docs (verified by grep across both). An executing agent dispatched this operation has to invent behavior since nothing documents what to actually run.@@ -0,0 +39,4 @@- **operation:** string, one of:- configure: init-package, compile-manifest-check (does `apm.yml` parse and match `type:`)- marketplace: init-marketplace, check-marketplace, add-package, add-marketplaceadd-packageandadd-marketplaceare never mapped to their underlyingapm marketplace package add/apm marketplace addcommands anywhere in this file, despite this same file's Hard Rule insisting these two directions must 'never guess from context alone.' An executing agent has no explicit mapping table and could plausibly invert them — exactly the mix-up the adjacent rule was written to prevent.@@ -0,0 +1,127 @@---This and the other three apm-workflow reference files (compile.md, audit.md, marketplace.md) restate their sibling research docs under docs/research/docs/microsoft-apm/ nearly verbatim (same command lists, same caveats, same CI YAML block in audit.md) — two sources of truth for the same facts. An upstream apm flag/behavior change applied to the research doc (the stated source of truth) can be missed here, leaving the actionable skill guidance silently stale while looking equally authoritative. Consider having these reference files point at the research docs rather than duplicating their content.
This is/should be done through the sources.md. Not sure this finding is correct
@@ -0,0 +11,4 @@$EDITOR apm.yml # 2. describe each packageapm marketplace check # 3. validate refs resolveapm pack # 4. build marketplace artifactsgit add apm.yml .claude-plugin/marketplace.jsonDocuments a raw
git add/git commit/git tag/git push --tagschain as the literal release step. AGENTS.md's 'Prefer plugin skills over raw shell' section states commits/remotes are owned bygit:git-commits/git:git-remotesand to fall back to raw shell 'only when no skill covers it' — an agent following this reference verbatim shells out directly in a repo that already has dedicated skills for exactly this operation.@@ -0,0 +15,4 @@## Pack```bashapm pack --dry-run # resolve and print; do not writeShould this not be part of the audit before we compile? or do we compile first and then audit?
All findings addressed in
e16c3dc(pushed). Threaded replies 405'd on this Gitea instance's API, so consolidating responses here instead of inline.Fixed:
plugins/apm/rationale mismatch — reworded to acknowledge the shipped skills are generic/repo-agnostic (agreeing with your comment), while keeping the kyberforge placement justified as a single-consumer tradeoff (same pattern as ADR-0011's gitea-workflow naming call)..agent.mdmisattribution in ADR-0010 forward-note — reworded so only the.apm/agents/directory move is flagged as an APM-move consequence;.agent.mdis called out as the pre-existing ADR-0005/ADR-0010 convention, unaffected.${VAR}secrets bullet from bothapm-orchestrate.md/.agent.md. It never fires (orchestrator has no Edit/Write, never touchesapm.yml); it's already correctly owned byapm-workflow/SKILL.md's Gotchas.compile-manifest-checkhas no backing CLI command — removed from the operation enum. Grepped the full microsoft-apm research doc set to confirm noapmcommand backs it.add-package/add-marketplacedirection ambiguity — added explicit CLI mappings inline (add-package → apm marketplace package add,add-marketplace → apm marketplace add), cross-checked against cli-reference.md.git-commits/git-remotesskills, per AGENTS.md's "prefer plugin skills over raw shell" rule.apm auditscans deployed/compiled output,apm compile/apm packproduces it, so compile/pack runs first. Added a one-line ordering note to compile.md.Investigated and left as-is (false positives against repo precedent):
sources.md= provenance link) is used identically by git-history, git-commits, gitea-issues, and vale-config across the repo. Not specific to this PR..md= full JSON schema with error-code enum,.agent.md= prose summary without it). Established convention, not drift.bash tests/run-tests.sh: 12 passed, 0 failed. Ready for re-review.Full re-review of the fix commit (
e16c3dc) against PR review conventions. 6 findings — mostly places where this same fix commit's own edits now contradict its own claims or leave dangling references. See inline comments.@@ -0,0 +44,4 @@- Actually translating the existing plugins into `apm.yml` + `.apm/` and running the realconversion is deferred to issue #90(https://git.dev.rkdr.net/Defame1297/holocron/issues/90).- `CONTEXT.md`'s "Plugin"/"Skill"/"Plugin marketplace" glossary entries remain accurate asADR-0015 states "CONTEXT.md's Plugin/Skill/Plugin-marketplace glossary entries remain accurate as written until issue #90 actually executes — this ADR does not update them," but this same PR's fix commit (
e16c3dc) edited CONTEXT.md's Plugin and Plugin marketplace entries to add an ADR-0015 forward-pointer sentence.Failure scenario: A reader trusts the ADR's own claim about its change boundary and skips re-checking CONTEXT.md, or a future auditor treats the ADR as the authoritative record of what this PR touched and gets it wrong — the document contradicts the diff it ships alongside.
@@ -0,0 +59,4 @@## OutputReturns structured JSON with operation status, result (output — or a list of per-package results when fanned out — plus resolved package-root/registry context), and optional error details with recovery suggestions.The Output contract here is collapsed to one prose sentence and omits the explicit error-code enum (
not_confirmed | apm_unavailable | manifest_invalid | dependency_unresolved | publish_failed) that the siblingapm-orchestrate.mdspecifies as JSON schema, so the two paired provider files disagree on the actual output contract.Failure scenario: A Copilot CLI caller consuming this file has no enumerated error codes to branch on programmatically, while a Claude Code caller relies on the exact enum from
apm-orchestrate.md— the two provider paths silently diverge on whaterror.codecan contain.@@ -0,0 +49,4 @@## Process1. Validate the request structure and check if `operation` is known2. Check the request against the Hard rules above (publish confirmation, secret indirection, marketplace-add direction, `type:` ordering, audit-vs-audit-ci, registries precondition) — refuse outright on violation, independent of `confirm`Process step 2 still tells the orchestrator to check the Hard rules for "secret indirection," but that Hard Rule bullet was deleted from the Hard rules section in this same PR's fix commit (
e16c3dc), leaving a dangling reference to a rule that no longer exists.Failure scenario: An executing agent reads Process step 2, tries to locate and enforce the named "secret indirection" Hard Rule, and finds nothing in the Hard rules list — it either hallucinates a check to satisfy the reference or silently skips it while the text implies the gate still exists.
Identical issue at
plugins/kyberforge/agents/apm-orchestrate.agent.md:52.@@ -0,0 +31,4 @@Escape hatches — combine as needed:- Pin a version: append `@vX.Y.Z` to the piped script's arguments, e.g. `curl -sSL https://aka.ms/apm-unix | sh -s -- @v1.2.3`.- Custom install directory: set `APM_INSTALL_DIR` before running, e.g. `APM_INSTALL_DIR=$HOME/.local/bin sh`.The "custom install directory" example
APM_INSTALL_DIR=$HOME/.local/bin shomits thecurl -sSL https://aka.ms/apm-unix | ...pipe entirely, so copying it literally just runs a bareshwith no install script.Failure scenario: A user or agent follows the escape hatch verbatim, runs
APM_INSTALL_DIR=$HOME/.local/bin sh, gets an empty/hanging shell (or an immediate no-op non-interactively) instead of an apm install, with no error to indicate anything went wrong.Identical issue at
plugins/kyberforge/docs/research/docs/microsoft-apm/installation.md:20.@@ -0,0 +27,4 @@- `apm experimental enable registries` must run before any `registry.*` config takes effect. Declaring a `registries:` block or running `apm config set registry.*` without it silently does nothing — no error, no warning.- Plain `apm audit` and `apm audit --ci` check different things: plain `apm audit` scans deployed files for hidden Unicode only; `--ci` additionally runs lockfile-consistency checks, install-replay drift detection, and org policy checks. A clean plain `apm audit` is not a CI-equivalent pass.## Step 1 — DispatchNeither apm-workflow's dispatch table nor apm-orchestrate's operation list ever routes to plain
apm install [PACKAGE_REF]— the core command that actually resolves/downloads a dependency declared in apm.yml — and apm-install (the skill whose name most resembles it) is explicitly scoped only to installing the apm binary/runtime, not runningapm install.Failure scenario: A caller adds a dependency under
dependencies.apmin apm.yml per configure.md's guidance, then has no documented next step to actually fetch it (apm install) — an agent following only these two skills scaffolds a manifest that's never installed.@@ -0,0 +33,4 @@repository: https://github.com/org/my-pkgkeywords: [ai, review, python]type: skill # instructions | skill | hybrid | prompts — constrains .apm/ contentsThis apm.yml schema block omits the "legacy singular
target:CSV form is still accepted" caveat that the sibling research doc (docs/research/docs/microsoft-apm/configuration.md:32) documents for the sametargets:field.Failure scenario: An agent following only this file — the one its own SKILL.md dispatch table points to — encounters an existing apm.yml using
target: copilot,claudeand flags or "corrects" it as invalid, when apm actually still accepts that legacy form.Full re-review of the fix commit (
e16c3dc) against PR review conventions. 6 findings — mostly places where this same fix commit's own edits now contradict its own claims or leave dangling references. See inline comments.@@ -0,0 +44,4 @@- Actually translating the existing plugins into `apm.yml` + `.apm/` and running the realconversion is deferred to issue #90(https://git.dev.rkdr.net/Defame1297/holocron/issues/90).- `CONTEXT.md`'s "Plugin"/"Skill"/"Plugin marketplace" glossary entries remain accurate asADR-0015 states "CONTEXT.md's Plugin/Skill/Plugin-marketplace glossary entries remain accurate as written until issue #90 actually executes — this ADR does not update them," but this same PR's fix commit (
e16c3dc) edited CONTEXT.md's Plugin and Plugin marketplace entries to add an ADR-0015 forward-pointer sentence.Failure scenario: A reader trusts the ADR's own claim about its change boundary and skips re-checking CONTEXT.md, or a future auditor treats the ADR as the authoritative record of what this PR touched and gets it wrong — the document contradicts the diff it ships alongside.
@@ -0,0 +59,4 @@## OutputReturns structured JSON with operation status, result (output — or a list of per-package results when fanned out — plus resolved package-root/registry context), and optional error details with recovery suggestions.The Output contract here is collapsed to one prose sentence and omits the explicit error-code enum (
not_confirmed | apm_unavailable | manifest_invalid | dependency_unresolved | publish_failed) that the siblingapm-orchestrate.mdspecifies as JSON schema, so the two paired provider files disagree on the actual output contract.Failure scenario: A Copilot CLI caller consuming this file has no enumerated error codes to branch on programmatically, while a Claude Code caller relies on the exact enum from
apm-orchestrate.md— the two provider paths silently diverge on whaterror.codecan contain.@@ -0,0 +49,4 @@## Process1. Validate the request structure and check if `operation` is known2. Check the request against the Hard rules above (publish confirmation, secret indirection, marketplace-add direction, `type:` ordering, audit-vs-audit-ci, registries precondition) — refuse outright on violation, independent of `confirm`Process step 2 still tells the orchestrator to check the Hard rules for "secret indirection," but that Hard Rule bullet was deleted from the Hard rules section in this same PR's fix commit (
e16c3dc), leaving a dangling reference to a rule that no longer exists.Failure scenario: An executing agent reads Process step 2, tries to locate and enforce the named "secret indirection" Hard Rule, and finds nothing in the Hard rules list — it either hallucinates a check to satisfy the reference or silently skips it while the text implies the gate still exists.
Identical issue at
plugins/kyberforge/agents/apm-orchestrate.agent.md:52.@@ -0,0 +31,4 @@Escape hatches — combine as needed:- Pin a version: append `@vX.Y.Z` to the piped script's arguments, e.g. `curl -sSL https://aka.ms/apm-unix | sh -s -- @v1.2.3`.- Custom install directory: set `APM_INSTALL_DIR` before running, e.g. `APM_INSTALL_DIR=$HOME/.local/bin sh`.The "custom install directory" example
APM_INSTALL_DIR=$HOME/.local/bin shomits thecurl -sSL https://aka.ms/apm-unix | ...pipe entirely, so copying it literally just runs a bareshwith no install script.Failure scenario: A user or agent follows the escape hatch verbatim, runs
APM_INSTALL_DIR=$HOME/.local/bin sh, gets an empty/hanging shell (or an immediate no-op non-interactively) instead of an apm install, with no error to indicate anything went wrong.Identical issue at
plugins/kyberforge/docs/research/docs/microsoft-apm/installation.md:20.@@ -0,0 +27,4 @@- `apm experimental enable registries` must run before any `registry.*` config takes effect. Declaring a `registries:` block or running `apm config set registry.*` without it silently does nothing — no error, no warning.- Plain `apm audit` and `apm audit --ci` check different things: plain `apm audit` scans deployed files for hidden Unicode only; `--ci` additionally runs lockfile-consistency checks, install-replay drift detection, and org policy checks. A clean plain `apm audit` is not a CI-equivalent pass.## Step 1 — DispatchNeither apm-workflow's dispatch table nor apm-orchestrate's operation list ever routes to plain
apm install [PACKAGE_REF]— the core command that actually resolves/downloads a dependency declared in apm.yml — and apm-install (the skill whose name most resembles it) is explicitly scoped only to installing the apm binary/runtime, not runningapm install.Failure scenario: A caller adds a dependency under
dependencies.apmin apm.yml per configure.md's guidance, then has no documented next step to actually fetch it (apm install) — an agent following only these two skills scaffolds a manifest that's never installed.@@ -0,0 +33,4 @@repository: https://github.com/org/my-pkgkeywords: [ai, review, python]type: skill # instructions | skill | hybrid | prompts — constrains .apm/ contentsThis apm.yml schema block omits the "legacy singular
target:CSV form is still accepted" caveat that the sibling research doc (docs/research/docs/microsoft-apm/configuration.md:32) documents for the sametargets:field.Failure scenario: An agent following only this file — the one its own SKILL.md dispatch table points to — encounters an existing apm.yml using
target: copilot,claudeand flags or "corrects" it as invalid, when apm actually still accepts that legacy form.Addressed all 6 findings from the latest review (review #24, re-review of
e16c3dc) in394052f:apm-orchestrate.md/.agent.mdProcess step 2) — removed; it pointed at a Hard Rule bullet the same fix commit had already deleted.APM_INSTALL_DIRescape-hatch example — fixed in bothapm-install/SKILL.mdand theinstallation.mdresearch doc. Verified the correct form against the upstream Microsoft APM docs via Context7: the env var goes after the pipe (curl -sSL https://aka.ms/apm-unix | APM_INSTALL_DIR=$HOME/.local/bin sh), not as a standalone prefix with no script piped in.apm install [PACKAGE_REF]— added a 5th "install" dispatch action toapm-workflow(newreferences/install.md, dispatch table row, README sync, provenance entry insources.md) and a matchinginstalloperation group onapm-orchestrateto route to it..md/.agent.mdOutput-contract "disagreement" — checked this againstgit-orchestrateandgitea-orchestrate's existing pairs and found it matches established repo convention exactly (full JSON schema + error enum in.md, prose summary without the enum in.agent.md). Treating as a false positive, left unchanged.target:CSV caveat inapm-workflow/references/configure.md— added, matching the sibling research doc.Also fixed a doc-drift item the closing skill-audit surfaced (not part of the original 6):
apm-workflow/README.md's Usage list and Files table didn't mention the newinstallaction — synced.All closing audits (
agent-audit,skill-audit, Vale) came back clean, full local test suite (12/12 suites) and manifest check pass, no version-bump collisions (kyberforge1.3.0 → 1.3.1, applied once).Summary of
92e7ff2— a grill-me session resolved howplugin-author/marketplace-authorrelate to the newly-landedapm-install/apm-workflow/apm-orchestrateartifacts (issue #88).Decision:
plugin-author/marketplace-authorare fully superseded (not adapted) —apm compile/apm packwill generate.claude-plugin/marketplace.jsonand per-providerplugin.jsondirectly fromapm.yml+.apm/, so both skills get deleted as part of issue #90's execution, once compiled-output parity is verified. This narrows issue #89's scope toskill-author/agent-authoronly (forgestays untouched throughout).Before that deletion happens, a content audit sorted what
plugin-author/marketplace-authorcurrently document into three buckets, now recorded in ADR-0015's new "Content migration out ofplugin-author/marketplace-author" section:agents/-directory stray-.mdvalidator gotcha,claude plugin validateas a required terminal check, a removal/re-pack confirm-before-proceeding note) — already migrated intoapm-workflow's reference docs (marketplace.md,compile.md,audit.md) in this commit, verified viakyberforge:skill-audit(PASS).apm.yml's single-manifest model, deliberately dropped.apm.yml'smarketplace.packages[].version.Tracker updated to match: #89 retitled/narrowed to
skill-author/agent-author, #90 commented with the added deletion scope and a pointer to what still needs preserving.Files changed:
docs/adr/0015-apm-replaces-plugin-marketplace-authoring.md,plugins/kyberforge/skills/apm-workflow/{README.md,references/audit.md,references/compile.md,references/marketplace.md,references/sources.md}.Review response
Ran a full
/code-reviewpass on this PR (8 finder angles). 7 of 8 confirmed findings were fixed ind6fd9b6:apm marketplace checkdispatch entry — removed from theauditrow, kept solely undermarketplace(its real usage-sequence home per references/marketplace.md).Not fixed (by design): the near-verbatim duplication between apm-workflow/references/.md and docs/research/docs/microsoft-apm/.md is intentional — the skill docs independently restate the CLI reference rather than pointing at the research mirror.
All tests pass (
tests/run-tests.sh,scripts/check-manifests.sh).Resolved the second review round in
591b9cc.Fixed:
plugin-author/SKILL.md(ADR-0016 doesn't exist → ADR-0006).plugin-author/marketplace-authorwasn't reflected anywhere it's actually discovered — routingdescription:frontmatter, forge's routing table/Gotchas, both kyberforge README skill tables, CONTEXT.md's Skill composition entry. Now propagated consistently. Routing behavior itself is unchanged — issue #90 (repo-wide apm conversion) hasn't landed, so these skills still need to work normally for now.apm-orchestrate'sconfigureoperation only covered scaffolding a new package, with no operation for editing an existingapm.yml(e.g. removing amarketplace.packages[]entry). I checked the actual apm CLI reference — there's noapm marketplace package removeverb, removal is just a manifest edit — so I put the design question to the user rather than guessing: addededit-config, intentionally ungated (no Hard Rule/confirm gate), since a local file edit isn't the same class of irreversible action asapm publishand normal PR review already gates the commit.references/configure.md's ~55-lineapm.ymlschema dump — copied near-verbatim from the research doc — down to a compact key list with a pointer, matchingreferences/install.md's existing terse cross-referencing style.Deliberately left as-is: the simplification pass flagged
apm-orchestrate's parallel/sequential fan-out logic as one-off prose not shared withgit-orchestrate/gitea-orchestrate. Left it inline — neither of those orchestrators needs that logic today, so extracting a shared abstraction now would be premature for a single caller.Full test suite (
tests/run-tests.sh) andscripts/check-manifests.shboth pass on the new commit.