Skip to content

feat(review,docs-hygiene): wire documentation-existence admission pre-check#846

Merged
kyle-sexton merged 1 commit into
mainfrom
feat/505-admission-precheck
Jul 21, 2026
Merged

feat(review,docs-hygiene): wire documentation-existence admission pre-check#846
kyle-sexton merged 1 commit into
mainfrom
feat/505-admission-precheck

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Jul 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

  • Adds a report-only existence-before-accuracy pre-check to review:doc-drift-detector — before classifying Stale/Missing/Aspirational, ask whether a reader with repository search could derive the page's content from the code itself; an admission failure routes to a new Deletion-candidate category (recommend relocate-then-delete, never auto-delete). Decisions, domain language, thin navigation, and policy/wiring pages always pass admission.
  • Adds the same whole-page admission pre-check to docs-hygiene:audit-noise ahead of its five in-page NOISE shapes — a FAIL skips the in-page tier table and recommends relocate-then-delete instead.
  • Both reuse the shipped docs-hygiene:audit-derivability rubric by reference (namespaced skill invocation, optional — degrades to the admission question standalone when that plugin is unavailable), per the plugin-isolation convention already established in this repo (plugins/discovery/reference/artifact-protocol.md: "must not import sibling plugin internals or assume another plugin is installed").
  • Ships as the operator-ACCEPTED portable-baseline default (2026-07-19, decision-session batch 2); a consuming repo's own declared documentation-existence convention overrides via re-anchor:follow-our-standards's resolution ladder (repo-declared source → repo's own conventions → this portable baseline).
  • Zero hard-coded references to melodic-software/standards (verified via grep on the diff) — the hard constraint from the issue holds.
  • Version bumps: review 0.14.8 → 0.14.9, docs-hygiene 0.8.0 → 0.8.1, with CHANGELOG entries for both.

Closes #505

Related

Test plan

  • skill-quality:check on audit-noise — PASS, 0 errors, 1 pre-existing warning (no Gotchas surface, unrelated to this change)
  • Verified no hard-coded melodic-software/standards reference in the diff
  • Verified allowed-tools semantics against https://code.claude.com/docs/en/skills — it is a pre-approval list, not a hard restriction, so the added pre-check can invoke Read/Grep/Skill under normal permission handling even though audit-noise's frontmatter only pre-approves its detect script
  • Commit is locally signed (git log --show-signature confirms "Good git signature")

@kyle-sexton
feat(review,docs-hygiene): wire documentation-existence admission pre…
c2ebac4 
…-check

Adds a report-only existence-before-accuracy pre-check to doc-drift-detector
and a whole-page admission pre-check to audit-noise, both reusing the shipped
audit-derivability rubric by reference (optional namespaced skill invocation,
degrading to the admission question standalone when unavailable). Ships as a
portable-baseline default; a consuming repo's own declared
documentation-existence convention overrides via the follow-our-standards
resolution ladder. Never hard-codes melodic-software/standards.

Closes #505
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Jul 21, 2026

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@claude

claude Bot commented Jul 21, 2026 •
edited
Loading

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1m 19s —— View job

Security Review

  • Gather PR diff and changed files
  • Review plugins/review/agents/doc-drift-detector.md for security issues
  • Review plugins/docs-hygiene/skills/audit-noise/SKILL.md for security issues
  • Review plugin.json version bumps
  • Review CHANGELOG entries
  • Synthesize findings and post review

Scope

Six files changed: two agent/skill instruction files (markdown), two plugin.json version bumps, two CHANGELOG.md entries. All changes are text/instruction additions — no new scripts, no workflow files, no dependency pins, no settings/permissions files.

Findings

No new exploitable security vulnerabilities introduced by this PR.

Rationale for each surface checked:

1. Cross-plugin invocation (/docs-hygiene:audit-derivability)
Both modified files instruct the agent to optionally invoke a sibling skill by namespaced reference. The invocation is read-only, degrades gracefully to a standalone admission question when unavailable, and passes no credentials or sensitive state. No supply-chain or privilege-escalation risk.

2. allowed-tools frontmatter in audit-noise
The pre-check behavior added by this PR requires Read, Grep, and Skill access beyond the frontmatter's pre-approved Bash(bash *audit-noise/scripts/detect.sh*). The PR description correctly notes that allowed-tools is a pre-approval list (reduces permission prompts), not a hard capability fence — those additional tool calls will surface normal user permission prompts. No silent capability expansion.

3. doc-drift-detector agent tool access (Bash included)
The agent has had Bash access since before this PR. The new instructions reinforce the read-only, never-auto-delete constraint rather than relaxing it. The fundamental prompt-injection risk from reading untrusted doc content is a pre-existing condition of the agent, not introduced here.

4. Org-override resolution ladder (/re-anchor:follow-our-standards)
The override path references an external plugin. If that plugin is absent the instructions fall back to the portable baseline — a safe degradation path. No trust delegation to an uncontrolled external origin.

5. Pre-computed context shell invocations in SKILL.md
Lines 12–14 (git branch --show-current, git status --porcelain, bash detect.sh) were present before this PR and are unchanged. Not in scope for this diff.

6. No secrets, PII, credentials, or hardcoded paths introduced.
Verified by grep on the diff — confirmed by the PR description as well.

7. No GitHub Actions workflow changes. No pull_request_target, workflow_run, script injection, or permission-widening changes.

No security issues to report.

@claude

claude Bot commented Jul 21, 2026 •
edited
Loading

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 2m 38s —— View job

PR Review — feat(review,docs-hygiene): wire documentation-existence admission pre-check

  • Gather context and read changed files
  • Review plugins/review/agents/doc-drift-detector.md
  • Review plugins/docs-hygiene/skills/audit-noise/SKILL.md
  • Review plugin.json version bumps and CHANGELOG entries
  • Check alignment with CLAUDE.md / plugin design rules
  • Post final review findings

Summary

The change is well-structured, correctly repo-agnostic, and maintains the read-only contract for both surfaces. The degradation path for the optional cross-plugin audit-derivability reference is the right call. Two issues with the output schema are worth addressing before merge; one minor asymmetry is worth a note.

Findings

1. doc-drift-detector output format doesn't define how deletion-candidates appear in the table (medium)

The existing output table | Doc file | Line | Claim | Actual state | Action | was designed for line-level findings. Adding Deletion-candidate as a new category (whole-page, pre-check level) doesn't update the table schema or provide a row example for it. An agent following these instructions would have to invent column values — Line, Claim, and Actual state have no clear meaning for a page-level verdict.

Compare to audit-noise/SKILL.md, which explicitly separates the pre-check output from the in-page tier table with its own format:

<file>: admission PASS
<file>: admission FAIL — deletion candidate (relocate-then-delete recommended)

doc-drift-detector.md should either (a) add a pre-check verdict line before the findings table (mirroring audit-noise's approach), or (b) add an example row for a deletion-candidate showing what Line/Claim/Actual state should contain (e.g., —, —, Failed existence pre-check).

Permalink to the output format section →

Fix this →

2. audit-noise batch aggregate doesn't count deletion-candidate (FAIL) pages (medium)

The batch aggregate in audit-noise/SKILL.md's output schema is:

Total: <N> file(s) audited, <T1> Tier 1, <T2> Tier 2, <T3> Tier 3 findings.

Because a FAIL page skips the in-page tier table, its finding never lands in T1/T2/T3. A consuming workflow or user reading the batch summary has no way to know how many pages failed admission. The aggregate should include a deletion-candidate count, e.g.:

Total: <N> file(s) audited — <D> deletion-candidate(s), <T1> Tier 1, <T2> Tier 2, <T3> Tier 3 findings.

Permalink →

Fix this →

3. Asymmetric verbosity of the four always-pass categories (minor)

doc-drift-detector.md explains each always-pass category with a descriptive clause (e.g., "Decisions — a chosen option erases the record of alternatives rejected"). audit-noise/SKILL.md lists the same four as a compact inline run without explanation: "decisions, domain language, thin navigation, and policy/wiring."

Since these are semantically the same admission criteria, a user applying audit-noise in isolation would have less guidance on what qualifies. This isn't a blocking concern given the degradation path to audit-derivability, but it's worth either expanding audit-noise's list inline or adding a cross-reference note ("for definitions, see doc-drift-detector or audit-derivability's rubric").

Permalink →

What looks good

  • Repo-agnostic: confirmed zero hardcoded references to melodic-software/standards in the diff.
  • Plugin isolation: the cross-plugin reference to /docs-hygiene:audit-derivability is optional and degrades cleanly; this follows the artifact-protocol convention.
  • Read-only contract preserved: neither surface introduces mutations; the "never auto-delete" and "report-only" constraints are explicit in both.
  • Org override mechanism: the resolution ladder (re-anchor:follow-our-standards) is consistently present in both surfaces and matches the portable-baseline framing.
  • Version bumps: both are correct patch increments (0.8.0→0.8.1, 0.14.8→0.14.9) and match CHANGELOG entries.
  • ${CLAUDE_PLUGIN_ROOT} usage: the severity baseline reference uses the correct variable (no ../ reach-outs).
  • CHANGELOG entries: both are accurate and clearly attributed to Adopt the documentation-existence admission test as a portable-baseline pre-check in doc-drift and declutter tooling #505.

@kyle-sexton kyle-sexton mentioned this pull request Jul 21, 2026
Open
@kyle-sexton

kyle-sexton commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

Tower merge-sit (T4 dark, stall protocol): CLEAN, 22/22 green, 0 unresolved threads. This was generated by AI (control tower, session 6).

@kyle-sexton
kyle-sexton merged commit 92ac236 into main Jul 21, 2026
24 of 25 checks passed
@kyle-sexton
kyle-sexton deleted the feat/505-admission-precheck branch July 21, 2026 16:11
kyle-sexton added a commit that referenced this pull request Jul 21, 2026
@kyle-sexton
Merge origin/main into fix/474-noise-shapes-helper
6f2a6b9 
Resolves a version-stacking conflict in session-flow: origin/main landed
0.12.0 (new orient skill, hardened keep-going) after this branch bumped
0.11.0 -> 0.11.1. Restacked this PR's handoff doc-pointer fix on top as
0.12.1, preserving both CHANGELOG entries and origin's current
nine-skill description. docs-hygiene had no collision -- the PR the
conflict was suspected to be from (#846) is still open, not merged.
kyle-sexton added a commit that referenced this pull request Jul 21, 2026
@kyle-sexton
Merge origin/main into fix/474-noise-shapes-helper
31286aa 
#846 (whole-page existence pre-check for audit-noise) merged to main
moments ago and independently bumped docs-hygiene 0.8.0 -> 0.8.1 with a
different CHANGELOG entry -- an exact version collision with this
branch's own 0.8.0 -> 0.8.1 bump (git's 3-way merge silently kept one
side since the version-string edit was textually identical on both,
which would have masked this PR's fix entry). Restacked this PR's
convention-roots parser fix on top as 0.8.2, keeping #846's 0.8.1 entry
intact beneath it.
This was referenced Jul 21, 2026
Merged
Open
kyle-sexton added a commit that referenced this pull request Jul 21, 2026
@kyle-sexton @claude
fix(docs-hygiene,session-flow): wire remaining memory_dir consumers t…
3962e8f 
…o shared parse-concern-value helper (#847)

## Summary

Final wiring of the ratified bounded-widen scope on #474: fold the last
two verified hand-rolled `memory_dir`/`contract_dir` copies into #482's
already-landed `parse-concern-value.sh` helper (merged via #756).

-
**`plugins/docs-hygiene/skills/audit-noise/scripts/lib/noise-shapes.sh`**
— `audit_noise_convention_roots_pattern` hand-rolled the same naive
`${val%%#*}`-first strip #482 fixed at its two hot sites (truncates a
quoted value at an interior `#`), plus a defect unique to this copy:
`${val//[[:space:]]/}` collapsed *all* interior whitespace, and there
was no quote-stripping. Now resolves both keys through the shared
helper, materialized to
`plugins/docs-hygiene/skills/audit-noise/scripts/lib/parse-concern-value.sh`
(byte-identical + mode-identical, `100755`, to the two existing copies)
and registered in `scripts/sync-parse-concern-value.sh`'s `copies` array
— the same upstream-SSOT + sync-materialization pattern as the two prior
sites (installed plugins are cache-isolated and cannot `source` a
sibling's file).
- **`plugins/session-flow/skills/handoff/context/structure.md`** L116 —
replaced the placeholder note (`# the concern file's memory_dir when set
— resolve it first`, no mechanism named) with a pointer to the shared
helper via the retro skill's Phase 1.1 as the worked call form. Doc note
only: the handoff skill has no script of its own to rewire.

Per the operator's ratified bounded-widen decision (2026-07-19T23:26Z)
and the tower's 2026-07-21 decision comment on #474, the broader
tracker-seam-resolution class (#470/#469/#432/#365/#369) stays
explicitly OUT of scope.

Version bumps stack past two independently-merged sibling PRs that
landed while this one was open: docs-hygiene `0.8.0` → `0.8.2` (stacks
past #846's `0.8.1`, an exact version collision resolved by re-numbering
this PR's entry on top and preserving #846's CHANGELOG entry
underneath); session-flow `0.11.0` → `0.12.1` (stacks past a merged
`0.12.0` that added the `orient` skill).

## Related

- #482 / #756 — landed the shared `parse-concern-value.sh` helper and
its first two consumers; this PR extends it to the remaining two per the
ratified bounded-widen scope.
- #846 — merged concurrently with this PR and independently bumped
`docs-hygiene` to `0.8.1`; this PR's version was restacked to `0.8.2` on
top, both CHANGELOG entries preserved.
- #470 / #469 / #432 / #365 / #369 — the broader tracker-seam-resolution
class; explicitly excluded from this PR's scope per the operator's
ratified decision.

## Test plan

- [x] `bash
plugins/docs-hygiene/skills/audit-noise/scripts/detect.test.sh` — 34/34
pass, including a new regression case: a quoted `memory_dir:
".scratch#dir/" # trailing comment` now correctly resolves to
`.scratch#dir` (flags `.scratch#dir/foo/` as a concrete slice) instead
of the old parser's silent truncation.
- [x] Manual before/after demonstration of the two defects the old
parser had:
- Quoted + interior `#` + trailing comment + trailing slash: old
`[.scratch]` (wrong — truncated inside the quotes) vs new
`[.scratch#dir]` (correct).
- Unquoted value with interior whitespace: old `[mydir]` (wrong —
collapsed) vs new `[my dir]` (correct, surrounding whitespace trimmed
only).
- [x] `scripts/sync-parse-concern-value.sh --check` — all 3 plugin
copies match `lib/parse-concern-value.sh`.
- [x] `scripts/check-cross-plugin-source-drift.sh --check` — no
unregistered/drifted clusters (the three copies have distinct
within-plugin relpaths, so the generic drift check doesn't need to
cluster them; the dedicated sync gate already covers them).
- [x] `scripts/check-changelog-parity.sh --check` and `--check-bump
origin/main` — both touched plugins (`docs-hygiene` 0.8.0→0.8.2,
`session-flow` 0.11.0→0.12.1) carry matching CHANGELOG entries.
- [x] `shellcheck` + `shfmt -d` — clean on all new/edited scripts.
- [x] `scripts/validate-plugins.sh` — all manifests + catalog valid.

Closes #474

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Adopt the documentation-existence admission test as a portable-baseline pre-check in doc-drift and declutter tooling

1 participant

@kyle-sexton