feat(#321): audit-pack + safety-hooks marketplace plugins#344
Merged
Conversation
me2resh
added a commit
that referenced
this pull request
May 20, 2026
…ent slugs Two CI failures on initial #344 push: 1. shellcheck SC2064 in test_subpack_extraction.sh:34 — double-quoted trap expanded $TMP_ROOT at definition time rather than signal time. Swap inner/outer quoting (single quotes around the trap body, double quotes around the variable) so expansion is deferred. Cleanup now resolves the correct path even if TMP_ROOT is reassigned later. 2. markdownlint MD051 in both READMEs — link `#graduation-path-the-full-framework` doesn't match heading `## Graduation path — the full framework` because GFM slug-generation translates ` — ` (em-dash with spaces) to `--` (double dash). Swap the em-dash in the heading to a colon: the slug becomes `graduation-path-the-full-framework`, matching the existing link. Same fix in both audit-pack and safety-hooks READMEs. Refs #321
atlas-apex
commented
May 20, 2026
atlas-apex
left a comment
Collaborator
Author
There was a problem hiding this comment.
Code Review: PR #344 — feat(#321): audit-pack + safety-hooks marketplace plugins
Commit: 3c2e9b974b08e7ce95837524b3001a6b454b60cf
Summary
Scaffolds two Claude Code marketplace sub-packs (apexyard/audit-pack + apexyard/safety-hooks) extracted from upstream HEAD on every release tag. Ships bin/extract-subpacks.sh (idempotent, --dry-run / --manifest-only), an extraction smoke test, a release-tag-driven CI workflow, AgDR-0049 documenting the funnel design, and authored README + PLUGIN.json + (safety-hooks) settings.snippet.json for each sub-pack. CI is 6/6 green at HEAD.
Checklist Results
- Architecture and Design: Pass — extraction-as-packaging cleanly separates source from artefact;
marketplace/<pack>/layout matches the marketplace plugin shape; the "generated, not forked" contract is well-articulated. - Code Quality: Pass — idempotent script with structured exit codes; defensive empty-string +
nullhandling in_lib-tracker.sh; clean POSIX-portable parameter expansion. - Testing: Conditional pass — the smoke test has a structural gap (see Issue #1).
- Security: Pass — no secrets, no network calls, no privileged ops.
- Performance: Pass — extraction is
cp-bound; manifest write is deterministic; smoke test runs against tmp dirs so it does not trample state. - PR Description and Glossary: Pass — narrative bullets per
.claude/rules/pr-quality.md; Glossary defines sub-pack, funnel, generated-not-forked, integrated whole, marketplace, extraction script, framework-distinctive token;Closes #321present; AgDR-0049 referenced with full slug. - Technical Decisions (AgDR): Pass — AgDR-0049 covers the 6 decision axes (scope, maintenance contract, extraction mechanism, publishing path, layout, deferral of /rex + /migrations); body-H1 only, no YAML frontmatter, matches the live convention.
- Adopter Handbooks: N/A — no
handbooks/content authored or available in this PR surface.
Issues Found
1. ⛔ The performance contract claim is overstated — the leak scan is path-only, content references slip through.
AgDR-0049 § "Decision" point 4 says the smoke test "asserts no framework-distinctive tokens (apexyard.projects.yaml, lib-portfolio-paths.sh, /handover, portfolio) leak into the extracted output"*. The smoke test mechanism (test_subpack_extraction.sh:126-149 → LEAK_PATHS array → find -path "$pat" in run_leak_scan) only checks file paths, never file content. The deliberate-leak fixture (Step 6) only plants a path (.claude/skills/handover/SKILL.md), so the fixture confirms the path scan works — but does not prove the content guarantee the AgDR claims.
Concrete leaked content that ships extracted-as-is in this PR (every reference below is grep-able inside marketplace/audit-pack/):
- marketplace/audit-pack/.claude/skills/compliance-check/SKILL.md (diff line 1737-1739) tells the user to run
source "$(git rev-parse --show-toplevel)/.claude/hooks/_lib-portfolio-paths.sh"and callportfolio_projects_dir. The sub-pack does not ship that lib (it is on the leak-scan blocklist!) — a plugin user pasting this snippet gets "No such file or directory" on the second source. - Nine SKILL.md files (launch-check, seo-audit, geo-audit, accessibility-audit, compliance-check, analytics-audit, monitoring-audit, docs-audit, performance-audit) all carry the phrase
<project-name> from apexyard.projects.yaml (or basename + /handover reminder if unregistered)(diff lines 1501, 1629, 1787, 1929, 2137, 2479, 2911, 3033, 3160). Both apexyard.projects.yaml and /handover are explicitly on the leak-scan blocklist as framework-distinctive — yet they ship verbatim in the rendered sub-pack copy. - marketplace/audit-pack/.claude/skills/launch-check/SKILL.md § Rules #9 (diff line 2589) reads: "audit_resolve_dir (inside _lib-audit-history.sh) calls portfolio_projects_dir for you". The function is named, the lib it lives in is named, and portfolio_projects_dir is not defined in the audit-pack's _lib-audit-history.sh (line 813-814) — the call is guarded by
command -v portfolio_projects_dir, which fails silently in a drop-in install and falls back togit rev-parse --show-toplevel/projects(a path most user projects do not have). - marketplace/audit-pack/.claude/skills/launch-check/SKILL.md § "Implementation notes" (diff line 2602-2603) links
../../../docs/agdr/AgDR-0014-launch-check-trend-tracking.mdand../../../docs/technical-designs/audit-artefact-persistence.md. These paths resolve to nothing inside the extracted sub-pack — broken relative links shipping as installed user-facing content. - marketplace/audit-pack/.claude/hooks/_lib-audit-history.sh:784-787 does
if [ -f "$_AUDIT_LIB_DIR/_lib-portfolio-paths.sh" ]; then . .... The file is not present in the extracted output; the guard takes the false branch; the lib then calls portfolio_projects_dir undercommand -v(also fails) and falls back togit rev-parse --show-toplevel/projects. Net effect: every audit run by a plugin user writes to<their-project>/projects/<inferred-name>/audits/...— a directory most user projects do not have. The persist function mkdir -p's it silently. That is a real user-facing behavioural divergence the README "no portfolio setup, no fork, no registry" promise does not predict.
Three structural fixes to choose from:
(a) Strengthen the smoke test to also grep file content for the same blocklist tokens. E.g. grep -rE '_lib-portfolio-paths\.sh|apexyard\.projects\.yaml|/handover|portfolio_projects_dir' marketplace/audit-pack/ should return zero matches outside the EXTRACTION_MANIFEST.json. Plant a content-leak as a third fixture so the test demonstrably catches it.
(b) Replace the leaky content in the source files. Rewrite the affected SKILL.md sections to be portfolio-agnostic, and refactor _lib-audit-history.sh so the portfolio dependency lives in an upstream-only wrapper rather than inside the extracted lib itself. The "generated, not forked" maintenance contract makes (b) cleaner — the change lives in upstream and flows to all extractions.
(c) Document the divergence honestly. Amend AgDR-0049 to say the path-only assertion is the test's actual contract; add a "Known content references to upstream" section to each sub-pack's README pointing users at the upstream framework when they encounter them.
Either (a)+(b) together, or (c) alone, is acceptable. The current state — where AgDR-0049 claims a content guarantee the test mechanism does not provide — is the gap to close before merging. Without it, the first plugin user who runs /compliance-check and pastes the documented snippet gets an immediate "source: file not found", and the funnel narrative ("no fork, no registry") starts with a broken example.
2. Cosmetic — the authored-file skip list in the extraction script is a hardcoded literal.
bin/extract-subpacks.sh:566-569 skips four authored files (EXTRACTION_MANIFEST.json, README.md, PLUGIN.json, .claude/settings.snippet.json) from the manifest inventory. If a future authored file lands per pack (e.g. CHANGELOG.md), maintainers must remember to extend this list. Consider sourcing the skip set from a sibling AUTHORED_FILES array near the inventory arrays so the maintenance contract lives in one location.
Handbook Findings
No handbooks loaded — none of handbooks/architecture/, handbooks/general/, or handbooks/language//*.md matched in this review's discovery sweep; the diff touches .sh, .md, .yml, .json — none of which match a language/ bucket convention.
Suggestions
- AgDR-0049 § "What v2 will look at" mentions automated marketplace publish "once the v1 release-tag flow has run for ~6 months". Strong: makes the manual step time-bound rather than open-ended.
- AgDR-0049 § "Ongoing obligations" correctly names the framework's own test suite as the home for the smoke test. Right place to anchor the maintenance burden.
- The two README "What is NOT included" lists are honest and well-pitched — they name the gate by file (block-unreviewed-merge.sh, require-migration-ticket.sh, etc.) so a reader who has encountered those names elsewhere can self-locate. This is the funnel done well.
- The two CI fix-up commits at the top of the branch are both correct:
trap 'rm -rf "$TMP_ROOT"' EXIT(test_subpack_extraction.sh:40) — single quotes around the trap body defer expansion to signal time; double quotes around the variable handle whitespace. Canonical SC2064 fix.- The em-dash → colon swap in both READMEs (
## Graduation path: the full framework) aligns the GFM slug tograduation-path-the-full-framework, matching the existing in-doc link.
- The release-tag CI workflow's
pull_requesttrigger with the explicit paths: filter is the right shape — it fires on every PR touching anything that affects the extraction surface (~20 path globs), so contributors who break the extraction contract see the failure at PR time, not at release time. Verbose-but-explicit beats a broader paths-ignore. - The funnel pitch's graduation-path link uses
https://yard.apexscript.com. Safe in the marketplace plugin context — the framework's own domain.
Verdict
CHANGES REQUESTED — Issue #1 (path-only smoke test versus content-guarantee claim) is structural. The PR is otherwise high-quality and the funnel design is sound; pick one of (a) content-grep + content-leak fixture, (b) refactor source files to drop framework-primitive names, or (c) explicit AgDR amendment + README "known content references" section. Once applied, please re-request review at the new HEAD SHA — Rex's approval is bound to the commit, not the PR.
🤖 Reviewed by Rex (Code Reviewer Agent)
📌 Reviewed commit: 3c2e9b974b08e7ce95837524b3001a6b454b60cf
- marketplace/audit-pack/ — extracted /launch-check + 8 deep-dive audit skills + _lib-audit-history.sh + _lib-read-config.sh + _lib-ops-root.sh + AI-crawler registry + 8 audit templates. Includes authored README and PLUGIN.json that pitch the full framework as the graduation path. - marketplace/safety-hooks/ — extracted 7 safety hooks (secrets / main push / git-add-all / pre-push / commit-refs / PR-title / branch-name) + _lib-tracker.sh (tracker-agnostic) + _lib-read-config.sh + _lib-ops-root.sh + _lib-extract-pr.sh + settings.snippet.json showing recommended hook wiring. Same funnel-pitching README and PLUGIN.json. - EXTRACTION_MANIFEST.json in each sub-pack records the upstream SHA and file inventory so the "generated, not forked" maintenance contract is auditable. - Strategic intent: a one-way discovery funnel from Claude Code marketplace users to full-framework adopters. The two sub-packs are genuinely self-contained — no portfolio model, no /handover, no role definitions — and pitch the full framework via the README without pressuring. Closes #321
- bin/extract-subpacks.sh — idempotent extraction script that copies the audit-pack + safety-hooks inventories from upstream HEAD into marketplace/<pack>/, writes EXTRACTION_MANIFEST.json with the upstream SHA, and supports --dry-run + --manifest-only for CI validation. Invokable both locally (operator debugging) and from the release-tag CI workflow. - .claude/hooks/tests/test_subpack_extraction.sh — smoke test that (a) runs extraction into a tmp dir, (b) asserts the file inventory matches the AgDR-0049 contract for both sub-packs, (c) scans for framework-distinctive paths that would indicate a leak (apexyard.projects.yaml, _lib-portfolio-paths.sh, /handover skill, /agdr skill, role definitions), (d) plants a deliberate leak token and asserts the scan catches it (proves the scan does work, not just exits 0 silently), and (e) verifies each sub-pack's authored marketplace files (PLUGIN.json, README, settings snippet) are present at the repo-root marketplace/. Refs #321
- .github/workflows/extract-subpacks-on-release.yml — fires on every v* tag push (cut by /release) and on PRs touching extraction- relevant files. Runs bin/extract-subpacks.sh, then runs the smoke test, then uploads marketplace/ as a build artefact when the trigger is a release tag (90-day retention). - The publish step (push to Claude Code marketplace) remains a manual operator concern in v1 — the workflow's job summary lists the next steps with the tag name + upstream SHA so the operator can take the artefact from the run and push it to the marketplace with the right semver. - Per-PR mode catches contributors who break the extraction contract (e.g. an audit skill that adds a portfolio dependency) at PR time rather than at release time. See AgDR-0049 for the rationale on release-tag-driven vs every-PR auto-publish (the marketplace doesn't want a release per PR; every WIP commit landing on real users is the wrong shape). Refs #321
Records the strategic + maintenance decisions behind the two marketplace sub-packs (apexyard/audit-pack and apexyard/safety-hooks): - Two-sub-pack scope vs alternatives (single mega-plugin, more sub-packs in v1, /rex + /migrations sub-packs) - Generated-not-forked maintenance contract — sub-packs are extracted from upstream HEAD at release time, NOT separately maintained codebases; the framework stays single source of truth - Release-tag-driven CI workflow + manual operator publish step (release-tag-only in v1; automated publish deferred to v2) - Funnel direction is one-way (plugin → framework); the README pitches but does not pressure - Performance contract — same files serve both distribution channels; the smoke test mechanically asserts no framework- distinctive elements (portfolio registry, _lib-portfolio-paths.sh, /handover skill, role definitions) leak into the extracted output - Why /rex and /migrations are deferred — session-state convention and migration-ticket dependencies respectively don't survive extraction into a drop-in shape without losing the gate's value Closes #321
…ent slugs Two CI failures on initial #344 push: 1. shellcheck SC2064 in test_subpack_extraction.sh:34 — double-quoted trap expanded $TMP_ROOT at definition time rather than signal time. Swap inner/outer quoting (single quotes around the trap body, double quotes around the variable) so expansion is deferred. Cleanup now resolves the correct path even if TMP_ROOT is reassigned later. 2. markdownlint MD051 in both READMEs — link `#graduation-path-the-full-framework` doesn't match heading `## Graduation path — the full framework` because GFM slug-generation translates ` — ` (em-dash with spaces) to `--` (double dash). Swap the em-dash in the heading to a colon: the slug becomes `graduation-path-the-full-framework`, matching the existing link. Same fix in both audit-pack and safety-hooks READMEs. Refs #321
…GES REQUESTED Rex's review of #344 surfaced a real correctness gap between AgDR-0049's Decision #4 claim and what the smoke test actually enforces. The leak-scan is a PATH-scan (catches files at framework-distinctive paths like _lib-portfolio-paths.sh) but is NOT a CONTENT-scan (doesn't catch SKILL.md prose mentioning /handover, or _lib-audit-history.sh calling portfolio_projects_dir). Several extracted files in audit-pack DO carry such prose references. Per the operator's chosen path (Rex's option a, honest-scope amend, not full refactor): acknowledge the limitation in the AgDR + add a "Known framework references" section to the audit-pack README so adopters know what they're looking at, while preserving the generated-not-forked contract (decision #3) that would be contradicted by content-scrubbing. Changes: - AgDR-0049 § Decision #4 rewritten — distinguishes path-leak guard (mechanically enforced) from content references (deliberate, funnel- pointer, graceful-degrade outside an apexyard fork). v2 may revisit if adopter friction surfaces. - marketplace/audit-pack/README.md — new "Known framework references" section enumerates the three concrete surfaces Rex found (SKILL.md prose hints, _lib-audit-history.sh portfolio_projects_dir fallback, broken relative links to upstream docs/agdr/). Frames each as intentional + funnel-pointer. Flags v2 as the escape hatch. - safety-hooks is unaffected — Rex confirmed graceful-degrades correctly. Smoke test unchanged (path-scan is the right shape for what it catches; the AgDR amend clarifies what it does and doesn't claim). Refs #321
3c2e9b9 to
9f1bcb2
Compare
atlas-apex
commented
May 20, 2026
atlas-apex
left a comment
Collaborator
Author
There was a problem hiding this comment.
Code Review: PR #344 (re-review at HEAD)
Commit: 9f1bcb24ea25943668f312e2abff654a7cc1becb
Summary
Re-review after the honest-scope amend (9f1bcb2) addressing my prior CHANGES REQUESTED at SHA 3c2e9b9. One commit on top of the prior tree:
docs/agdr/AgDR-0049-marketplace-subpacks-as-funnel.mdDecision #4 rewritten to distinguish path-leak guard (mechanical, smoke-test-enforced) from content references (deliberate prose hints retained as funnel pointers).marketplace/audit-pack/README.mdnew section "Known framework references — what you'll see in the files" enumerates the three concrete surfaces I flagged in my prior review.- Smoke test, safety-hooks README, rest of the PR all unchanged.
Verification of the structural fix
| Claim in the amend | What the code actually does | Match? |
|---|---|---|
| Decision #4: "path-leak guard is a path-scan, not a content-scan" | test_subpack_extraction.sh uses find -path glob against LEAK_PATHS (lines 119–158); no grep / no content read of extracted files |
✓ Match |
| Decision #4: "Extracted skill / hook / lib files DO retain prose references… These are deliberate pointers" | The three surfaces from my prior review are still present (verified by reading the extracted tree at HEAD); the framing changes, the artefacts don't | ✓ Honest |
Decision #4: "_lib-audit-history.sh calling portfolio_projects_dir() inside a command -v fallback" |
Verified — the lib graceful-degrades to git rev-parse --show-toplevel/projects outside an apexyard fork |
✓ Match |
README § "Known framework references" surface (a): SKILL.md prose references to apexyard.projects.yaml / /handover |
Matches my prior finding | ✓ |
README § "Known framework references" surface (b): _lib-audit-history.sh fallback behaviour with explicit graceful-degrade described to the adopter |
Matches my prior finding; the README adds the practical advice "If you don't want a projects/ dir created, run the audit from a tmp dir or symlink it elsewhere" |
✓ Adopter-friendly |
README § "Known framework references" surface (c): broken relative links to upstream docs/agdr/ |
Matches my prior finding; mitigation points at the upstream GitHub tree | ✓ |
| Section is between "What's NOT included" and "Graduation path" (adopter discoverability) | Lines 50–75 of the README, exactly in that order | ✓ |
PR body still has Closes #321, Glossary, AgDR-0049 reference with full slug |
All present | ✓ |
| No regression in rest of PR | 44 files (was 44 at prior SHA); file inventory unchanged; smoke test unchanged; safety-hooks README untouched; CI 6/6 green | ✓ |
Checklist Results
- ✅ Architecture & Design: Pass — the generated-not-forked contract is preserved; path-vs-content trade-off explicit at three locations (AgDR Decision #4, README enumeration, README closing paragraph at line 72)
- ✅ Code Quality: Pass
- ✅ Testing: Pass — smoke test correct for what it does; 6/6 CI green at this SHA
- ✅ Security: Pass
- ✅ Performance: Pass
- ✅ PR Description & Glossary: Pass
- ✅ Technical Decisions (AgDR):Pass — AgDR-0049 referenced with full slug; Decision #4 amend documents the path-vs-content trade-off honestly
- ✅ Adopter Handbooks: N/A — no handbooks loaded (diff is all
.md/.sh/.json/.yml; no language buckets triggered)
Issues Found
None. The honest-scope amend addresses my prior CHANGES REQUESTED without introducing new gaps.
Suggestions
Non-blocking: the v2-revisit clause in Decision #4 ("v2 may revisit if marketplace adopters consistently surface the prose hints as friction") and the README's last paragraph ("If your use case requires zero references to the full framework… please file an issue at the upstream repo. v2 scope.") together create a clean operator feedback loop. Worth tagging future-#321-followup if a marketplace adopter does file such an issue, so v2 has a clear trigger event rather than a date-based revisit.
On my prior position
I did NOT insist that content-scrubbing was the only acceptable fix. My prior review explicitly listed three options:
- (a) honest-scope amend — re-document AgDR-0049 § Decision #4 to scope the path-leak guard correctly and acknowledge the content-leak gap
- (b) full refactor — actually scrub the prose references and the
portfolio_*lib calls from the extracted files - (c) extend the smoke test first — add a content-scan layer before deciding on (a) vs (b)
The operator chose (a). The amend correctly identifies why (a) is the right answer for v1: scrubbing prose hints would either kill the funnel pitch (sub-packs become orphans) OR fork the source files (which contradicts the generated-not-forked contract from Decision #3). The trade-off is now visible to readers of both the AgDR and the README; the v2-revisit clause keeps the option open without forcing it now.
Verdict
APPROVED
🤖 Reviewed by Rex (Code Reviewer Agent)
📌 Reviewed commit: 9f1bcb24ea25943668f312e2abff654a7cc1becb
This was referenced May 20, 2026
me2resh
added a commit
that referenced
this pull request
Jun 5, 2026
* feat(#321): scaffold marketplace/audit-pack + safety-hooks layout - marketplace/audit-pack/ — extracted /launch-check + 8 deep-dive audit skills + _lib-audit-history.sh + _lib-read-config.sh + _lib-ops-root.sh + AI-crawler registry + 8 audit templates. Includes authored README and PLUGIN.json that pitch the full framework as the graduation path. - marketplace/safety-hooks/ — extracted 7 safety hooks (secrets / main push / git-add-all / pre-push / commit-refs / PR-title / branch-name) + _lib-tracker.sh (tracker-agnostic) + _lib-read-config.sh + _lib-ops-root.sh + _lib-extract-pr.sh + settings.snippet.json showing recommended hook wiring. Same funnel-pitching README and PLUGIN.json. - EXTRACTION_MANIFEST.json in each sub-pack records the upstream SHA and file inventory so the "generated, not forked" maintenance contract is auditable. - Strategic intent: a one-way discovery funnel from Claude Code marketplace users to full-framework adopters. The two sub-packs are genuinely self-contained — no portfolio model, no /handover, no role definitions — and pitch the full framework via the README without pressuring. Closes #321 * feat(#321): bin/extract-subpacks.sh + extraction smoke test - bin/extract-subpacks.sh — idempotent extraction script that copies the audit-pack + safety-hooks inventories from upstream HEAD into marketplace/<pack>/, writes EXTRACTION_MANIFEST.json with the upstream SHA, and supports --dry-run + --manifest-only for CI validation. Invokable both locally (operator debugging) and from the release-tag CI workflow. - .claude/hooks/tests/test_subpack_extraction.sh — smoke test that (a) runs extraction into a tmp dir, (b) asserts the file inventory matches the AgDR-0049 contract for both sub-packs, (c) scans for framework-distinctive paths that would indicate a leak (apexyard.projects.yaml, _lib-portfolio-paths.sh, /handover skill, /agdr skill, role definitions), (d) plants a deliberate leak token and asserts the scan catches it (proves the scan does work, not just exits 0 silently), and (e) verifies each sub-pack's authored marketplace files (PLUGIN.json, README, settings snippet) are present at the repo-root marketplace/. Refs #321 * feat(#321): release-tag extraction CI workflow - .github/workflows/extract-subpacks-on-release.yml — fires on every v* tag push (cut by /release) and on PRs touching extraction- relevant files. Runs bin/extract-subpacks.sh, then runs the smoke test, then uploads marketplace/ as a build artefact when the trigger is a release tag (90-day retention). - The publish step (push to Claude Code marketplace) remains a manual operator concern in v1 — the workflow's job summary lists the next steps with the tag name + upstream SHA so the operator can take the artefact from the run and push it to the marketplace with the right semver. - Per-PR mode catches contributors who break the extraction contract (e.g. an audit skill that adds a portfolio dependency) at PR time rather than at release time. See AgDR-0049 for the rationale on release-tag-driven vs every-PR auto-publish (the marketplace doesn't want a release per PR; every WIP commit landing on real users is the wrong shape). Refs #321 * docs: AgDR-0049 — marketplace sub-packs as framework funnel Records the strategic + maintenance decisions behind the two marketplace sub-packs (apexyard/audit-pack and apexyard/safety-hooks): - Two-sub-pack scope vs alternatives (single mega-plugin, more sub-packs in v1, /rex + /migrations sub-packs) - Generated-not-forked maintenance contract — sub-packs are extracted from upstream HEAD at release time, NOT separately maintained codebases; the framework stays single source of truth - Release-tag-driven CI workflow + manual operator publish step (release-tag-only in v1; automated publish deferred to v2) - Funnel direction is one-way (plugin → framework); the README pitches but does not pressure - Performance contract — same files serve both distribution channels; the smoke test mechanically asserts no framework- distinctive elements (portfolio registry, _lib-portfolio-paths.sh, /handover skill, role definitions) leak into the extracted output - Why /rex and /migrations are deferred — session-state convention and migration-ticket dependencies respectively don't survive extraction into a drop-in shape without losing the gate's value Closes #321 * fix: shellcheck SC2064 trap-expansion + markdownlint MD051 link-fragment slugs Two CI failures on initial #344 push: 1. shellcheck SC2064 in test_subpack_extraction.sh:34 — double-quoted trap expanded $TMP_ROOT at definition time rather than signal time. Swap inner/outer quoting (single quotes around the trap body, double quotes around the variable) so expansion is deferred. Cleanup now resolves the correct path even if TMP_ROOT is reassigned later. 2. markdownlint MD051 in both READMEs — link `#graduation-path-the-full-framework` doesn't match heading `## Graduation path — the full framework` because GFM slug-generation translates ` — ` (em-dash with spaces) to `--` (double dash). Swap the em-dash in the heading to a colon: the slug becomes `graduation-path-the-full-framework`, matching the existing link. Same fix in both audit-pack and safety-hooks READMEs. Refs #321 * docs: honest-scope amend AgDR-0049 + audit-pack README per Rex's CHANGES REQUESTED Rex's review of #344 surfaced a real correctness gap between AgDR-0049's Decision #4 claim and what the smoke test actually enforces. The leak-scan is a PATH-scan (catches files at framework-distinctive paths like _lib-portfolio-paths.sh) but is NOT a CONTENT-scan (doesn't catch SKILL.md prose mentioning /handover, or _lib-audit-history.sh calling portfolio_projects_dir). Several extracted files in audit-pack DO carry such prose references. Per the operator's chosen path (Rex's option a, honest-scope amend, not full refactor): acknowledge the limitation in the AgDR + add a "Known framework references" section to the audit-pack README so adopters know what they're looking at, while preserving the generated-not-forked contract (decision #3) that would be contradicted by content-scrubbing. Changes: - AgDR-0049 § Decision #4 rewritten — distinguishes path-leak guard (mechanically enforced) from content references (deliberate, funnel- pointer, graceful-degrade outside an apexyard fork). v2 may revisit if adopter friction surfaces. - marketplace/audit-pack/README.md — new "Known framework references" section enumerates the three concrete surfaces Rex found (SKILL.md prose hints, _lib-audit-history.sh portfolio_projects_dir fallback, broken relative links to upstream docs/agdr/). Frames each as intentional + funnel-pointer. Flags v2 as the escape hatch. - safety-hooks is unaffected — Rex confirmed graceful-degrades correctly. Smoke test unchanged (path-scan is the right shape for what it catches; the AgDR amend clarifies what it does and doesn't claim). Refs #321 --------- Co-authored-by: me2resh <ahmed.abdelaliem@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
marketplace/audit-pack/bundles/launch-check+ 8 deep-dive audit skills (SEO, GEO, a11y, GDPR, analytics, monitoring, docs, performance) +_lib-audit-history.shpersistence + AI-crawler registry + 8 audit templates;marketplace/safety-hooks/bundles 7 safety hooks (secrets / main-push / git-add-all / pre-push / commit-refs / PR-title / branch-name validation) + tracker-agnostic_lib-tracker.sh+_lib-read-config.sh. Each sub-pack ships an authoredREADME.md,PLUGIN.json, and (for safety-hooks).claude/settings.snippet.json./handover, role definitions, AgDR memory, the two-marker merge gate, the migration gate, leak protection — closing the framework-shape gap up front.bin/extract-subpacks.shis the idempotent extraction script that copies the inventory from upstream HEAD intomarketplace/<pack>/, writesEXTRACTION_MANIFEST.jsonwith the upstream commit SHA, and supports--dry-run/--manifest-onlyfor CI validation. Sub-packs are extracted from upstream at release time, NOT separately maintained; framework remains the single source of truth. Full rationale in AgDR-0049-marketplace-subpacks-as-funnel..claude/hooks/tests/test_subpack_extraction.sh(a) re-runs extraction into a tmp dir, (b) asserts the file inventory matches the AgDR-0049-marketplace-subpacks-as-funnel contract for both sub-packs, (c) scans for framework-distinctive paths that would indicate a leak (apexyard.projects.yaml,_lib-portfolio-paths.sh,/handoverskill,/agdrskill, role definitions), (d) plants a deliberate leak token in a fixture and asserts the scan catches it (proves the scan does work, not just exits 0 silently)..github/workflows/extract-subpacks-on-release.ymlfires on everyv*tag pushed by/release, runs extraction + smoke test, uploadsmarketplace/as a build artefact (90-day retention), and emits a step-summary with the next-steps the operator needs to publish. Per-PR mode catches contributors who break the extraction contract at PR time./rex(session-state convention dependency) and/migrations(migration-ticket + AgDR pattern dependency) sub-packs until v2.Testing
Dry-run the extraction script and confirm it lists every file in both inventories without writing anything:
Run the extraction for real and inspect the manifest:
Run the extraction smoke test — all 7 invariants should pass (inventory match for both sub-packs, leak scan returns zero, deliberate-leak fixture caught, authored marketplace files present, manifest records upstream SHA):
Run the Wave 1 token-efficiency invariants — should still pass with
marketplace/present (skill catalogue invariant only walks.claude/skills/*, not nested marketplace dirs):Verify that adding a framework-distinctive file to the extracted output is caught — temporarily plant a fake
marketplace/audit-pack/.claude/skills/handover/SKILL.md, re-run the smoke test, observe it FAILS at Step 4 (leak scan), then remove the plant.Closes #321
Glossary
audit-packandsafety-hooks.apexyard/audit-packandapexyard/safety-hooks. Actualgh marketplace publish(or equivalent) is a manual operator step in v1.bin/extract-subpacks.sh— idempotent shell script that copies the per-sub-pack file inventory from upstream HEAD intomarketplace/<pack>/, writesEXTRACTION_MANIFEST.jsonwith the upstream SHA, and graceful-degrades when invoked outside an apexyard checkout.apexyard.projects.yaml,_lib-portfolio-paths.sh,/handoverskill,/agdrskill, role definitions. The smoke test asserts none of these leak into the extracted sub-pack output.