[jimcadden]

Summary

• Adds the model-free doc-drift program: doc-drift-check.py (pinned-ref checker), scanner, fixer, and precheck.
• Checks: paths, flag/subcommand parity, configured enumerations, retired names, versions, absence claims, and Keep a Changelog date vs GitHub publishedAt plus Unreleased drafts from merged PRs.
• Wires doc-drift into KNOWN_PROGRAMS, repoman-run.sh, and the automation-health dashboard; shellcheck + fixture tests in CI.
• Review follow-up: changelog history and dependency pins are not version drift, mechanical edits touch only the flagged line, a failed scan keeps the previous findings, compact YAML lists parse, and an older tag does not become the version to rewrite toward.

Engine source of truth for rossoctl/repoman#8. Companion vendor PR will follow in rossoctl/repoman after this merges.

Follow-ups

Out of scope here, tracked separately (both from the review at #pullrequestreview-5742023444; neither blocks this PR):

• doc-drift-scanner.sh has no behavioural test: carry-forward, rollup open/close and ledger are untested #128 — doc-drift-scanner.sh has no behavioural test: carry-forward, rollup open/close and ledger are untested
• doc-drift: release/PR metadata fetch is duplicated in scanner and precheck and costs N+3 gh calls per repo #129 — release/PR metadata fetch is duplicated in scanner and precheck and costs N+3 gh calls per repo

Test plan

• bash tests/test-doc-drift-check.sh
• bash tests/test-doc-drift-precheck.sh
• shellcheck --severity=warning scripts/doc-drift-scanner.sh scripts/doc-drift-fixer.sh scripts/doc-drift-precheck.sh
• CI shellcheck (doc-drift) + suite entries for the new tests

Assisted-By: Claude Code

[clawgenti]

Solid, well-structured addition — CI green, DCO signed, the fail-open/fail-closed contracts are consistent, and the offline checker avoids injection-prone subprocess patterns. Non-blocking findings:

• scripts/automation-health-dashboard.sh:409 — trend Delta column will always render 0 (see inline).
• scripts/doc-drift-check.py:241 — mini YAML parser hardcodes which keys are lists.
• scripts/doc-drift-check.py:39 — SUBCMD_DOC_RE only matches same-line subcommand lists.
• tests/test-doc-drift-precheck.sh:41 — exit-code/stdout contracts are simulated rather than exercised against the real script.

---

Reviewed by clawgenti using the github-pr-review skill

[jimcadden]

Addressed the four review suggestions in fbf8f7b:

1. Dashboard Delta — uses (.new // 0) / (.fixed // 0) (scanner already emits both; coalescing covers older rows).
2. YAML list keys — list-vs-map is decided by peeking at the next significant line for a deeper/same-block - item; no hard-coded key names.
3. Subcommands bullets — extract_listed_subcommands also collects following - item lines (same column as the header is OK).
4. Precheck test — exercises the real doc-drift-precheck.sh against clean (exit 1) and drift (exit 0 + worklist:) fixtures with gh stubbed.

Assisted-By: Claude Code

[clawgenti]

All four previous-round findings are addressed — the scanner now emits new/fixed deltas for the dashboard trend, the YAML subset parser detects lists structurally, subcommand lists accept multi-line bullets, and the precheck test exercises the real script. Remaining findings are minor dead-code cleanups; nothing blocking.

• scripts/doc-drift-check.py:234 — unreachable branch in parse_simple_yaml (nit)
• scripts/doc-drift-check.py:402 — redundant exists re-check in check_paths (nit)
• scripts/doc-drift-check.py:809 — dead pass block in check_versions (nit)
• scripts/doc-drift-fixer.sh:240 — enumerations|paths case never stages an edit (suggestion)

---

Reviewed by clawgenti using the github-pr-review skill

[jimcadden]

Addressed the second-round nits in c4b55a6:

1. Deleted the unreachable {...} branch inside the dash-item parser.
2. Removed the redundant path-existence re-check in check_paths.
3. A stable tag boundary now overrides the marketplace version instead of pass.
4. enumerations and paths corrections stay on the rollup issue; the fixer no longer checks out a branch it never edits.

Assisted-By: Claude Code

[clawgenti]

Well-structured addition — the pinned-ref design, offline checker, and fixture coverage are solid, and the scanner/fixer wiring matches the existing link-health/dep-bump patterns. One real bug in the versions check slipped past the fixtures (the CHANGELOG.md exclusion never matches), plus two small dead-code leftovers.

• must-fix scripts/doc-drift-check.py:809 — the CHANGELOG.md skip in check_versions never fires, so historical ## [x.y.z] headings become false-positive version-drift findings (verified on a fixture), and the fixer would then rewrite those headings repo-wide.
• suggestion scripts/doc-drift-check.py:998 — newest_blob_cited is computed but never used.
• nit scripts/doc-drift-check.py:431 — both branches of this conditional are identical.

---

Reviewed by clawgenti using the github-pr-review skill

[jimcadden]

Addressed the open findings in b95f446.

• CHANGELOG.md skip compares the lowercased basename, and version matches now require the word "version" nearby, so changelog headings and requirements.txt pins are not mechanical drift.
• Fixer replaces only the flagged line (word boundaries, so 1.2.3 does not match inside 11.2.34). A failed rebase aborts and returns the shared clone to main. The rolling branch pushes with --force-with-lease.
• Scanner carries the previous findings forward when the checker fails, so the rollup issue is not closed and the next scan does not report them as new.
• Unreleased is mergedAt after the newest publishedAt, not a stamp on every merged PR.
• Invalid stable_tag_pattern / source-of-truth regexes are config findings instead of a crash. The dedupe key includes doc_line. Compact YAML sequences (key: then - item in the same column) stay lists. An older stable tag is a non-mechanical mismatch and does not replace the declared version. Dropped the unused newest_blob_cited and the identical tokens.add branches.

Assisted-By: Claude Code

[clawgenti]

Follow-up on c4b55a6: the changelog-history false positive is fixed (basename-based CHANGELOG skip), the dead paths flagged last round are gone, and the dashboard trend reads the new new/fixed deltas — but the ubuntu CI leg is now red: the fixture loader crashes on Python 3.14.

Findings:

• must-fix tests/test-doc-drift-check.sh:388 — ubuntu-latest (Python 3.14) fails in Run test suite: the heredoc loads doc-drift-check.py via exec_module without registering it in sys.modules, and @dataclass then raises AttributeError (details inline; one-line fix).

---

Reviewed by clawgenti using the github-pr-review skill

[jimcadden]

Addressed the Python 3.14 fixture crash in 7f836fd and merged upstream/main in 139cec5.

• The fixture loader registers the checker in sys.modules before exec_module, so @dataclass can resolve the class module.
• tests.yml keeps the doc-drift shellcheck step and tests alongside the attribution-consumer checks from main.

Assisted-By: Cursor

[clawgenti]

Solid addition of the deterministic doc-drift program — the checker/scanner/fixer/precheck are well-factored and fail-safe throughout. Verified locally: full fixture test suite passes, shellcheck is clean on all new scripts, the create_fork_pr/diff_against_previous contracts match program-lib on main, and the registry/dashboard wiring is consistent.

All checks pass. Ready for human review.

---

Reviewed by clawgenti using the github-pr-review skill

[jimcadden]

Reviewed at 7f836fd (10 files, +2976/−11). Verdict: not ready to merge — one systemic precision problem; everything else is polish.

What passes

tests/test-doc-drift-check.sh → ALL PASS, tests/test-doc-drift-precheck.sh → ALL PASS, shellcheck --severity=warning on the three scripts → clean, and the neighbouring suites (-run, -setup, -check, -requirements, -index, automation-health-dashboard, lib-inventory, lib-modules) still pass.

The release logic (mergedAt vs publishedAt, draft vs published, tag ≠ declared version), per-line dedupe keys, compact-YAML parsing and the line-scoped fixer replace are correct and genuinely well tested.

P1 — the checker is ~all false positives on real input

I ran it against rossoctl/automation itself at main (197af28, no .repoman/doc-drift.yaml): 969 findings, 964 corrections, and I could not find a single true one.

• 670 flags — two causes:
  • (a) for a doc that names more than one script, every flag in the whole doc is checked against each named script's parser, so README's --profile / --org / --fork-owner / --flag become "corrections" against github-api.sh, dep-bump-scanner.sh, pr-review-fixer.sh, … — and those flags are consumed by load_org_profile() in program-lib.sh, not by the individual parsers;
  • (b) the intra-script check treats prose words on a # Subcommands: line as subcommands (Each, writes) and any inner case arm as one (central, same, end).
• 299 paths — no basename/relative resolution: 89 of them name a file that does exist in the tree (link-health-fixer.sh → scripts/link-health-fixer.sh). It also captures non-paths: rossoctl/agent-skills, gemini-2.5-flash, discord.accounts, ., ../README.md.

Consequence is operational, not cosmetic: the rollup issue is 900 bullets nobody can action, doc-drift-precheck.sh exits 0 on every repo forever (so every scheduled run burns a model turn), and the dashboard's correction count is pure noise.

Repro:

python3 scripts/doc-drift-check.py --clone <main checkout> \
  --repo rossoctl/automation --releases '[]' --prs '[]' | jq .counts

P1 — ships no documentation

This repo's convention per program is a README Programs row + <program> in the repoman-run.sh --help list at README:53 + standing-orders/<program>.md (docs/program-template.md:405) + a dated design spec in docs/specs/.

The PR adds 2.4k lines and zero .md. The .repoman/doc-drift.yaml schema is documented nowhere, and with the file absent the defaults silently disable sources_of_truth / retired_names, so per-repo behaviour quietly differs.

P2

• The scanner is untested — CI only does bash -n on it. link-health and dep-bump each have index-wiring, subset-carryforward and dry-run-baseline tests; the carry-forward-on-failed-repo and issue open/close/ledger paths are the risky bits and have no coverage.
• The release/PR fetch block is copy-pasted between scanner and precheck and costs 21 gh calls per repo (1 + one gh release view per tag) on a path that runs before every model turn. gh api repos/{o}/{r}/releases returns bodies in one call; the shared helper belongs in program-lib.sh.
• enumerations corrections and paths corrections are mechanical=true with replace_with: "", but the fixer's *) branch prints "no mechanical edit path" — that breaks the meaning the mech_corr partition relies on.
• boundary: main resolves HEAD, not origin/main; the fixer checks branches out in the shared clone, so an interrupted run makes "pinned ref" non-deterministic.
• scripts = {basename: path} silently drops duplicates — repoman has both scripts/x.sh and skills/repoman/scripts/x.sh, so one is checked arbitrarily.

Nits

• .gitignore has no __pycache__/: the test imports the checker via importlib and a local run creates scripts/__pycache__/doc-drift-check.cpython-312.pyc (reproduced) → untracked junk on the next git add -A.
• tag.lstrip("v") / tag.lstrip() — use removeprefix / .strip().
• Fixer replacing a retired name with "" leaves double spaces.
• test-automation-health-dashboard.sh wasn't extended even though the dashboard grew a whole doc-drift section.

[jimcadden]

Re-review at f1b4e06 (7f836fd..f1b4e06, +672/−130). The P1 precision blocker is cleared — this is now a useful tool rather than a noise generator.

Precision, measured on real input

• rossoctl/automation @ main 197af28: 969 findings → 10 (9 corrections, 1 addition). I checked all ten by hand; 9 hold up.
• True ones: scripts/lib/ and four tests/*.sh that no longer exist; set-requirements present in repoman-setup.sh's dispatcher but missing from its Subcommands: header; --kagenti-dir claimed on automation-health-dashboard.sh (no KAGENTI anywhere in scripts/); and the best one — docs/running-without-openclaw.md:187 tells people to run link-health-scanner.sh --profile myorg, which actually exits 1 with Unknown option: --profile. That's a real doc bug, pre-existing, that nothing else in the pipeline would have found.
• The one false positive: dep-bump-scanner.sh accepts --program at docs/specs/2026-09-30-repoman-phase5-index-dashboard-design.md:227. That --program belongs to repoman-index.sh, named on the previous line of the same wrapped bullet.
• jimcadden/repoman @ b3f128c: 3 findings — the same set-requirements gap in the vendored copy (true), plus 2 bundled-SKILL path hits (see item 4).
• Whole-repo scans take 4.3s, so per-line findings are cheap.

Repro:

python3 scripts/doc-drift-check.py --clone <main checkout> \
  --repo rossoctl/automation --releases '[]' --prs '[]' | jq .counts

Docs P1: done properly

README Programs row, the repoman-run.sh --help entry, standing-orders/doc-drift.md, and a dated spec carrying the yaml schema and the precision rules — and the rules it states are actually what the code does (I checked each one against the fixture). Line-scoped and mechanical: false semantics are documented too. .gitignore has __pycache__/ and *.pyc; paths / enumerations corrections are non-mechanical as agreed.

Tests

+120 lines hitting exactly the FP classes from the first review — span binding, prose Subcommands: lines, inner case arms, basename resolution, non-paths. All 35 suites in the CI list pass locally; shellcheck --severity=warning is clean on all three groups (lib modules, repoman tests, doc-drift scripts).

New, small

1. P3 — a flag binds to every mention before it on the line, not the nearest: `scripts/alpha.sh` and `scripts/beta.sh --frobnicate` reports both alpha and beta accepting --frobnicate (reproduced; only the second is a claim). Fix is one line: bind each flag to the greatest mention-end ≤ flag start.
2. P3 — _line_in_span does text.find(span), so a doc with two identical lines yields one finding at the first line and silently drops the second (reproduced). Pair each span with its offset instead.
3. P2 — ignore_paths is only read by check_retired_names and check_versions (check.py:1021, :1169). paths / flags findings cannot be suppressed at all, and this repo's docs/specs/** are historical — the --kagenti-dir finding is un-silenceable without a code change. Honor ignore_paths in those checks too (or add ignore_checks).
4. P3 — path resolution is repo-relative only, so a bundled skill doc misreports its own bundle: skills/repoman/SKILL.md:399 names scripts/org.sh, which exists at skills/repoman/scripts/org.sh. Before giving up on a slash path, try dirname(doc)/raw.
5. Nit — PATH_EXTS silently ignores .csv/.ini/.cfg/.conf/.js/.ts/.jsonl, so those claims are never checked. Defensible, but say so in the spec.

Still open from the first review

The scanner still has no behavioural test — CI runs bash -n on it and nothing else, and carry-forward-on-failure plus the open/close ledger are the risky paths. The release/PR fetch is still copy-pasted between scanner and precheck and still costs 21 gh calls per repo (1 + one gh release view per tag) on the path that runs before every model turn. boundary: main still resolves HEAD (check.py:214) — the standing order now documents that honestly, but the fixer checks branches out in the shared clone, so a "pinned" ref still isn't deterministic.

Verdict

Precision blocker gone, docs and tests in good shape — I'd merge this and take item 3 plus the scanner test as follow-ups rather than hold it.

[clawgenti]

This follow-up binds doc claims to single real files: flags attach only to scripts named earlier on the same line (inheriting accepted flags through program-lib.sh and its modules), path claims require a repo-file extension plus a first directory present in the tree, and subcommand extraction is limited to dispatcher case arms — with new fixture coverage for each rule and the design doc and standing orders recording the semantics. CI is green on all four checks and the checker at head parses cleanly.

All checks pass. Ready for human review.

---

Reviewed by clawgenti using the github-pr-review skill

[jimcadden]

Addressed the five items from the re-review in 97254a5.

1. A flag binds to the nearest script named before it on the same line. `scripts/alpha.sh` and `scripts/beta.sh --frobnicate` is a claim about beta only.
2. Each line span carries its byte offset, so two identical lines stay two findings.
3. ignore_paths now skips path and flag findings as well as version and retired-name findings. The default is still CHANGELOG.md and docs/history/**; docs/specs/** is not ignored unless a repo config says so.
4. A slash path that misses at the repo root is tried relative to the document. A hit there suppresses the finding. skills/repoman/SKILL.md citing scripts/org.sh matches skills/repoman/scripts/org.sh.
5. The spec now says .csv, .ini, .cfg, .conf, .js, .ts, and .jsonl are not path claims.

The wrapped-bullet --program case (the flag sits on the line with dep-bump-scanner.sh, while repoman-index.sh is on the previous line) is unchanged. Nearest-mention only chooses among scripts on the same line.

Still open, as follow-ups: scanner behavioral tests (carry-forward and the issue ledger), the duplicated release/PR fetch, and boundary: main resolving HEAD.

Assisted-By: Cursor

[jimcadden]

Re-review at 97254a5 (f1b4e06..97254a5, 4 files, +112/−59). All four items from the last review are fixed and verified end-to-end, not just by unit test.

Verified

1. Nearest script before the flag. `scripts/alpha.sh` and `scripts/beta.sh --frobnicate` now reports beta only; alpha is clean. The new fixture asserts both halves of that.
2. Duplicate lines. _doc_spans returns (offset, line) and _line_at counts from the offset, so two identical lines are two findings with their own line numbers — the test now asserts exactly 2 distinct doc_line values for the repeated beta.sh --dry-run line.
3. ignore_paths reaches paths and flags. Verified against a real repo, not a fixture: committed .repoman/doc-drift.yaml with ignore_paths: ["skills/**"] to jimcadden/repoman takes that scan from 2 findings to 0.
4. Bundle-relative path fallback. jimcadden/repoman scan drops 3 → 2; skills/repoman/SKILL.md:399 citing scripts/org.sh for the copy at skills/repoman/scripts/org.sh is correctly suppressed, and a genuine miss in the same doc is still reported.
5. PATH_EXTS blind spots and the ignore_paths scope are now stated in the spec, which matches the code.

Precision, current

Repo @ ref	Findings	True
rossoctl/automation @ 197af28	10	9
jimcadden/repoman @ b3f128c	2	1

All 35 suites in the CI list pass locally; shellcheck --severity=warning is clean on the doc-drift scripts and their tests.

The one remaining false positive

dep-bump-scanner.sh accepts --program at docs/specs/2026-09-30-repoman-phase5-index-dashboard-design.md:227 is unchanged, and the nearest-mention rule cannot catch it by construction: that bullet names only dep-bump-scanner.sh and elides the real command with "same shape with". Two ways out, both cheap — name repoman-index.sh on that line (the claim becomes checkable, which is the point of the check), or add docs/specs/** to ignore_paths, which now covers flag findings.

For the repoman hit, skills/repoman/SKILL.md:405 cites tests/test-repoman-match-name.sh, which lives in this repo and the doc says so explicitly — that is what ignore_paths is for.

Still open from the first review

The scanner has no behavioural test (bash -n only in CI); carry-forward-on-failure and the issue ledger are the risky paths. The release/PR fetch is still duplicated between scanner and precheck and still costs 21 gh calls per repo (1 + one gh release view per tag) on the path that runs before every model turn. boundary: main still resolves HEAD (check.py:214).

Verdict

Precision is where it needs to be, the rules are documented as implemented, and every rule has a test. I'd merge this and take the scanner test plus the fetch de-duplication as separate PRs.

[rubambiza]

Independent review at 97254a5. I re-verified the key claims from a clean checkout rather than relying on the prior review loop:

• Both suites pass clean (test-doc-drift-check.sh 26/26, test-doc-drift-precheck.sh all pass), shellcheck --severity=warning clean on all three scripts.
• Reproduced the checker against this repo at main → 10 findings, 9 true (the one FP is --program at docs/specs/2026-09-30-...-design.md:227, where the nearest-mention rule cannot catch it by construction).
• ignore_paths now reaches check_paths and check_flags — confirmed.
• Security surface is read-only: all subprocess use is list-form git, no shell=True/eval/os.system/network in the Python, output only to stdout; the fixer stays on fork branches with --force-with-lease and never pushes the default branch.

The test (macos-latest) red is a runner-capacity infra failure ("the job was not acquired by Runner of type hosted"), not a test failure — ubuntu, pr-title, and DCO all pass.

Author: jimcadden (MEMBER — maintainer)
Areas reviewed: Python, Shell, CI, Docs
Agent/IDE config (.claude/.vscode): none
Commits: 8, all signed-off: yes
CI status: ubuntu / pr-title / DCO passing; macOS failed on runner capacity (infra, not code)

Two non-blocking notes inline: the checker is a strong candidate for the same module split program-lib.sh got (follow-up), and one test fixture is fragile under tag.gpgsign. Approving.

[rubambiza]

suggestion: At 1636 LOC this exceeds the per-module bar the program-lib.sh decomposition set for this repo (~185-278 LOC, single-purpose, self-contained). The file already has clean cohesion seams — git helpers, the YAML-subset config reader, the Finding model, code-claim checks (paths/flags), metadata checks (enum/retired/versions/absence), release/changelog, and orchestration — that would split into importable units of roughly 300 LOC or less (the paths+flags cluster would itself split to get there).

One real constraint the shell refactor did not face: the tests load this file via importlib.util.spec_from_file_location and scripts/ is not a Python package, so sibling imports would need a package layout or sys.path handling, and agent-skills vendors scripts as a flat per-skill directory. That makes the split a deliberate refactor with tradeoffs rather than a mechanical move. Given that and the long review loop already behind this PR, I would take it as a dedicated follow-up rather than hold the merge. Not blocking.

[rubambiza]

nit: git tag v1.0.0 is a lightweight tag with no -m. For a contributor whose global config sets tag.gpgsign=true (common), this fixture dies with fatal: no tag message? and aborts the whole suite locally — even though CI is green, because CI has no such config. The fixtures already isolate identity with -c user.email/-c user.name; adding -c tag.gpgsign=false to the tag call(s) closes the same gap. Non-blocking.