Round-2 sweep after re-auditing all 15 reported issues against the merged dev:
- #885 generalized: the original fix only renamed self-improving-agent's
status/review, but three more plugins shipped skills whose bare names
shadow Claude Code built-ins. Renamed with the same convention:
playwright-pro init/review -> pw-init/pw-review, agenthub init/status ->
hub-init/hub-status, autoresearch-agent status/resume -> ar-status/
ar-resume. All command references (/pw: /hub: /ar:), docs, audit records,
harness manifests, and mirror trees/indexes updated; the flat mirror
namespace no longer collides on 'status'. New scripts/check_skill_names.py
gate (wired into ci-quality-gate.yml as blocking) fails CI on any future
bare reserved name; rule added to SKILL-AUTHORING-STANDARD.md.
- #969 follow-through: five more scripts print box-drawing characters that
cannot exist in cp1252 (api_scorecard, api_linter,
breaking_change_detector, humanizer_scorer, content_scorer) — same
guarded UTF-8 reconfigure applied; all smoke-tested under a forced
legacy encoding.
Verified: check_skill_names (incl. negative test), check_plugin_json,
check_paths, derive_counters, check_dual_publish, smoke_scripts (634/634),
0 broken mirror symlinks.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qgc6RYXWJPr5oW9DHU7zR4
Review follow-up on PR #972: the Windows Notes section now sits between
Verification & Testing and Troubleshooting, matching the Table of Contents
order, and the four scripts patched for #969 are back to exactly two blank
lines after the reconfigure block.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qgc6RYXWJPr5oW9DHU7zR4
Review follow-up on PR #972: docs/plugins/index.md still described the old
"two approved extension fields in plugin.json" policy reversed by #954 —
rewritten to point at .claude-plugin/authoring-notes.json and the CI
hard-fail. 32 generated docs pages still linked the gitignored megaprompts/
tree via absolute GitHub URLs (404s); converted to the same annotated
plain-text form used in the source files.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qgc6RYXWJPr5oW9DHU7zR4
Public audit record audit/pr-stream-2026-08/: every open PR against dev
(#788-#967) deep-read and re-executed against the repo's own gates.
Verdicts: 8 MERGE, 8 MERGE-WITH-CHANGES, 6 CLOSE, 3 maintainer-draft
finish plans. Names the four blocking maintainer decisions (extension-key
policy #966-vs-#940, agent skills: preloading, DESIGN-only folders,
release framing), a six-phase global merge order, and per-PR executable
verification plans.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017Bzm6Pafyxja6g4jUDPcei
- #954: strip non-spec source/attribution keys from all 39 plugin.json
manifests so Claude Code's validator accepts them; metadata preserved in
new .claude-plugin/authoring-notes.json sidecars; check_plugin_json.py now
hard-fails manifests carrying those keys and sanity-checks the sidecar;
CLAUDE.md ClawHub schema section updated to the new rule.
- #949: move the c-level-agents plugin out of c-level-advisor/ to a
top-level directory so the two marketplace sources no longer overlap;
updated marketplace.json source, homepage, descriptions, all
cross-references, docs, harness manifest, mirror-tree symlinks/indexes,
and rebased the moved files' relative links; domain counters trued up
(18 -> 19 domains).
- #933: replace dead links to the gitignored maintainer-local megaprompts/
tree with annotated plain-text references (44 files: SKILL.md, READMEs,
agents, commands).
- #931: DynamoDB on-demand pricing updated to post-Nov-2024 rates
($0.625/M writes, $0.125/M strongly consistent reads).
- #969: skill_security_auditor.py and the three dossier scripts reconfigure
stdout/stderr to UTF-8 (errors=replace) so legacy Windows codepages no
longer crash at print time; PYTHONUTF8=1 documented.
- #968: Windows Notes section in INSTALLATION.md + README pointer for the
core.symlinks mirror-tree checkout caveat.
- #924/#885 residuals: hook commands quote "${CLAUDE_PLUGIN_ROOT}" paths in
all plugin hooks.json/settings.json (space-safe roots); removed the stale
pre-rename status/review mirror symlinks and index entries left over from
the memory-status/memory-review rename.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qgc6RYXWJPr5oW9DHU7zR4
GitHub Copilot CLI reads .claude-plugin/marketplace.json but enforces a
1024-character cap on each plugin's description. Four entries exceed it, so
Copilot rejects the entire catalog with:
Invalid marketplace.json: plugins.N.description: String must contain at most
1024 character(s)
Shortens engineering-advanced-skills (1132 -> 986), research-ops-skills
(1593 -> 957), markdown-html-skills (1240 -> 914) and memory-engineering
(1044 -> 964), trimming only redundant parenthetical detail. Skill inventories,
version history, hard rules, tool/reference counts and attributions are kept.
No other field is touched.
Verified with Copilot CLI: marketplace adds successfully and all 90 plugins are
listed.
Fourth review on PR #947 reported two functional bugs, explicitly noting it had
not run anything. Both reproduced, and the first is severe.
1. F1 -- the blocking gate -- could be bypassed by a typo.
_check_forgetting_rule() failed only when `rule` was literally
"none"/""/"never", and otherwise inferred PASS from what the rule was *not*.
So anything unrecognized fell through to the PASS branch with an empty
mechanism list. Reproduced:
{"rule": "asdf"} -> F1=PASS "Forgetting is designed: ."
{"rule": "ttl"} (no ttl_days)-> F1=PASS "Forgetting is designed: ."
A misspelling silently passed the one check this entire skill is built
around, and the nonsensical detail string was the only hint.
The check is now allowlist-based: PASS is unreachable unless a concrete
mechanism is actually found (ttl_days > 0, max_records/max_bytes > 0, or a
decay setting). Failure messages now distinguish an unrecognized rule from a
declared-but-unconfigured one, so a typo is never mistaken for a deliberate
decision not to forget. Booleans are rejected where a number is expected,
and ttl_days=0 counts as absent.
Verified across 10 cases: all six bypass variants now FAIL at exit 4, all
four legitimate mechanisms still PASS, and the empty-mechanism string can no
longer be emitted.
2. --print-sample-spec was unreachable on all three scripts that offer it.
The flag sat outside a mutually-exclusive group declared required=True, and
argparse enforces that during parse_args() -- before any of our code runs.
So the flag alone exited 2 with a usage error, which broke the first line of
the workflow SKILL.md documents verbatim:
python scripts/memory_cost_profiler.py --print-sample-spec > workload.json
The group is now required=False with explicit post-parse validation, so
no-args still errors helpfully and names all valid entry points. Verified the
full round-trip on all three: --print-sample-spec > f.json, then feed f.json
back in.
This slipped through because the PR's own checklist covered --help, --sample
and --output json, but never ran --print-sample-spec standalone.
Also removed the identity dict in render() flagged as a nit.
Verified: 4/4 scripts help/sample/json; error paths 3/4/4; all six blocking
gates; checklist 6/6 PASS; security auditor PASS (0 critical, 0 high, 0 info).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jt1sqt5kQmopyfXu2Hhjnv
Third review on PR #947 caught a counter this branch's sync missed.
README.md line 30 read "**Reference docs** — 746 templates, checklists, and
domain-specific knowledge files". The sync pass replaced the string "746
reference guides", which is the phrasing used in CLAUDE.md and
marketplace.json, so this differently-worded line was never matched.
Root cause worth recording: `derive_counters.py --check` does not cover this
line. Verified by setting it to a deliberately wrong 111 — the gate still
passes. So this class of drift is invisible to CI, which is why a reviewer
found it and the automated gate did not. Not fixing the checker here: it is
shared infra, and widening its coverage could surface pre-existing drift in
unrelated rows and turn CI red for reasons that have nothing to do with this
PR. Flagged on the PR for the maintainer instead.
Also swept README for any other stale counter adjacent to counter vocabulary
(skills/tools/references/agents/commands/plugins, old value vs new): no other
occurrences. Badges and both prose lines now read 364 / 667 / 750 / 104 / 120 / 90.
Left untouched: CLAUDE.md line 183, which records book-to-skill's own
"refs 741 -> 746" delta. That is historically correct for that release note
and must not be re-pointed at the current total.
All six blocking gates pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jt1sqt5kQmopyfXu2Hhjnv
Second automated review on PR #947 found a real bug, verified by execution:
`pick()` looks up `tuple(sorted([winner, runner_up]))`, but two of the four
TIE_BREAKERS keys were authored in the other order, so they could never match:
('flat_rag', 'structured_rag') sorted -> matches
('structured_rag', 'agentic') sorted -> ('agentic', ...) DEAD
('long_context', 'flat_rag') sorted -> ('flat_rag', ...) DEAD
('long_context', 'structured_rag') sorted -> matches
Only 2 of 4 authored questions were reachable. The two dead ones are the
plausible near-ties (structured_rag vs agentic on a high-recall/high-mutability
workload; long_context vs flat_rag under build-budget pressure), so the tool's
most distinctive behaviour — an authored, specific tie-breaking question —
silently degraded to the generic fallback with no error.
Keys are now normalized through sorted() at import, with a collision check that
raises if two entries describe the same pair. Verified by injecting a duplicate
in the reverse order: the guard fires. This repo has no test suite, so the check
runs at import rather than living in a test.
Confirmed by execution, not inspection: all 4 keys reachable, and a constraint
set that ties structured_rag against agentic now returns the authored question
("Does your memory need to correct itself without a human in the loop?")
instead of the fallback.
Also from the review: the SKILL.md workflow block labelled steps 1-4 while the
prose referenced a step 5, so a reader skimming only the code block would not
know it existed. Added a `# 5 - No command.` line and compensated elsewhere to
stay within the checklist's 100-line limit (still 6/6 PASS).
Verified: 4/4 scripts --help/--sample/--output json; all six blocking gates.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jt1sqt5kQmopyfXu2Hhjnv
Automated review on PR #947 flagged find_duplicates() undercounting. Verified
and fixed, though the diagnosis was incomplete in a way that changes the fix.
`seen.add(j)` only ever recorded the second member of a matching pair, so for
a fully-connected cluster of k mutually-duplicate records len(seen) == k-1.
That number is not meaningless -- it is exactly the count of redundant copies
you could delete. The actual defect is that it was reported under the name
`records_with_a_duplicate`, and rendered as "N records have a near-duplicate",
which describes participants (k), not redundant copies (k-1). Two valid
metrics, one reported under the other's name.
So rather than just adding `seen.add(i)`, both are now computed and reported:
participants -- every record with >= 1 near-duplicate; drives duplicate_share
and the DUPLICATE_BLOATED threshold, matching the wording
redundant -- participants minus one survivor per connected cluster
Clusters are resolved with union-find, not by counting pair endpoints: a
3-record cluster emits pairs (i,j), (i,k), (j,k), so endpoint arithmetic gets
the redundant count wrong. Verified k=2/3/4 give participants=k, redundant=k-1,
and that a 3-cluster plus an unrelated record still gives 3/2.
Sample output moves from "1 record (17%)" to "2 records (33%), of which 1
redundant" -- the reviewer's point that this could tip a real store under the
15% threshold was correct.
Also from the same review:
- Dropped the stale `-> list[dict]` hint (the function returns a tuple); the
return shape is documented in the docstring instead.
- Capped the O(n^2) scan at MAX_DUPLICATE_SCAN=2000 eligible records and made
the tool print how many were skipped, per this repo's no-silent-caps rule --
a quiet cap reads as "no duplicates found".
Verified: 4/4 scripts --help/--sample/--output json; all six blocking gates;
skill checklist 6/6 PASS; real-directory run unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jt1sqt5kQmopyfXu2Hhjnv
dev moved 8 commits ahead (engineering/book-to-skill), and both branches
touched the same three headline-counter files, so marketplace.json, CLAUDE.md
and README.md all conflicted.
Resolved by taking dev's version of each file wholesale, then re-applying this
branch's additions on top — rather than hand-merging the counter arithmetic,
which is exactly the kind of edit that silently drifts:
- marketplace.json: re-inserted the memory-engineering entry next to
agent-harness (dev's 89 plugins -> 90)
- CLAUDE.md: re-applied the memory-engineering release note above the
fable-goal note
- README.md: engineering-POWERFUL row 85 -> 86 plus the plugin mention
Then re-derived every headline number from the merged tree instead of
assuming my original deltas still held — they did not, since book-to-skill
had already consumed them. True values are now skills 364, tools 667, refs
750, agents 104, commands 120, plugins 90. The release note's own delta line
was rewritten to sit on top of book-to-skill rather than claiming the stale
362->363 range.
Verified on the merged tree: all six blocking gates pass (check_plugin_json,
check_paths, check_dual_publish, smoke_scripts, smoke_json_output,
derive_counters --check), compileall clean, skill checklist still 6/6 PASS,
no conflict markers remain.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jt1sqt5kQmopyfXu2Hhjnv
CI gate G1 (scripts/check_paths.py) failed with 8 unresolvable references.
Both command files referenced `scripts/<tool>.py` and `assets/<file>` as if
they were relative to the command file, but commands/ sits at the plugin root
while the scripts live under skills/memory-engineering/. SKILL.md was correct
already — it sits inside the skill directory, so its bare `scripts/...` paths
resolve — which is why this only showed up in the two command files.
Rewritten to the plugin-root-relative form
(`skills/memory-engineering/scripts/...`), matching how agent-harness writes
its command paths.
check_paths.py --all now reports 0 findings across 586 files. Also re-ran the
other five blocking gates locally: check_plugin_json, check_dual_publish,
smoke_scripts, smoke_json_output, derive_counters --check — all pass, plus
compileall on the plugin.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jt1sqt5kQmopyfXu2Hhjnv
Two follow-ups after running the repo's own CI auditors locally:
- Removed `from __future__ import annotations` from all 4 scripts. The repo's
skill_validator.py counts it as an external import (script_tester.py
disagrees and reports the same files as stdlib-only, but the stricter of the
two is what CI surfaces). PEP 585 generics are native on the 3.11 CI runner,
and the single PEP 604 union annotation was the only thing needing the
import — it is now unannotated. Takes this skill from 7 validator errors to
3, matching productivity/weekly-review and better than productivity/fable-goal
(4). The 3 remaining are the legacy v2.0.0 schema checks (min-100-lines, and
Tier/Category/Features/Usage frontmatter+sections) that directly contradict
the Matt Pocock checklist CLAUDE.md declares binding for post-v2.6.0 skills —
every recently-merged skill carries the same ones.
- SKILL.md: steps 4 and 5 used a bold-number style left over from the
compression pass while steps 1-3 had become prose. Reworded to match; still
exactly 100 lines and 6/6 PASS.
Verified after the change: 4/4 scripts pass --help / --sample / --output json;
script_tester 5/5 PASS; security auditor PASS (0 critical, 0 high, 0 info).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jt1sqt5kQmopyfXu2Hhjnv
New engineering/memory-engineering/ plugin. The repo had no skill for
designing, pricing, or auditing an agent memory system: llm-wiki maintains
one vault, skillopt-sleep runs a consolidation loop, agent-harness bounds a
task loop. This bounds a store.
Four stdlib scripts, one per lens:
- memory_cost_profiler.py — construction vs query split, cost per correct
answer, amortization ratio, construction co-location warning
- memory_architecture_picker.py — scores the four paradigm families,
disqualifies on hard constraints, names the cost the choice makes you pay,
and refuses to pick when the top two tie (exit 2 + tie-breaking question)
- memory_density_auditor.py — classifies records FACT/SKILL/LOG/PROSE, finds
near-duplicates, flags staleness and time-relative wording, scores density;
runs on a real --dir or --jsonl
- forgetting_policy_linter.py — the gate. 8 checks; F1 (explicit forgetting
rule) and F4 (contradictions surfaced, never auto-merged) block at exit 4
Evidence discipline: the four-lens framing is synthesized from @N01ennn's
"How to be a Memory Engineer", but every quantitative claim is re-cited to
the primary source, and two of the article's paraphrases are corrected in the
references rather than propagated — the 47x energy figure is the spread across
ten evaluated systems (not an accuracy-matched pair), and the 97%
first-pass-error figure is Rakuten's named vendor testimonial (not a
controlled study). Per-claim confidence levels throughout.
Three classifier defects found and fixed during the build, each of which would
have produced garbage on a real repo: markdown headings inside fenced code
blocks were splitting records (258 phantom records -> 107 on a real directory);
short fragments matched trivially at 1.00 Jaccard (41 false-positive
duplicates); and signal-less prose was labeled LOG, firing LOG_HEAVY at 74% on
a documentation folder — now its own PROSE class.
Also ships 4 references (7 sources each), a seven-question forcing worksheet,
a combined example spec consumed by all three spec-taking scripts, an F1–F8
policy template, cs-memory-engineer, /cs:memory-engineering and
/cs:forgetting-audit.
Verified: all 4 scripts pass --help / --sample / --output json, stdlib-only,
correct exit codes on error paths; SKILL.md 6/6 PASS on the write-a-skill
checklist; check_plugin_json.py --all and derive_counters.py --check both pass.
Counters: skills 362->363, tools 644->648, refs 741->745, agents 102->103,
commands 116->118, plugins 88->89.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jt1sqt5kQmopyfXu2Hhjnv
Addresses the residual TOCTOU raised in the fourth review. The reviewer called
it non-blocking; it verified as slightly worse than described, and the fix is
small, so it is closed rather than deferred.
The claim checks out: `mkdir(parents=True, exist_ok=True)` does NOT raise on a
symlink-to-directory, because its exists-branch tests `is_dir()`, which follows
symlinks. Demonstrated directly — mkdir succeeded silently on a planted link and
a subsequent write landed in the attacker's directory.
What the review did not note is why the second layer failed to catch it: a file
inside a swapped directory is an ordinary file, not a symlink, so
`_write_private`'s `is_symlink()` check could never see a directory swap. The
artifact-level guard did not back up the directory-level one at all.
Three changes:
- `resolve_workdir()` attempts `mkdir` FIRST and only inspects a path that
already existed, via `os.lstat` — which does not follow the final component.
That removes the check-then-create ordering.
- `open_workdir()` pins the directory with `O_NOFOLLOW|O_DIRECTORY`, and both
artifacts are written through that descriptor. An fd names an inode, so a
rename or symlink swap of the path afterwards cannot redirect the write.
- `_write_private()` creates with `O_CREAT|O_EXCL|O_NOFOLLOW` at mode 0600 —
no check-then-act window at all. An artifact from a previous run into the same
--workdir is unlinked first; unlink removes the link, never its target.
Verified against a live race: pin the directory, rename it away, plant a symlink
to an attacker directory, then write — data lands in the pinned inode, attacker
directory stays empty. Also verified a pre-planted `full_text.txt -> victim`
symlink leaves the victim's content intact and is replaced by a 0600 file we own,
and that re-running into the same --workdir still succeeds.
Degrades to the previous path-based checks where `dir_fd`/`O_NOFOLLOW` are
unavailable (Windows).
Recorded as deviation 25. Full regression re-run: EPUB bomb, EPUB entity,
extensionless sniff bomb, DOCX bomb, emitter symlink, rights gate and the
estimator path check all still refuse; a clean EPUB still extracts. All gates
green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017zu9Gmm9S78c2t3kDLnpPX
Read the skill as a skill rather than as code, which the previous three review
rounds had not done. Four findings, all now fixed and verified.
1. The documented quick-start did not run. SKILL.md's copy-paste block referenced
$WORKDIR and $SKILLS_HOME without ever assigning them, so following it
literally produced a FileNotFoundError traceback at step 2. Both are now real
assignments, and all five steps were executed verbatim end to end as a check.
The plugin README's block had the same defect and is fixed the same way. A
quick-start that does not run is the worst kind of doc bug: it is the part a
reader trusts most.
2. A gate tool reported success for a path that was not there.
`token_budget_estimator.py --skill-dir <typo>` produced a complete,
plausible-looking budget audit — every row "missing", every cap satisfied,
exit 0 — which reads as a pass. It now refuses a missing directory, a
non-directory, and a directory with no SKILL.md (exit 2). `--full-text
<missing>` raised a bare traceback and now refuses cleanly. The other three
tools already validated their inputs; this one was the outlier.
3. Three upstream artifacts cleaned, one of them load-bearing. epub.py's
`except (KeyError, Exception)` is simply `except Exception` — it swallowed
everything including the size refusal `safe_read()` now raises, quietly
disarming deviation 17 at that call site. Narrowed so ExtractionError
propagates and only genuine parse failures fall through to the .opf glob.
utils.py emitted a dynamic {pages_label: pages} key beside a literal "pages",
colliding whenever the label was "pages"; the alias is now conditional. A
stray artifact word removed from a pdf.py comment.
4. `tool | head` no longer tracebacks. Observed once on the emitter (racy on
flush timing, 0/20 on retry) — all four CLIs now exit 141 quietly, the
standard SIGPIPE convention.
Token cost re-measured: SKILL.md 2,256 tokens resident (229 lines), references
10,216 on demand. Healthy against the ~5k practical ceiling for a resident body.
Full security regression re-run after the changes: EPUB bomb, EPUB entity,
extensionless sniff bomb, DOCX bomb, planted workdir symlink, emitter symlink
and the rights gate all still refuse; a clean EPUB still extracts.
Recorded as deviations 21-24; count synced across plugin.json, CLAUDE.md and
CHANGELOG. All gates green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017zu9Gmm9S78c2t3kDLnpPX
Addresses the third automated review on PR #941.
[High] The magic-byte sniffing path bypassed the zip-bomb budget it was built
to enforce. extract_single_file() reads a `mimetype` member with a bare
zf.read() when the extension is unrecognized — the earliest attacker-controlled
point in the pipeline, running before a format is chosen and before any check in
zip_safety.py. zip_safety.py's own docstring claims "every read goes through
safe_read()"; this one did not, which makes it a documentation defect as much as
a security one. Now routed through safe_read(). Its ExtractionError sits outside
the surrounding except tuple on purpose, so a bomb reports as a bomb rather than
as a generic unsupported format.
Verified: a 200 MB / 1029x fixture with no file extension is refused at ~15 MB
peak RSS instead of being decompressed.
[Medium] --author / --author-url never reached the printed marketplace entry.
_plugin_manifest() threaded them correctly into the emitted plugin.json, but
_marketplace_entry() took no author parameter and hardcoded one name — so the
snippet whose entire purpose is preventing hand-edit mistakes contradicted the
manifest sitting next to it for anyone but the default author. Threaded through.
Verified: --author "Jane Doe" now appears in both.
[Low] Narrow TOCTOU between _assert_no_symlinks() and copytree. copytree already
runs with symlinks=True, so a link planted in that window is copied as a link
rather than dereferenced — no content leak. Now fully closed: the emitted tree
is re-walked after the copy, and the package is deleted rather than shipped if
any link appeared.
Verified with a monkeypatched guard that plants a symlink immediately after the
check passes: refused, package removed, secret content absent.
[Nit] plugin.json asserted "license": "MIT" unconditionally, with the "MIT
covers the converter, not the compiled content" caveat living only in README
prose. Added source.license_scope stating it in the manifest, so a tool reading
only the manifest sees the distinction, plus a code comment at the assignment.
Recorded as deviations 19 and 20; count synced across plugin.json, CLAUDE.md
and CHANGELOG.
All gates green: compileall, check_paths --all, check_dual_publish,
smoke_scripts (0 failed), derive_counters --check, check_plugin_json --all
(0 FAIL). All four CLIs pass --help / --sample.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017zu9Gmm9S78c2t3kDLnpPX
Addresses the second automated review on PR #941. All three code findings
verified against the actual behaviour, not just patched.
[Medium] shutil.copytree dereferenced symlinks the validator never saw. The
validator checks SKILL.md, the three supporting files and chapters/*.md; the
copy then followed a link anywhere else in the tree (assets/, any subdirectory)
and baked the target's real content into a package that can go out as
--distribution shareable. _assert_no_symlinks() now walks the whole tree and
refuses, and runs BEFORE the validation branch so --skip-validation cannot
bypass it. copytree also passes symlinks=True so loosening that check later
cannot silently reintroduce dereferencing.
Verified: a symlink in assets/ pointing at a secret file is refused both with
and without --skip-validation, the secret never lands in a package, and a clean
tree still emits.
[Medium] The DOCX XXE/entity guard did not extend to EPUB's ebooklib path.
Upstream hardened DOCX only. EPUB is the same zip-of-XML shape and ebooklib —
one of the packages this skill recommends installing — parsed container.xml,
the OPF and content docs with no equivalent pre-check. The guard moved to a new
book_to_skill/zip_safety.py and now runs for both formats.
Verified: an EPUB whose OPF declares an entity is refused; a clean EPUB still
extracts and detects its chapter.
[Low] No size cap before decompressing zip members. Every archive read now goes
through safe_read(), which checks the declared uncompressed size and the
compression ratio against the central directory before decompressing, and
charges actual bytes against a per-archive budget so a lying directory cannot
get past it either.
Verified: a 200 MB / 1029x bomb is refused at ~14 MB peak RSS instead of being
materialized.
[Low] The PR body's "12 numbered items" was stale against README's list. Fixed
in the PR description; the in-repo count is synced to 18 across plugin.json,
CLAUDE.md and CHANGELOG.
Recorded as deviations 17 and 18. Counters: tools 662 -> 663 (zip_safety.py);
that module is allowlisted in smoke_exceptions.txt like its siblings.
All gates green: compileall, check_paths --all, check_dual_publish,
smoke_scripts (0 failed), derive_counters --check, check_plugin_json --all
(0 FAIL). Security auditor unchanged at 0 critical / 4 high.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017zu9Gmm9S78c2t3kDLnpPX
Addresses the automated review on PR #941.
Security (the one item flagged as wanted-before-merge): upstream defaults the
extraction workdir to a fixed `<tempdir>/book_skill_work`. On a shared host that
is CWE-377/CWE-59 — any local user can pre-create the directory in a
world-writable /tmp (the sticky bit prevents deletion, not creation) and plant a
symlink named full_text.txt or metadata.json pointing at a file the victim can
write, because Path.write_text follows symlinks. Two concurrent runs also
silently clobber each other.
- Default workdir is now a fresh `tempfile.mkdtemp(prefix="book_skill_work_")`:
unpredictable name, 0700 by construction, never shared with a concurrent run.
The path is printed and carried in metadata.json's `output_text`.
- Artifacts are written 0600, and each write refuses a symlink at the target.
- An explicit --workdir / BOOK_SKILL_WORKDIR is still honoured, but is
symlink-refused, created 0700, and chmod-tightened if it already exists.
- parsers/calibre.py no longer writes its ebook-convert scratch file to the
shared directory. That also fixes a real bug the review did not name: it read
a module-level OUTPUT_DIR constant, so the scratch file ignored --workdir
entirely and escaped the directory the caller asked for.
Verified: default workdir 0700 with 0600 artifacts and a per-invocation name;
two runs get distinct directories; a 777 --workdir is tightened to 700; a
symlinked workdir is refused; and a planted `full_text.txt -> victim` symlink is
refused with the victim file left untouched.
Also from the review:
- book_skill_validator.py and token_budget_estimator.py restated the same
BUDGETS dict. Both now import SKILL_FILE_BUDGETS / CHAPTER_TOKEN_CEILING from
book_to_skill/config.py so the two gating tools cannot drift.
- Corrected the smoke_exceptions.txt rationale: the list is "modules the G8
probe trips on", not "modules that aren't CLIs". config.py, exceptions.py,
sanitize.py, parsers/__init__.py, parsers/pdf.py and parsers/text.py are
equally not CLIs and pass only because they have no argv handling.
Recorded as deviations 15 and 16; count synced in plugin.json, CLAUDE.md and
CHANGELOG. Docs updated: the workdir path is now read from the tool's output
rather than hardcoded.
All gates green: compileall, check_paths --all, check_dual_publish,
smoke_scripts (0 failed), derive_counters --check, check_plugin_json --all
(0 FAIL). Security auditor unchanged at 0 critical / 4 high (documented).
End-to-end pipeline re-run clean: extract -> verdict -> validate -> emit.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017zu9Gmm9S78c2t3kDLnpPX
CI gate G1 (check_paths.py) failed: the agent's tool table referenced
`scripts/<tool>.py`, which resolves relative to the agent's own folder, not the
skill's. Now uses `../skills/book-to-skill/scripts/...` like cs-skill-author.
Two more issues found running the full gate set locally:
- Renamed `parsers/html.py` -> `parsers/html_text.py`. A module named `html.py`
shadows the stdlib `html` package whenever its own directory lands on
sys.path[0], and `import html.parser` then fails with "'html' is not a
package". Renaming removes the hazard rather than documenting it; two import
lines changed. Verified: HTML extraction still detects chapters, emits block
boundaries, and tab-joins table cells.
- Registered the eight vendored library modules in scripts/smoke_exceptions.txt.
They are imported as `book_to_skill.*`, never run as CLIs, so gate G8's
`--help` probe can only ever fail on them. The four real entry points are
smoke-tested normally and pass.
Recorded as deviations 13 and 14 in the plugin README; count synced in
plugin.json, CLAUDE.md and CHANGELOG.
All blocking gates green locally: compileall, check_plugin_json --all (89 OK),
check_paths --all (0 findings), check_dual_publish (0 drift), smoke_scripts
(0 failed), derive_counters --check.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017zu9Gmm9S78c2t3kDLnpPX
Derived from virgiliojr94/book-to-skill (MIT). Compiles a book, docs folder, or
spec collection (PDF, EPUB, DOCX, HTML, Markdown, RST, AsciiDoc, RTF, MOBI/AZW)
into an agent skill: a resident master SKILL.md (core frameworks + chapter index
+ topic index, capped at 4k tokens) plus on-demand chapter files, a glossary, a
patterns file, and a decision cheatsheet.
The extraction library (scripts/book_to_skill/, 12 modules incl. 7 per-format
parsers) is vendored close to verbatim and keeps upstream's format chains,
chapter detection across Latin/Roman/Chinese/Thai/Korean heading styles,
invisible-Unicode (Trojan Source) sanitization, and the DOCX entity guard.
12 numbered deviations recorded in the plugin README (authoritative list):
- No implicit installs: --install-missing defaults to `report`, printing the pip
command and using the stdlib fallback, where upstream prompts on a TTY and
installs into the caller's environment.
- Rights gate: emitting a shareable package refuses without --rights from
public-domain|open-license|internal-docs|author-permission. `fair-use` is
deliberately excluded — a defence, not a licence.
- Validator merged and extended: upstream's two validators become one four-family
gate, adding budget (token caps) and index (dead chapter links, unindexed
chapters, dangling topic refs) — the failure that silently breaks navigation
while the skill still looks complete.
- Folded YAML scalars now parse, so a wrapped description no longer under-reports
its length past the 1024-char cap.
- token_budget_estimator replaces discovery_tax: tiktoken path dropped for one
deterministic estimator, post-flight budget audit added, plus an explicit
worth-converting verdict that says "just read it" below ~3x the compiled size.
- Two PRIV-ESC criticals fixed: upstream install hints contained a literal
`sudo apt install`; they now name the package manager without escalating.
Repo-native addition with no upstream counterpart — Step 11 / /cs:book-to-plugin:
upstream stops at a bare ~/.claude/skills folder this library cannot route to.
skill_plugin_emitter.py wraps a compiled skill as a full plugin package (manifest
+ cs-<slug> agent + /cs:<slug> command + README) and prints the marketplace entry
without editing marketplace.json. Its --force path is guarded against symlinks,
paths outside the destination root, and non-package directories.
Ships 4 stdlib-only tools (all --help/--sample/--output json), 5 references citing
7-8 sources each, 3 asset templates, cs-book-to-skill agent, 2 commands.
Cross-linked into write-a-skill ("author first, compile second").
Regenerated the engineering harness manifest: picked up book-to-skill plus three
skills that had drifted out (minimalist, skillopt-sleep, strict-api), 81 -> 85.
Counters: skills 362 -> 363, tools 644 -> 662, refs 741 -> 746, agents 102 -> 103,
commands 116 -> 118, plugins 88 -> 89 (derive_counters.py --check passes).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017zu9Gmm9S78c2t3kDLnpPX
Clears every reference the new G7 lint flags, then makes it blocking so the
class cannot drift back. audit/engineering-agentic-2026-07 marked the
senior-ml-engineer half of this STILL-OPEN.
Deleted rather than updated:
- agent-designer/agent_evaluator.py's _define_cost_benchmarks() held
per-token prices for gpt-4, gpt-3.5-turbo and claude-3 at 2024 rates. The
result was assigned to self.cost_benchmarks and never read by anything, so
the method is gone. Cost analysis uses the cost_usd the caller supplies per
execution log, which is the only figure that can be accurate
Made model-agnostic, following the precedent already set by
senior-prompt-engineer/scripts/prompt_optimizer.py's --price-per-mtok:
- senior-ml-engineer SKILL.md and llm_integration_guide.md drop both 2024
price tables and the context-window table (which claimed GPT-4 = 8,192).
calculate_cost() takes rates as parameters; count_tokens() takes an
encoding name, since encodings outlive model IDs and
encoding_for_model() raises KeyError on anything unmapped
- OpenAIProvider loses its default model, so the caller must pass one
- llm-cost-optimizer's routing table names tiers, not models
Pinned to current IDs where an example genuinely needs one: SKILL_PIPELINE.md
(claude-opus-4-6 -> claude-opus-5), prompt-governance (claude-sonnet-4-5 ->
claude-sonnet-5), agent-designer README. Both dual-publish copies of the CAIO
pricing move together, so G4 stays green.
TEAM_STRUCTURE_GUIDE.md documented `prompt_optimizer.py --model gpt-4 --task
classification`. That contract no longer exists: there is no --task flag and
`prompt` is a required positional. Replaced with a runnable invocation.
Four references stay, with reasons in the allowlist: two litreview examples
where the retired model is the subject of the literature being reviewed, one
dated Computer Use citation, and the embedding benchmark already labelled a
2024 snapshot.
Assisted-by: Claude Code:claude-opus-5
audit/newgen-2026-06/00-MASTER.md proposed a "model-name freshness ... regex
deny-list for retired model identifiers" gate. It was never built, which is
why retired IDs and 2024 price tables survived both the June and July 2026
audits and are still in the tree today.
check_model_freshness.py flags references that mislead or break on execution:
script defaults, config values, cost tables keyed on a retired model, and
copy-pasteable CLI examples pinning a retired versioned ID. It distinguishes
these from legitimate dated citations, which stay silent when the line carries
a year, an arXiv ID, or wording like "model card" / "as of" / "historical" —
unless the line also looks like a live default, since
`model: str = "claude-3-opus" # 2024 default` still breaks.
Haiku 4.5 is excluded from the Claude 4 sweep in the patterns rather than
per-file, because claude-haiku-4-5-20251001 is current.
Advisory (continue-on-error) for now: it reports 34 references, 13 of them in
executable positions, and the content fixes land in the next change. Flip to
blocking there. --executable-only prints just the 13 that matter first.
Assisted-by: Claude Code:claude-opus-5
Every existing gate reads frontmatter with a regex or a line scan
(generate-docs.py, sync-codex-skills.py, check_paths.py), so a block that is
not valid YAML passed CI while Claude Code loaded the skill with no metadata.
The 14 files fixed in the previous commit had drifted that way unnoticed.
check_frontmatter.py parses each block with yaml.safe_load and enforces what
Claude Code actually reads:
errors - unparseable YAML, non-mapping frontmatter, missing description,
missing agent name, an agent name containing ':' (refused since
CC 2.1.218), or a missing frontmatter block
warnings - keys outside the current skill/agent frontmatter spec, and a
combined description + when_to_use over the 1536-char cap that
the skill listing truncates at
Warnings are non-blocking so this lands without requiring the wider metadata
cleanup; --strict flips them fatal. The run also tallies the off-spec keys no
runtime reads (license 172, metadata 125, domain 76, compatible_tools 37,
triggers 14), which gives that cleanup a worklist regenerated on every run.
Clean on the current tree: 593 files, 0 errors, 17 warnings.
Assisted-by: Claude Code:claude-opus-5
skill_validator.py required frontmatter fields ["Name", "Tier", "Category",
"Dependencies", "Author", "Version"] and sections ["Name", "Description",
"Features", "Usage", "Examples"]. That is the bold key/value convention used
by its own assets/sample-skill fixture, not YAML frontmatter and not a schema
any real skill has followed. All 362 skills failed both checks identically, so
skill-quality-review.yml has been printing the same two errors on every run
and nobody has acted on them.
- frontmatter now requires name + description, what Claude Code actually
reads, and lists the current optional fields for reference
- the fixed section list becomes a scored recommendation drawn from measured
usage. No heading appears in even 30% of the 361 real SKILL.md files, so a
required list cannot be justified; a miss is now a warning, not an error
- _check_external_imports uses sys.stdlib_module_names instead of a
hand-maintained set. The old set omitted __future__, so every script using
`from __future__ import annotations` was reported as carrying an external
dependency. Real third-party imports are still caught
- the sample-skill fixture gets valid YAML frontmatter, so the reference
implementation stops teaching the schema that caused this
cfo-advisor goes from 86.4 with 2 bogus errors to 95.5 with none. The one
remaining error class, "SKILL.md too short: minimum 100 lines", is left alone:
it contradicts skill_review_checklist_runner.py's under-100-lines check and
SKILL-AUTHORING-STANDARD.md's 10KB cap, and picking a winner belongs with the
wider CONVENTIONS.md reconciliation.
Assisted-by: Claude Code:claude-opus-5
Twelve files had frontmatter that yaml.safe_load rejects, all from the same
cause: an unquoted plain scalar containing ": " inside description. Claude Code
responds by loading the body with empty metadata, so the skill keeps working
via /name but has no description for the model to match against, and the seven
affected agents (where name and description are required) may not load at all.
Eleven are fixed by quoting the existing scalar, leaving the text
byte-identical. design-system carries both ' and " so quoting would defeat the
repo's regex extractors in generate-docs.py and sync-codex-skills.py; its one
colon-space is reworded instead ("Precedence:" -> "Precedence is").
Two agents had no frontmatter at all and were being listed with a placeholder
description; both now declare name and description. tools is deliberately
omitted so they keep inheriting the full set, as before.
Assisted-by: Claude Code:claude-opus-5
- Replace subprocess.run(cmd, shell=True) with shlex.split(cmd, comments=True) +
shell=False. Recipe templates contain unquoted {placeholder} tokens clearly
meant for future parameter substitution; shell=True combined with any future
substitution logic is a straightforward shell command injection vector, since
none of the placeholder positions are quoted. Removing shell=True closes that
class of bug now, before substitution is added, instead of relying on future
contributors to remember to quote/escape interpolated values.
- Require an explicit --yes flag before executing a non-dry-run recipe. Several
recipes are irreversible (gws drive files emptyTrash, gws gmail
users.messages send, gws drive permissions create, ...); previously --run
alone (without --dry-run) executed them immediately with no confirmation
gate.
