chore: the audit tail — frontmatter nit, skill sizes, and settled decisions - #20
Conversation
A blank line between `description` and the closing `---` — harmless to the parsers that read it, but the only skill of 19 with it. Also adds a frontmatter well-formedness check to the sweep so it stays that way.
The worklog nit was invisible to the existing check, which only asserted that name and description were present. One line to also assert the block is tight; verified it fails against the pre-fix file.
…/, 4,822 -> 3,678 The Python and TypeScript detection blocks are 1,057 words of greps, tool flags and thresholds — and **only one of them ever applies**, since `config.language` picks. Loading both on every run to use half is the definition of what references/ is for. What stays inline is the judgment: the category table that decides whether something is a finding, the repo-invariant drift check, the caps, and the routing rules. The reference is how to find it in a given language; SKILL.md is what counts as a finding. Scheduling also moves, matching what auto-dev and dependabot already do — cadence and model tier are the maintainer's setup question, not the run's.
…0 -> 4,063 Same split as audit-architecture: the per-language greps and tool flags go to references/, the category table that decides what counts as a weak test stays inline. The in-body anchor link to the section still resolves — the heading remains as the pointer. Scheduling moves too, for consistency with the rest of the suite. Lands at 4,063, above the 3k soft ceiling. What's left is the category table (1,142), the workflow (927), and the out-of-scope/what-not-to-do guidance that keeps an unattended skill from over-filing. Cutting further would mean moving the judgment, which is the thing that must be in front of the model.
…59 -> 5,209 The report format is output shaping — needed once, at the end, and never by the tick logic that decides what to do. It belongs beside comment-formats.md, which was extracted for the same reason. Still 209 words over the 5,000 guideline, and deliberately so. What remains is the tick state machine (2,859), the six safety invariants (652), the hard prohibitions (419), and the config contract (386). Every one of those has to be in front of the model on a run that writes code and pushes branches unattended — this is the skill where "load it from a reference when you need it" is the wrong trade. The 169 words it gained in #18 were invariant 7, added because the injection sweep found a real gap. Trimming those to reach 4,999 would be optimising the metric against the thing the metric exists for. Flagging the overage rather than hiding it.
Three questions that have each come up more than once — restoring the deleted code-review reference docs, building a docs site, and auto-dev's word count — now have their reasoning written down next to the answer. The first is the one worth recording: seeding bootstrap's coding.md with the old typescript-patterns.md would make the audits enforce generic practice, which is precisely the boundary #15 drew. That's not obvious from the deletion alone, so without a note the suggestion looks reasonable every time.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📜 Recent review details🧰 Additional context used🧠 Learnings (1)📚 Learning: 2026-08-10T21:45:18.765ZApplied to files:
🪛 SkillSpector (2.5.1)plugins/audits/skills/audit-security/SKILL.md[warning] 248: [EA2] Autonomous Decision Making: Skill enables autonomous high-impact decisions without human-in-the-loop verification. Critical operations (destructive commands, financial transactions, data deletion) should require explicit user confirmation. Remediation: Add human-in-the-loop confirmation for destructive, irreversible, or high-impact operations. Never auto-execute commands that modify files, send data, or alter system state. (Excessive Agency (EA2)) [info] 76: [EA3] Scope Creep: Skill's behavior or capabilities extend beyond its stated purpose. Scope creep allows an agent to perform actions unrelated to its documented functionality, increasing the attack surface. Remediation: Limit the skill's scope to its documented purpose. Remove instructions that enable the agent to perform actions outside its stated functionality. (Excessive Agency (EA3)) plugins/auto-dev/skills/auto-dev/SKILL.md[error] 56: [P2] Hidden Instructions: Hidden instructions were detected in comments or invisible text. These could contain malicious directives. Manual review is recommended. Remediation: Audit all comments and invisible characters. Remove any instructions that direct the agent to perform unauthorized actions. Use plain, reviewable content. (Prompt Injection (P2)) [error] 107: [YR1] YARA rule 'agent_skill_destructive_autonomous_actions': Autonomous destructive filesystem, shell history, or repository actions in AI agent skills [agent_skills]: YARA rule matched a known malware signature (reverse shell, backdoor, ransomware, C2 framework, or info stealer). Remediation: Remove the malware payload or compromised file entirely. Investigate how it entered the skill and audit all other artifacts for additional indicators of compromise. (YARA Match (YR1)) 🔇 Additional comments (3)
WalkthroughThe pull request moves audit and auto-dev procedures into reference documents, adds settled roadmap decisions, tightens skill frontmatter validation, and corrects Markdown rendering of a security regex. ChangesSkill guidance and repository updates
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/validate.yml:
- Around line 51-52: Update the frontmatter validation condition in the
workflow’s Python check to detect whitespace-only lines using re.search(r'(?m)^[
\t]*$', fm), while retaining the existing fm != fm.strip() check and failure
behavior.
In `@docs/roadmap.md`:
- Around line 164-166: Capitalize the documentation format reference in the
roadmap sentence by changing “markdown” to “Markdown”; leave the surrounding
wording and formatting unchanged.
In `@plugins/audits/skills/audit-architecture/references/language-detection.md`:
- Line 9: Update the Python and TypeScript language headings to H2 in
plugins/audits/skills/audit-architecture/references/language-detection.md at
lines 9 and 30, and in
plugins/audits/skills/audit-tests/references/language-mechanics.md at lines 9
and 22, so both documents consistently use H2 sections beneath their H1 titles.
- Line 22: Update the version note in the naive datetime usage entry to state
that datetime.utcnow() has been deprecated since Python 3.12, leaving the
surrounding detection command and review guidance unchanged.
- Line 16: Update the Any-overuse grep command in the language-detection
guidance to use portable grep syntax: invoke extended regular expressions and
replace GNU-specific \> and \| constructs with explicit character classes or
separate -e patterns, while continuing to match the existing type and cast cases
and exclude import lines.
- Line 36: Update the oversized-files command in the language-detection guidance
to use the recursive per-file find pipeline already used by the Python check,
ensuring nested TypeScript files are scanned without relying on Bash globstar.
Apply the same command change to the corresponding oversized-files guidance in
the SKILL.md audit instructions.
In `@plugins/audits/skills/audit-architecture/SKILL.md`:
- Around line 44-46: Update the audit-architecture skill to complete the
extraction of language-specific detection mechanics: remove the duplicated
commands and thresholds from the language blocks around lines 81-104, and revise
the editing guidance around line 324 to direct threshold changes to
references/language-detection.md. Keep the findings-judgment table and its
single source-of-truth behavior intact.
In `@plugins/audits/skills/audit-tests/references/scheduling.md`:
- Around line 3-5: Update the cross-audit coordination wording in the scheduling
guidance and the related audit-tests documentation around its open arch-* PR
skip rule: describe audit-tests and audit-architecture as having separate
ownership while both avoiding conflicts by skipping files touched by the other
audit’s open PRs. Remove the inaccurate claim that they do not overlap or
deduplicate only within their own labels and branch prefixes.
In `@plugins/auto-dev/skills/auto-dev/references/exit-report.md`:
- Around line 6-10: Update the structured report example in the exit-report
documentation so it includes explicit outcome entries for all six steps,
0-failed through 5-idle, rather than a single “step executed” value. Keep the
format consistent with SKILL.md and distinguish completed, no-work, and
unreached outcomes where applicable.
In `@plugins/auto-dev/skills/auto-dev/SKILL.md`:
- Around line 258-261: Update the “Reference files” index in SKILL.md to include
references/exit-report.md, matching the existing entry format and preserving the
current reference ordering.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6c2c51b1-aeb5-4965-9e31-60c95d57e83f
📒 Files selected for processing (11)
.github/workflows/validate.ymldocs/roadmap.mdplugins/audits/skills/audit-architecture/SKILL.mdplugins/audits/skills/audit-architecture/references/language-detection.mdplugins/audits/skills/audit-architecture/references/scheduling.mdplugins/audits/skills/audit-tests/SKILL.mdplugins/audits/skills/audit-tests/references/language-mechanics.mdplugins/audits/skills/audit-tests/references/scheduling.mdplugins/auto-dev/skills/auto-dev/SKILL.mdplugins/auto-dev/skills/auto-dev/references/exit-report.mdplugins/journal/skills/worklog/SKILL.md
💤 Files with no reviewable changes (1)
- plugins/journal/skills/worklog/SKILL.md
📜 Review details
🧰 Additional context used
🪛 LanguageTool
plugins/audits/skills/audit-tests/references/scheduling.md
[grammar] ~3-~3: Ensure spelling is correct
Context: ...ts is fine; they don't overlap and each dedups against its own label/branch prefix. *...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
plugins/audits/skills/audit-architecture/references/language-detection.md
[grammar] ~3-~3: Ensure spelling is correct
Context: ...tion — audit-architecture The concrete greps, tool invocations and thresholds for ea...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
plugins/audits/skills/audit-tests/references/language-mechanics.md
[grammar] ~3-~3: Ensure spelling is correct
Context: ...c mechanics — audit-tests The concrete greps and tool flags behind the category tabl...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
docs/roadmap.md
[uncategorized] ~166-~166: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...l, the repo already ships its docs as markdown, and a generated site would add a sync ...
(MARKDOWN_NNP)
🪛 markdownlint-cli2 (0.23.2)
plugins/audits/skills/audit-architecture/references/language-detection.md
[warning] 9-9: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
plugins/audits/skills/audit-tests/references/language-mechanics.md
[warning] 9-9: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🪛 SkillSpector (2.5.1)
plugins/auto-dev/skills/auto-dev/SKILL.md
[error] 56: [P2] Hidden Instructions: Hidden instructions were detected in comments or invisible text. These could contain malicious directives. Manual review is recommended.
Remediation: Audit all comments and invisible characters. Remove any instructions that direct the agent to perform unauthorized actions. Use plain, reviewable content.
(Prompt Injection (P2))
[error] 107: [YR1] YARA rule 'agent_skill_destructive_autonomous_actions': Autonomous destructive filesystem, shell history, or repository actions in AI agent skills [agent_skills]: YARA rule matched a known malware signature (reverse shell, backdoor, ransomware, C2 framework, or info stealer).
Remediation: Remove the malware payload or compromised file entirely. Investigate how it entered the skill and audit all other artifacts for additional indicators of compromise.
(YARA Match (YR1))
🔇 Additional comments (4)
plugins/audits/skills/audit-architecture/SKILL.md (1)
344-345: LGTM!plugins/audits/skills/audit-architecture/references/scheduling.md (1)
1-8: LGTM!plugins/audits/skills/audit-tests/references/language-mechanics.md (1)
7-11: 🎯 Functional CorrectnessDo not assume that Python means pytest.
Lines 11-20 hard-code pytest syntax, but
config.languageonly selects Python. If the configured test command usesunittestor another runner, these checks can produce false negatives. Make the Python mechanics conditional on the configured runner, or state that this reference supports pytest only.plugins/audits/skills/audit-tests/SKILL.md (1)
85-87: LGTM!Also applies to: 326-327
The check I added yesterday tested `'\n\n' in fm`, which sees a truly empty line and misses `\n \n`. A regex for a whitespace-only line catches both. Verified: the old form wrongly passes `name: x\n \ndescription: y`.
…e mechanics The first pass moved the per-language *detection table* to references/ and left the per-language *ordered sweep list* in the workflow, restating the same commands. Two sources of truth, and the threshold-tuning note still said "edit the language block of this skill" — pointing at content that had moved. Consolidates the sweep order into the reference beside the commands it orders, repoints the threshold note, and leaves zero language-specific commands in SKILL.md. 3,678 -> 3,081. Also from the review, all in the reference: - the `Any` grep used `\>` and `\|` and claimed `\>` was "POSIX BRE". Both are GNU/BSD extensions; switched to `grep -E` with an explicit non-word class. - `datetime.utcnow()` is deprecated since 3.12, not 3.13. - the TypeScript oversized-file scan used `**/*.ts`, which silently misses nested files without `shopt -s globstar` — the exact trap the Python row two lines up already documents. Now uses the same `find -exec` pipeline. - H1 -> H3 heading jump in both extracted references, now H2.
The extracted scheduling note said the two audits "don't overlap and each dedups against its own label/branch prefix." True as far as ownership goes, but this skill also skips any file touched by an open `arch-*` PR — a real cross-audit coordination rule that the note flatly contradicted.
…eport.md My pointer said "report **every** step outcome including 5-idle", which reads as "list all six" — the review took it that way and proposed six entries. The format is right and the pointer was wrong: a tick executes exactly one step, so `step executed:` names that one. Reworded rather than changing the format. Also adds exit-report.md to the Reference files index, which claims to be the index and wasn't, and capitalises Markdown in the roadmap.
|
Round 2 — ten findings, nine taken and one taken differently. The one that mattered: my extraction was half-done. Three factual corrections inside that reference, all pre-existing text this PR moved rather than wrote — fixed rather than carried forward:
Taken differently: the exit-report And one on yesterday's work: the frontmatter check I added in Final sizes: |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/audits/skills/audit-architecture/references/language-detection.md`:
- Around line 69-72: Update the TypeScript oversized-files check in the
language-detection audit to use the recursive find-based sweep established near
line 36, ensuring nested .ts files are included even without Bash globstar and
results are sorted by line count descending. Preserve the existing
threshold-reporting behavior.
- Line 36: Escape the pipeline separator in the command within the
oversized-files table cell, matching the escaped form used in the Python row.
Update only the Markdown table entry so the command remains accurate and the
cell is parsed as a single column.
- Line 23: Update the “Sync DB calls in async paths” guidance to exclude
session.add() from required await checks and state that AsyncSession.add() must
be called without await, while retaining await requirements for
session.execute(), session.commit(), and session.flush().
In `@plugins/auto-dev/skills/auto-dev/SKILL.md`:
- Around line 260-262: Update the SKILL.md tick/reporting guidance and the
aligned references/exit-report.md definition so “step executed:” identifies the
single terminal work outcome, not the entire tick. Account for PR-label
restamping before the numbered flow and Steps 2–4 falling through; record label
repairs or failures in actions or errors, while retaining the 5-idle outcome for
ticks with no terminal work.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4786e97c-a22b-4167-9eb3-3da49476ebf3
📒 Files selected for processing (7)
.github/workflows/validate.ymldocs/roadmap.mdplugins/audits/skills/audit-architecture/SKILL.mdplugins/audits/skills/audit-architecture/references/language-detection.mdplugins/audits/skills/audit-tests/references/language-mechanics.mdplugins/audits/skills/audit-tests/references/scheduling.mdplugins/auto-dev/skills/auto-dev/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/validate.yml
- docs/roadmap.md
- plugins/audits/skills/audit-tests/references/language-mechanics.md
📜 Review details
🧰 Additional context used
🪛 LanguageTool
plugins/audits/skills/audit-tests/references/scheduling.md
[grammar] ~3-~3: Ensure spelling is correct
Context: ...s separate — source vs tests — and each dedups against its own label/branch prefix, bu...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.23.2)
plugins/audits/skills/audit-architecture/references/language-detection.md
[warning] 36-36: Spaces inside code span elements
(MD038, no-space-in-code)
[warning] 36-36: Table column count
Expected: 3; Actual: 4; Too many cells, extra data will be missing
(MD056, table-column-count)
🪛 SkillSpector (2.5.1)
plugins/auto-dev/skills/auto-dev/SKILL.md
[error] 56: [P2] Hidden Instructions: Hidden instructions were detected in comments or invisible text. These could contain malicious directives. Manual review is recommended.
Remediation: Audit all comments and invisible characters. Remove any instructions that direct the agent to perform unauthorized actions. Use plain, reviewable content.
(Prompt Injection (P2))
[error] 107: [YR1] YARA rule 'agent_skill_destructive_autonomous_actions': Autonomous destructive filesystem, shell history, or repository actions in AI agent skills [agent_skills]: YARA rule matched a known malware signature (reverse shell, backdoor, ransomware, C2 framework, or info stealer).
Remediation: Remove the malware payload or compromised file entirely. Investigate how it entered the skill and audit all other artifacts for additional indicators of compromise.
(YARA Match (YR1))
[error] 56: [P2] Hidden Instructions: Hidden instructions were detected in comments or invisible text. These could contain malicious directives. Manual review is recommended.
Remediation: Audit all comments and invisible characters. Remove any instructions that direct the agent to perform unauthorized actions. Use plain, reviewable content.
(Prompt Injection (P2))
[error] 107: [YR1] YARA rule 'agent_skill_destructive_autonomous_actions': Autonomous destructive filesystem, shell history, or repository actions in AI agent skills [agent_skills]: YARA rule matched a known malware signature (reverse shell, backdoor, ransomware, C2 framework, or info stealer).
Remediation: Remove the malware payload or compromised file entirely. Investigate how it entered the skill and audit all other artifacts for additional indicators of compromise.
(YARA Match (YR1))
🔇 Additional comments (3)
plugins/audits/skills/audit-architecture/SKILL.md (1)
44-46: LGTM!Also applies to: 81-84, 300-300, 320-321
plugins/audits/skills/audit-tests/references/scheduling.md (1)
3-5: LGTM!plugins/auto-dev/skills/auto-dev/SKILL.md (1)
292-292: LGTM!
…ish the glob sweep `AsyncSession.add()` is synchronous and returns `None`. The detection rule grepped for it alongside `execute`/`commit`/`flush` and told the run to "confirm each is await-ed" — so an agent following it would take working code and produce `TypeError: object NoneType can't be used in 'await' expression`. A rule that manufactures the bug it screens for is worse than no rule. Narrowed the grep to the genuinely awaitable methods and said outright why `add` is excluded. The TypeScript glob fix landed on one row and missed the others. This time swept the whole file rather than the flagged line: three more sites, including one in the sweep-order list consolidated in the previous commit — the same fix, in a copy created by the same commit that applied it. Also escapes two raw `|` characters inside markdown table cells: one I introduced with the `find … | sort` command, one pre-existing in audit-security's secret-shapes regex. Added a repo-wide table-integrity check to the sweep; both were the only instances.
"A tick executes exactly one step" was my rewording and still imprecise: the step-0 PR-label restamp runs before the numbered flow, and steps 2-4 fall through when they have no work. `step executed:` names the one step that did the tick's work; restamps and failures belong under `actions:`/`errors:`.
|
Round 3 — four findings, all valid, in The one that mattered: a rule that manufactures the bug it screens for. The "sync DB calls missing await" check grepped for And a process failure worth naming. The TypeScript glob fix in round 2 landed on one row and missed three more — one of which was created by the same commit that applied the fix, when I consolidated the sweep-order list into that file. That's the third instance on this PR of fixing one site and missing its twin (round 2: moved one language block, left the other; round 2: fixed the table row, left the list; now this). So this round I swept the file rather than patching the flagged line — four glob sites, of which the review flagged two — and added a repo-wide table-integrity check, which turned up a pre-existing raw pipe in The pattern is consistent enough to state: after any fix, search for the shape, not the line. |
1 participant
The last three optional items. Two are done; the third is done as far as it should go, and the PR says where it stops and why.
1. The
worklogfrontmatter nitA blank line between
descriptionand the closing---— the only skill of 19 with it (aa3c446).6ed651dextends CI's frontmatter check to reject the class, verified to fail against the pre-fix file. The existing check only assertednameanddescriptionwere present, which is why it never saw this.2. Skill sizes
audit-architecturereferences/audit-testsreferences/auto-devreferences/The clean win in both audits is the same: per-language detection blocks, of which only one ever applies —
config.languagepicks — so loading both to use half is exactly whatreferences/is for. What stays inline is the judgment: the category table deciding what counts as a finding, the caps, the routing. The reference is how to find it in this language; SKILL.md is what counts.auto-devstops at 5,209, 209 over the guideline, deliberately. What's left is the tick state machine (2,859), six safety invariants (652), the hard prohibitions (419), and the config contract (386). Those have to be in front of the model on an unattended run that writes code and pushes branches — this is the one skill where "load it from a reference when needed" is the wrong trade. 169 of the words it gained came from #18's invariant 7, added because the injection sweep found a real gap. Trimming that to reach 4,999 would optimise the metric against its purpose.audit-testslikewise lands above the 3k soft ceiling. Cutting further means moving the category table, which is the judgment itself.3. The deleted reference docs — decided, not restored
typescript-patterns.md(~400 lines) andreview-checklist.md, dropped with #14. The standing suggestion was to restore them as seed material forbootstrap'scoding.md.No — and the reason is worth having written down:
coding.mdis whataudit-architectureand any reviewer read as "the rules this repo wrote down." Seeding it with generic TypeScript craft would make the audits enforce generic practice, which is exactly the boundary #15 drew when it handed dangerous-code-patterns to/security-reviewand kept only repo-specific invariants. Generic practice belongs to the built-in reviewer;guidelines/*.mdexists for what a general reviewer cannot know. Both files stay in git history.e85806arecords that indocs/roadmap.mdalongside the no-docs-site and auto-dev-size decisions — three questions that have each surfaced more than once and now have reasoning attached, since none is obvious from the artefact alone.Verification
All six commits pass CI standalone. Content moved, not lost — 143 links resolve in both layouts, 0 boundary escapes, 0 stale prose refs, vendored copies in sync, manifests agree with the marketplace, all 19 frontmatters clean. The in-body anchor to
#language-specific-mechanicsstill resolves; the heading remains as the pointer.Summary by CodeRabbit
Documentation
Refactor
Bug Fixes