Repository navigation
feat: Add doc-drift program (scanner, fixer, release assist) - #12
Conversation
|
Considering this PR vendors, we probably should bump up the version -> |
clawgenti
left a comment
clawgenti
left a comment
There was a problem hiding this comment.
Summary
This vendors the doc-drift engine (checker, scanner, fixer, precheck) and wires it into the marketplace, the dashboard, repoman-run and repoman-setup. Supply-chain checks pass: there are no .claude//.vscode/ changes, the new workflow pins actions/checkout to a SHA, the checker makes no network calls and runs nothing from scanned repos, and every vendored file matches rossoctl/automation@ecdc685 byte-for-byte. The blocking problems are the fixer's live mode, which rewrites version strings repo-wide (changelog history and dependency pins included), plus scanner/fixer state bugs that would wrongly close rollup issues and leave shared clones stuck mid-rebase. The vendor stamp also points at an unmerged fork commit.
Author: jimcadden (NONE — first-time)
Areas reviewed: Shell, Python, CI/GitHub Actions, Docs, plugin marketplace JSON
Agent/IDE config (.claude/.vscode): none (.claude-plugin/marketplace.json is plugin metadata only)
Commits: 1 commit, all signed-off: yes
CI status: DCO passing (the new shellcheck workflow has not run yet)
Provenance: _vendor.json stamps source_repo: rossoctl/automation @ ecdc685, but that SHA is the head of jimcadden/automation for the still-open rossoctl/automation#124 (1 commit ahead of automation main). Please land #124 upstream first, then re-vendor from the merged SHA, so the stamp records reviewed upstream code.
The test plan checkboxes are all unchecked.
| while IFS= read -r f; do | ||
| [ -n "$f" ] || continue | ||
| apply_replace "$f" "$old_tok" "$replace" && git add "$f" || true | ||
| done < <(git grep -l -I "$old_tok" -- '*.md' '*.rst' '*.txt' 2>/dev/null || true) |
There was a problem hiding this comment.
must-fix: Combined with the checker's versions check, this rewrites the repo wholesale. old_tok is the doc's version, and git grep -l plus apply_replace then replaces every occurrence of it in every .md, .rst and .txt file, including changelog history and unrelated dependency versions. It is also a substring match (1.2.3 matches inside 11.2.34), and apply_replace falls back to a case-insensitive re.subn. A mechanical fix should touch only the flagged doc_path:doc_line occurrence.
| repos_with_findings=$(jq -r '.repo' "$ENRICHED" 2>/dev/null | sort -u) | ||
| # Also repos that had findings before but none now (from fixed keys) | ||
| repos_prev=$(jq -r '.repo // empty' "$PREV" 2>/dev/null | sort -u) | ||
| repos_all=$(printf '%s\n%s\n' "$repos_with_findings" "$repos_prev" | awk 'NF && !seen[$0]++') |
There was a problem hiding this comment.
must-fix: If the checker fails for a repo (L103-107: crash, non-UTF-8 doc, invalid regex in .repoman/doc-drift.yaml, missing clone), that repo has no entries in current.jsonl but is still in repos_prev. The loop sees corr=0 adds=0 and closes its rollup issue with "no open corrections remain", and the ledger marks every previous finding obsolete/fixed. Failed repos should be skipped here and their previous findings carried forward. Also, latest.json is written with the failed repos' findings missing, so the next scan reports them all as new.
|
|
||
| gh pr list --repo "$full_repo" --state merged --limit 100 \ | ||
| --json number,title,url,labels,mergedAt 2>/dev/null \ | ||
| | jq '[.[] | . + {unreleased: true}]' > "$prs_json" 2>/dev/null \ |
There was a problem hiding this comment.
suggestion: The last 100 merged PRs are all tagged unreleased: true, regardless of the newest release tag (the precheck has the same code and a comment saying "after the newest stable tag"). Release-draft and coverage findings will therefore list already-released PRs. Filter by mergedAt against the newest stable release's publishedAt. This logic is also triplicated across the scanner, the precheck and the fixer (L83-86), and could live in one helper.
| "source_repo": "rossoctl/automation", | ||
| "source_sha": "0311752eba5350c74b8c9650137d3e6da65bd547", | ||
| "vendored_at": "2026-10-23T03:46:40Z", | ||
| "source_sha": "ecdc68577aa66e7959a4741b45ada741eac6cba2", |
There was a problem hiding this comment.
suggestion: ecdc685 is not on rossoctl/automation main. It is the head of the open, unmerged rossoctl/automation#124 on jimcadden/automation. Please re-vendor from the merged upstream SHA so the provenance stamp is accurate.
clawgenti
left a comment
clawgenti
left a comment
There was a problem hiding this comment.
Summary
Re-review covering 591480d..ac06326. The new commit re-vendors automation@fbf8f7b: a null-safe dashboard Delta, structural YAML list detection, and multi-line Subcommands: bullets. None of the four must-fix items from the previous review are addressed. All four are unchanged at head:
doc-drift-check.pyL815:.upper() == "CHANGELOG.md"can never match, and everyX.Y.Zis still treated as a mechanical correction.doc-drift-fixer.shL236: repo-wide substringgit grep/ replace of the old version.doc-drift-fixer.shL320:git rebase ... || truecan leave the shared clone mid-rebase, and the plain push at L389 fails as non-fast-forward.doc-drift-scanner.shL155: repos whose checker run failed still sit inrepos_prev, so their rollup issues get closed.
The suggestions about the unreleased-PR filter, the unguarded re.search on repo-supplied patterns, and the dedupe key are also still open. The new commit adds one regression in the YAML parser (inline below).
Provenance: _vendor.json now stamps fbf8f7b, but rossoctl/automation#124 is still OPEN and fbf8f7b is 2 commits ahead of automation main. The stamp still records unmerged code. Please merge #124 first and re-vendor from the merge SHA.
Version bump: agreed with the author's comment. marketplace.json metadata.version is still 0.1.0.
Author: jimcadden (NONE — first-time)
Areas reviewed: Python, Shell, Docs, plugin marketplace JSON (incremental diff plus a re-check of prior findings at head)
Agent/IDE config (.claude/.vscode): none
Commits: 2 commits, all signed-off: yes
CI status: DCO passing (the shellcheck workflow has not reported)
clawgenti
left a comment
clawgenti
left a comment
There was a problem hiding this comment.
Summary
No change since the previous review: head is still ac06326, with no new commits and no replies on the open threads. I re-checked the blocking items at head and all five are still present:
doc-drift-check.pyL815: the.upper() == "CHANGELOG.md"comparison can never match, so changelog history versions are still emitted as mechanical corrections.doc-drift-check.pyL257: the compact YAML sequence form (- itemat the same indent as its key) is still rejected.doc-drift-fixer.shL236: still a repo-wide substringgit grepand replace of the old version token.doc-drift-fixer.shL320:git rebase ... || truecan still leave the shared clone mid-rebase.doc-drift-scanner.shL155: repos whose checker run failed still sit inrepos_prev, so their rollup issues get closed.
Provenance: rossoctl/automation#124 is still open and unmerged, so _vendor.json still stamps unmerged code. Please merge it upstream first and re-vendor from the merge SHA.
The detail is in the inline threads from the two earlier reviews and is not repeated here. This review also records the reviewed SHA so automation does not re-queue this PR until new commits land.
Author: jimcadden (NONE — first-time)
Areas reviewed: Python, Shell (re-verification of prior must-fix items at head)
Agent/IDE config (.claude/.vscode): none
Commits: 2 commits, all signed-off: yes
CI status: DCO passing (the shellcheck workflow has not reported)
clawgenti
left a comment
clawgenti
left a comment
There was a problem hiding this comment.
Summary
Re-review covering ac06326..1a9498d ("dead-code cleanup", re-vendors automation@c4b55a6). The cleanup is correct: it removes the unreachable {-prefixed branch in parse_simple_yaml, simplifies the check_paths existence test with equivalent semantics, and drops the dead enumerations|paths fixer case. One behaviour change rides along: the stable tag now overrides the declared version (inline).
None of the blocking items from earlier reviews are addressed. All five are still present at head:
doc-drift-check.pyL809:.upper() == "CHANGELOG.md"can never match, so changelog history versions are still emitted as mechanical corrections.doc-drift-check.pyL254: the compact YAML sequence form (- itemat the same indent as its key) is still rejected.doc-drift-fixer.shL236: still a repo-wide substringgit grepand replace of the old version token.doc-drift-fixer.shL305:git rebase ... || truecan still leave the shared clone mid-rebase.doc-drift-scanner.shL155: repos whose checker run failed still sit inrepos_prev, so their rollup issues get closed.
Provenance: rossoctl/automation#124 is still OPEN, so c4b55a6 is unmerged code. Please merge it upstream and re-vendor from the merge SHA.
Author: jimcadden (NONE — first-time)
Areas reviewed: Python, Shell, vendor manifest (incremental diff plus a re-check of prior must-fix items at head)
Agent/IDE config (.claude/.vscode): none
Commits: 3 commits, all signed-off: yes
CI status: DCO passing (the shellcheck workflow has not reported)
|
The must-fixes on the vendored checker, fixer, and scanner are the same bugs as on rossoctl/automation#124. They are fixed there in b95f446 (changelog case, line-scoped replace, rebase abort, failed-scan carry-forward, compact YAML lists, unreleased-by-date). This PR will re-vendor from the merged automation SHA, not from the open branch, so the Assisted-By: Claude Code |
1a9498d to
afdd058
Compare
clawgenti
left a comment
clawgenti
left a comment
There was a problem hiding this comment.
Summary
Re-review at afdd058. This is a force-push of 1a9498d with a byte-identical tree (git diff 1a9498d afdd058 is empty). The only change is to the commit messages: the Assisted-By: trailers were removed, and DCO sign-off is still present on all three commits. No code changed, so none of the blocking items are addressed yet:
doc-drift-check.pyL809: the.upper() == "CHANGELOG.md"comparison can never match.doc-drift-check.pyL254: the compact YAML sequence form is still rejected.doc-drift-fixer.shL236: still a repo-wide substring replace of the old version token.doc-drift-fixer.shL305:git rebase ... || truecan still leave the shared clone mid-rebase.doc-drift-scanner.shL155: failed-scan repos still get their rollup issues closed.
The plan in the author's comment sounds right: fix upstream in rossoctl/automation#124 and re-vendor from the merge SHA. The upstream commit message (b95f446, now f668ded on the #124 branch) describes fixes for all five. #124 is still OPEN, though, so nothing on this branch can be verified yet. I'll re-review the vendored code once it lands here.
Author: jimcadden (NONE — first-time)
Areas reviewed: tree/commit comparison against the previously reviewed head
Agent/IDE config (.claude/.vscode): none
Commits: 3 commits, all signed-off: yes
CI status: DCO passing (the shellcheck workflow has not reported)
afdd058 to
cf1a96c
Compare
clawgenti
left a comment
clawgenti
left a comment
There was a problem hiding this comment.
Summary
Re-review at cf1a96c. The tree is byte-identical to the previously reviewed afdd058 (identical tree SHA 111a1cf; git diff empty), so this is another force-push with no code change. None of the blocking items are fixed at head — all five remain, and the vendor stamp still points at unmerged upstream code.
Author: jimcadden (NONE — first-time)
Areas reviewed: Python, Shell, CI/GitHub Actions, Docs, plugin marketplace JSON, vendor manifest
Agent/IDE config (.claude/.vscode): none (.claude-plugin/marketplace.json is plugin metadata only)
Commits: 3 commits, all signed-off: yes
CI status: DCO passing (the new shellcheck workflow has not reported)
Provenance: _vendor.json stamps source_sha: c4b55a6 — rossoctl/automation#124 is still OPEN (head f668ded), and c4b55a6 is not an ancestor of that head (branches diverged). Please merge #124 and re-vendor from the merged main SHA, so the stamp records reviewed upstream code.
The latest upstream commit on #124 (f668ded fix: Stop doc-drift from rewriting changelog history) suggests the changelog item is being fixed there — but none of those fixes are vendored in this PR yet.
jimcadden
left a comment
jimcadden
left a comment
There was a problem hiding this comment.
GitHub will not accept REQUEST_CHANGES on the author's own pull request, so this is filed as a comment. The verdict is request changes.
Vendored doc-drift scripts are behind the engine PR they snapshot, and the fixer in this copy still replaces a token across every markdown file. The precheck's default worklist is deleted on exit; that bug is also on the current automation HEAD, so a plain re-vendor will not fix it. Skill wiring, the marketplace entry, the vendor manifest counts, and the pinned checkout SHA match the rest of the plugin.
Author: jimcadden (MEMBER — maintainer)
Areas reviewed: Shell, Python (checker compared to automation HEAD), CI/GitHub Actions, Docs
Agent/IDE config (.claude/.vscode): none
Commits: 3 commits, all signed-off: yes
CI status: DCO passing. No Actions runs on cf1a96c (base has PR Verifier; shellcheck is new in this PR), so the title check and the new workflow are unverified here.
feat: Add Active Epics section to weekly org report All AI and human comments have been addressed.
cf1a96c to
2dff532
Compare
jimcadden
left a comment
jimcadden
left a comment
There was a problem hiding this comment.
Summary
The wiring is complete and the vendoring mechanics are correct, but the vendored engine is a pre-merge snapshot of rossoctl/automation#124 that predates the fixes which shipped in the merge. #124 merged at 2026-11-03T21:03Z as fa61909 (also main HEAD); the stamp here records c4b55a6, which is not an ancestor of main (git merge-base --is-ancestor c4b55a6 origin/main fails). Re-vendoring from fa61909 is a single command and closes every must-fix below plus the stamp provenance:
bash scripts/vendor-engine.sh --source <automation-clone> --dest skills/repoman/scriptsDelta c4b55a6..fa61909 over the 27-file manifest: 8 files, +683/−198 (doc-drift-check.py, doc-drift-fixer.sh, doc-drift-scanner.sh, doc-drift-precheck.sh, github-api.sh, pr-review-fixer.sh, pr-review-scanner.sh, weekly-report.sh). It also pulls the #127 REPORT_PY candidate-list fix — the copy vendored here still hardcodes the OpenClaw path at skills/repoman/scripts/weekly-report.sh:79, so the known-broken default is re-shipped by this PR.
The six defects below were raised against earlier heads (clawgenti's 2026-10-31 review, and the 2026-11-02 comment on this PR) and are still present verbatim at 2dff532; the head was force-pushed after those reviews without the re-vendor.
Verified working (executed, not just read):
- all 27 vendored files byte-identical to
automation@c4b55a6— vendoring is faithful, nothing hand-edited; bash tests/test-vendor-engine.sh→ 9 PASS, exit 0 (26 shell scripts +.py, stamp lists 27 files);--helpexits 0 on scanner, fixer, precheck anddoc-drift-check.py;- end-to-end on a fixture repo:
repoman-run.sh doc-drift --dry-run→ clone refresh, capability gate, 2 findings, exit 0;doc-drift-precheck.sh→worklist: …, exit 0; enable-program/set-output --program doc-driftaccepted;repoman-check.sh --program doc-driftpasses;- dashboard renders the new Doc Drift section (fabricated latest/history → cumulative 3 created / 1 closed, trend Δ +4 / −2,
repos_scanned4); marketplace.jsonand_vendor.jsonparse; requirements blocks match the scanner/fixer schemarepoman-requirements.shconsumes;doc-driftpresent inKNOWN_PROGRAMS, the run-path table, bothskills/repoman/SKILL.mdtables and the README;.claude//.vscode/gate: no matches (.claude-plugin/marketplace.jsonis plugin metadata only — no hooks, no commands);- workflow: top-level
permissions: {}, jobcontents: read,actions/checkoutpinned to a SHA; - shellcheck (CI parity,
--severity=warning) clean on the two files the workflow lints; - DCO 3/3 signed off; CI green at
2dff532.
Author: jimcadden (MEMBER — maintainer)
Areas reviewed: Shell, Python, CI/GitHub Actions, Docs, plugin marketplace JSON, vendor manifest
Agent/IDE config (.claude/.vscode): none
Commits: 3 commits, all signed-off: yes
CI status: passing (DCO, shellcheck, pr-title)
must-fix
All six are fixed in fa61909; the remedy is the single re-vendor above.
_vendor.json:3— stamp namesc4b55a6, a pre-merge branch commit. Provenance must point at merged upstream code.doc-drift-check.py:809—.upper()turns the basename intoCHANGELOG.MD, which can never equal the literal"CHANGELOG.md": the changelog is never skipped, so every historicalX.Y.Zin it is emitted as a mechanical version correction.doc-drift-check.py:254— compact YAML sequences (key:then- itemat the same indent) are dropped, because the list-vs-map decision requiresnxt_indent > indent. Merged version uses>=.doc-drift-fixer.sh:236— live mode rewrites the version token across every*.md/*.rst/*.txtin the clone, unscoped by the finding: bumping1.2.3also rewrites1.2.30, changelog history and dependency pins. Merged version replaces onlydoc_pathatdoc_line.doc-drift-fixer.sh:305—git rebase … || trueswallows the failure, leaving the shared clone mid-rebase; the later plaingit pushis then rejected as non-fast-forward. Merged version aborts the rebase and detects an existing rebase state.doc-drift-scanner.sh:90and:155— every merged PR is stampedunreleased: true(release drafts would list already-shipped work), andrepos_allunionsrepos_preveven for repos whose checker run failed, so a broken run closes their rollup issue. Merged version classifies unreleased frommergedAtvspublishedAtand carries previous findings forward for failed repos.
nits
skills/repoman/scripts/repoman-setup.sh:321-324—cmd_enable_program's--helpstill lists the six pre-doc-driftprograms (and omitsweekly-report), contradicting the allowlist the same file enforces; the top-level usage was updated, this copy was not.skills/repoman/SKILL.md:169— the "authoritative backend names" sentence still readslink-health, dep-bump, pr-review, automation-health, repo-sync(missingweekly-reportanddoc-drift). This is the list the entrypoint is told to resolve against.skills/automation-health-dashboard/scripts/automation-health-dashboard.shwas byte-identical to the bundle copy at base and was not updated, so the standaloneautomation-healthplugin now diverges and skipsdoc-driftsections;skills/automation-health-dashboard/SKILL.md:100("a registry entry for any other id is skipped with a note") is now false for the bundled copy. The other standalone twins already diverge at base, so this may be deliberate — if so, the docs should say which copy is canonical.- The dashboard's cross-program coverage table and cron-health table still hardcode link-health/dep-bump (upstream code, vendored verbatim — fix belongs in
rossoctl/automation, not here). .gitignoregains.cursor/andautomation-work/— author-tool noise for a doc-drift feature, against CLAUDE.md's scope discipline.- PR body leaves the test-plan boxes unticked and carries a
Made with [Cursor]line next toAssisted-By: Claude Code.
Verdict
REQUEST_CHANGES — re-vendor from merged rossoctl/automation (fa61909) and the PR is mergeable as-is; nothing in the RepoMan-side wiring needs to change. Filed as a comment because GitHub rejects REQUEST_CHANGES on the author's own PR.
Generated by RepoMan github-pr-review@235a7f7 (https://github.com/rossoctl/agent-skills/blob/235a7f7620139ee3e4ab0872b8439591fe52e8c1/skills/github-pr-review/SKILL.md).
jimcadden
left a comment
jimcadden
left a comment
There was a problem hiding this comment.
Summary
Re-review at e37cb75 (657d4f2 re-vendor from fa61909, e37cb75 entrypoint allowlist). All six must-fixes and the provenance finding from the previous review are resolved, and the re-vendor is faithful and reproducible. Nothing blocking remains.
Verified by executing, not reading:
- Fidelity: all 27 files in the manifest are byte-identical to
rossoctl/automationmainHEADfa61909. Re-running the tool from a cleanfa61909checkout (vendor-engine.sh --source … --dest /tmp/…) reproduces the committed tree exactly — the only difference isvendored_at, and the stamp (repo, sha, file list) matches. So the tree is the tool's output, not a hand-copy. - Completeness: no engine
.shpresent atfa61909is missing from the manifest (comm -23overscripts/*.shis empty). - Changelog skip now observable: fixture with
CHANGELOG.mdholding1.0.0/0.9.0history plus a staledocs/usage.md→ exactly one finding (versions docs/usage.md:3), zero findings whosedoc_pathisCHANGELOG.md. The superseded copy emitted the historical headings as corrections. - Other five:
nxt_indent >= indent(check.py:301); version fixes go throughreplace_on_linescoped todoc_path/doc_line; rebase is guarded (if ! git rebase …+rebase --abort+ mid-rebase detection); nounreleased: truestamping left in scanner or precheck;failed_repos.txtcarries previous findings forward for failed repos. - Integration still works after the engine jump (checker output is now an object, not a bare array):
repoman-run.sh doc-drift --dry-runend-to-end → clone refresh, gate, 2 findings, exit 0; scanner dry-run,doc-drift-precheck.shworklist, and all four--helps exit 0; dashboard renders the Doc Drift section with a correct trend table. - Hygiene:
bash tests/test-vendor-engine.sh9 PASS; shellcheck--severity=warningclean on the workflow's two files and on the vendored doc-drift/weekly-report/pr-review-fixer/github-apiscripts;marketplace.jsonand_vendor.jsonparse;py_compileclean; no.claude//.vscode/paths; DCO 5/5; CI 3/3 green. - Bonus from the re-vendor:
weekly-report.shnow resolvesREPORT_PYfrom a candidate list, and in this layout candidate 1 (skills/github-weekly-report/scripts/report.py) exists — the bundle no longer needsREPORT_PYexported to run the weekly report.
Author: jimcadden (MEMBER — maintainer)
Areas reviewed: Shell, Python, CI/GitHub Actions, Docs, plugin marketplace JSON, vendor manifest
Agent/IDE config (.claude/.vscode): none
Commits: 5 commits, all signed-off: yes
CI status: passing (DCO, shellcheck, pr-title)
nits
repoman-setup.sh:321-324—cmd_enable_program's own--helpstill lists the six pre-doc-driftprograms while the allowlist two screens up acceptsdoc-drift. This text is upstream (rossoctl/automation), so it will come back on every re-vendor: fix it there, then the next vendor run carries it.skills/repoman/SKILL.md:169—doc-driftwas added here butweekly-reportis still missing from the same list (pre-existing, one word, now that the line is being touched).skills/automation-health-dashboard/scripts/automation-health-dashboard.shis still the pre-doc-driftcopy, so the standaloneautomation-healthplugin skips Doc Drift sections that the bundled copy renders. Pre-existing pattern for the other standalone twins — worth a sentence in the docs saying the bundle copy is canonical, if that is the intent.
Verdict
APPROVE. Filed as a comment because GitHub rejects an approval from the author's own account; if branch protection needs a recorded approving review, another maintainer (or clawgenti's next pass) has to submit it.
Generated by RepoMan github-pr-review@235a7f7 (https://github.com/rossoctl/agent-skills/blob/235a7f7620139ee3e4ab0872b8439591fe52e8c1/skills/github-pr-review/SKILL.md).
clawgenti
left a comment
clawgenti
left a comment
There was a problem hiding this comment.
Summary
Independent review at e37cb75. The re-vendor is faithful and reproducible: all 27 manifest files are byte-identical to rossoctl/automation@fa61909, that SHA is an ancestor of automation main, _vendor.json stamps it, bash tests/test-vendor-engine.sh passes 9/9, and the previously blocking engine defects are fixed at head (changelog skip is now basename(...).lower(), YAML uses nxt_indent >= indent, version fixes go through replace_on_line on doc_path:doc_line, rebases are guarded with rebase --abort, and failed-scan repos carry previous findings forward). One documented feature is still broken in the vendored copy: the precheck's default worklist path lives in a temp dir that is deleted on exit, so the documented invocation prints a path that no longer exists.
Author: jimcadden (CONTRIBUTOR — returning external)
Areas reviewed: Shell, Python, CI/GitHub Actions, Docs, plugin marketplace JSON, vendor manifest
Agent/IDE config (.claude/.vscode): none (.claude-plugin/marketplace.json is plugin metadata only)
Commits: 5 commits, all signed-off: yes (5/5)
CI status: passing (DCO, shellcheck, pr-title)
Provenance verified: _vendor.json -> fa61909029207b08d17eff5625bd0aa4772c90b2; git merge-base --is-ancestor against automation main holds; every manifest file hashes identically to upstream; re-running the vendoring path reproduces the committed tree (only vendored_at differs). No engine .sh present at fa61909 is missing from the manifest.
must-fix
skills/repoman/scripts/doc-drift-precheck.sh:43— the default worklist is deleted before the caller can read it.setup_workspacecreates$PROGRAM_TMPDIRunder/tmpand registerstrap 'rm -rf "$PROGRAM_TMPDIR"' EXIT(core.sh:27-29). Line 43 defaultsWORKLISTto$PROGRAM_TMPDIR/worklist.json, and the success path printsworklist: <path>— after which the EXIT trap removes that directory. The documented invocation inskills/doc-drift-scanner/SKILL.md:83-87therefore hands the caller a dead path. Confirmed empirically (sourcecore.sh,setup_workspace, write$PROGRAM_TMPDIR/worklist.json, exit -> directory gone). Remedy, either: fix upstream inrossoctl/automationand re-vendor (default to a persistent path such as$REPORTS_DIR/worklist.json), or make--worklist <persistent path>a required part of the documented contract.
nits
skills/repoman/scripts/repoman-setup.sh:321-324—cmd_enable_program's--helpstill lists only six programs and omitsdoc-drift(andweekly-report), contradicting theKNOWN_PROGRAMSallowlist at line 34 that now acceptsdoc-drift. Upstream text, so fix it inrossoctl/automationor it returns on every re-vendor.skills/repoman/SKILL.md:169— the "authoritative backend names" line now listsdoc-driftbut still omitsweekly-report; the two lists should matchKNOWN_PROGRAMS.skills/automation-health-dashboard/scripts/automation-health-dashboard.sh— the standalone plugin's copy carries nodoc-driftextraction block while the bundled copy does, so the standaloneautomation-healthplugin silently skips Doc Drift sections. Pre-existing twin divergence; worth stating which copy is canonical..gitignore:8-9—automation-work/and.cursor/are author-tooling artifacts unrelated to doc-drift, against CLAUDE.md's scope discipline.- PR body: test-plan checkboxes unticked, and
Made with [Cursor]sits besideAssisted-By: Claude Code.
Verdict
REQUEST_CHANGES — one must-fix in the vendored precheck; the RepoMan-side wiring itself needs no changes.
|
|
||
| setup_workspace "doc-drift-precheck" | ||
| if [ -z "$WORKLIST" ]; then | ||
| WORKLIST="$PROGRAM_TMPDIR/worklist.json" |
There was a problem hiding this comment.
must-fix — this default is deleted before the caller can use it. setup_workspace (core.sh:24-30) makes $PROGRAM_TMPDIR under /tmp and registers trap 'rm -rf "$PROGRAM_TMPDIR"' EXIT, so the worklist: <path> printed on the success path points into a directory that no longer exists once the script exits. A DAM schedule running the documented bare command gets a dead path. Default to a persistent location (e.g. $REPORTS_DIR/worklist.json) or require --worklist <persistent path>. Inherited from automation@fa61909, so the clean fix is upstream + re-vendor.
| gate uses; use the program→skill-dir map above). The authoritative backend | ||
| names remain the `KNOWN_PROGRAMS` allowlist in `repoman-setup.sh` | ||
| (`link-health`, `dep-bump`, `pr-review`, `automation-health`, `repo-sync`) — | ||
| (`link-health`, `dep-bump`, `pr-review`, `automation-health`, `repo-sync`, `doc-drift`) — |
There was a problem hiding this comment.
nit — this list now has doc-drift but still omits weekly-report, so it does not match KNOWN_PROGRAMS in repoman-setup.sh:34. Add weekly-report while the line is being touched.
| skills/*/scripts/_meta.*.json | ||
| skills/*/scripts/_meta.*.json.tmp.* | ||
| automation-work/ | ||
| .cursor/ |
There was a problem hiding this comment.
nit — .cursor/ and automation-work/ are author-tooling artifacts unrelated to the doc-drift feature; CLAUDE.md scope discipline asks that unrelated config changes go in their own PR.
|
Addressed the review at
Assisted-By: Claude Code |
jimcadden
left a comment
jimcadden
left a comment
There was a problem hiding this comment.
Summary
Re-review at 84ac95e (docs: Require a persistent doc-drift precheck worklist). Three files changed since the approving review at e37cb75, all documentation/gitignore; the vendored engine is untouched. No blocking issues — verdict stays APPROVE.
Delta e37cb75..84ac95e:
.gitignore— theautomation-work/+.cursor/entries are gone, so the PR no longer carries author-tool noise (and no longer touches.gitignoreat all).skills/repoman/SKILL.md:169— the "authoritative backend names" list now namesweekly-reportas well asdoc-drift.skills/doc-drift-scanner/SKILL.md— the precheck section now requires an explicit persistent--worklist.
Verified by running:
- The worklist trap is real and the new text describes it correctly:
setup_workspaceputsPROGRAM_TMPDIRunder atrap 'rm -rf "$PROGRAM_TMPDIR"' EXIT, so a bare precheck printsworklist: /tmp/doc-drift-precheck-XXXXXX/worklist.jsonand that file is gone by the time the caller reads it (confirmed: path printed,test -ffails after exit). With--worklist "$REPORTS_DIR/worklist.json"the file survives with the expected shape ({"count":2,"findings":[…2…]}), and the documented snippet runs as written. - Vendored tree unchanged this round: 27/27 files still byte-identical to
automation@fa61909. bash tests/test-vendor-engine.sh9 PASS; DCO 6/6; CI 3/3 green at84ac95e; no.claude//.vscode/paths; working tree clean.
Author: jimcadden (MEMBER — maintainer)
Areas reviewed: Shell, Python, Docs, CI, plugin marketplace JSON, vendor manifest
Agent/IDE config (.claude/.vscode): none
Commits: 6 commits, all signed-off: yes
CI status: passing (DCO, shellcheck, pr-title)
nits (non-blocking)
skills/doc-drift-scanner/SKILL.md:82— "required" is a rule for callers, not something the CLI enforces:--worklistremains optional and silently defaults to the doomed temp path. The durable fix is upstream indoc-drift-precheck.sh(default to$REPORTS_DIR, or exit non-zero rather than print a path it is about to delete); until then this doc line is the right guardrail.- Still open from earlier rounds, both pre-existing / upstream:
repoman-setup.sh:321-324enable-program --helplists six programs (fix inrossoctl/automation, otherwise every re-vendor reintroduces it), and the standaloneskills/automation-health-dashboard/copy is still pre-doc-drift.
Verdict
APPROVE. Filed as a comment because GitHub rejects an approval from the author's own account; branch protection still needs an approving review from another account.
Generated by RepoMan github-pr-review@235a7f7 (https://github.com/rossoctl/agent-skills/blob/235a7f7620139ee3e4ab0872b8439591fe52e8c1/skills/github-pr-review/SKILL.md).
|
|
||
| ## Precheck (DAM schedules) | ||
|
|
||
| `--worklist` is required. Pass a persistent path. The script's default path |
There was a problem hiding this comment.
nit: "required" is accurate as a rule for callers, but nothing enforces it — the flag is optional and silently falls back to $PROGRAM_TMPDIR/worklist.json, which the EXIT trap in setup_workspace deletes. Verified: a bare run prints worklist: /tmp/doc-drift-precheck-XXXXXX/worklist.json, and that file is gone once the script returns; with --worklist "$REPORTS_DIR/worklist.json" the file survives with {count, findings[]}.
So this doc is the right guardrail for now. The durable fix is upstream in doc-drift-precheck.sh: default the worklist to $REPORTS_DIR (or exit non-zero instead of printing a path it is about to delete), otherwise the next caller that omits the flag hits the same silent dead end.
clawgenti
left a comment
clawgenti
left a comment
There was a problem hiding this comment.
Summary
Re-review at 84ac95e for this RepoMan run. The earlier review at this head (posted from the PR author's own account) carries no <!-- reviewed: SHA --> marker, so the scanner re-queued the PR.
Nothing has changed since that review — head is still 84ac95e. Re-confirmed this round:
- 27/27 vendored files byte-identical to
automation@fa61909029207b08d17eff5625bd0aa4772c90b2;_vendor.jsonfile list matches the directory exactly. - DCO 6/6, CI green (DCO, shellcheck, pr-title) at
84ac95e. - No
.claude/or.vscode/paths anywhere in the diff. - Prior must-fix (dead temp worklist path) resolved via the documented persistent
--worklistcontract.
Author: jimcadden (CONTRIBUTOR — returning external)
Areas reviewed: Shell, Python, YAML/CI, plugin marketplace JSON, docs
Agent/IDE config (.claude/.vscode): none
Commits: 6 commits, all signed-off: yes
CI status: passing
Raised this round (non-blocking)
skills/repoman/scripts/doc-drift-scanner.sh:44— theskill_attributionfooter points at a path that does not exist inrossoctl/agent-skillsyet (inline comment).
Still open from earlier rounds, unchanged and pre-existing: the standalone skills/automation-health-dashboard/scripts/automation-health-dashboard.sh copy has no doc-drift block, and repoman-setup.sh's enable-program --help list still has a second copy that drifts (upstream fix).
Verdict
Clean diff — no must-fix findings, and I would approve. Filed as COMMENT rather than APPROVE because the author is a returning external contributor and the review workflow's inspection gate requires explicit maintainer confirmation before an approval is recorded; that confirmation is not available in this unattended run. A maintainer should post the approving review.
| CHECK_PY="$SCRIPT_DIR/doc-drift-check.py" | ||
| [ -f "$CHECK_PY" ] || { echo "ERROR: missing $CHECK_PY" >&2; exit 1; } | ||
|
|
||
| SKILL_FOOTER=$(skill_attribution "doc-drift-scanner" 2>/dev/null || true) |
There was a problem hiding this comment.
suggestion: skill_attribution "doc-drift-scanner" builds its source URL against rossoctl/agent-skills, but that repo has no skills/doc-drift-scanner path yet (404) — same for doc-drift-fixer at line 46. Every generated issue/PR body therefore gets a footer link to a dead page, and each run logs a WARN. Either publish both skills upstream before enabling the program, or have the footer omit the link when the source path cannot be verified.
clawgenti
left a comment
clawgenti
left a comment
There was a problem hiding this comment.
Summary
Re-review at 8838271 (7 commits). Since the last pass at 84ac95e, the only delta is the release commit: .claude-plugin/marketplace.json metadata.version 0.1.0 → 0.1.1 plus a Keep a Changelog [0.1.1] entry. No other files changed.
Re-confirmed this round:
- 27/27 vendored files byte-identical to
rossoctl/automation@fa61909029207b08d17eff5625bd0aa4772c90b2;_vendor.json's file list matches the scripts directory exactly (only_vendor.jsonitself is extra). bash tests/test-vendor-engine.shpasses locally (9/9 checks, including the 27-file provenance stamp and fail-loud/no-half-vendor cases).doc-drift-check.py --helpruns; it shells out only togitwith argument vectors (nosh -c,eval, or network calls).- DCO 7/7 commits signed off;
shellcheckpass;pr-titlesteps all succeeded (check-run finalizing). - No
.claude/or.vscode/paths anywhere in the diff. The only agent-adjacent change is.claude-plugin/marketplace.json(plugin/version manifest, no hooks or commands), and.github/workflows/shellcheck.ymlpinsactions/checkoutto a SHA. - No hardcoded secrets in added lines.
- Branch is up to date with
main(a0b39d3), so no rebase needed.
Author: jimcadden (CONTRIBUTOR — returning external)
Areas reviewed: Shell, Python, YAML/CI, plugin marketplace JSON, docs
Agent/IDE config (.claude/.vscode): none
Commits: 7 commits, all signed-off: yes
CI status: passing (DCO, shellcheck; pr-title steps succeeded, run finalizing)
Open non-blocking items (unchanged from earlier rounds):
skill_attributionfooters fordoc-drift-scanner/doc-drift-fixerresolve torossoctl/agent-skills/..., which 404s — these skills ship in repoman, not agent-skills (inline comment).- The standalone
skills/automation-health-dashboard/scripts/automation-health-dashboard.shcopy still lacks the doc-drift section this PR adds to the vendored copy, so theautomation-healthstandalone plugin will not render doc-drift metrics.
Verdict
No must-fix findings — the diff is clean and I would approve. Filed as COMMENT rather than APPROVE because the author is a returning external contributor and the review workflow's inspection gate requires explicit maintainer confirmation before an approval is recorded; that confirmation is not available in this unattended run. A maintainer should post the approving review.
| CHECK_PY="$SCRIPT_DIR/doc-drift-check.py" | ||
| [ -f "$CHECK_PY" ] || { echo "ERROR: missing $CHECK_PY" >&2; exit 1; } | ||
|
|
||
| SKILL_FOOTER=$(skill_attribution "doc-drift-scanner" 2>/dev/null || true) |
There was a problem hiding this comment.
suggestion: skill_attribution "doc-drift-scanner" builds its source URL against rossoctl/agent-skills (SKILL_SOURCE_REPO, github-api.sh:204), but that repo has no skills/doc-drift-scanner path (404) — same for doc-drift-fixer. The doc-drift skills live in repoman, so the footer link will dangle. These scripts are vendored byte-identical to automation@fa619090, so the fix belongs upstream (per-skill source repo, or a _meta.doc-drift-*.json). Non-blocking.
rubambiza
left a comment
rubambiza
left a comment
There was a problem hiding this comment.
Quick human scan/review is completed. Filed tracking issue rossoctl/automation#130. One comment to ensure the decomposition of doc-drift-check.py into submodules of <= 300 LoCs is being tracked. AI-assisted review with Claude Opus will follow next.
| @@ -0,0 +1,1636 @@ | |||
| #!/usr/bin/env python3 | |||
| """Deterministic doc-drift checker (RepoMan). | |||
There was a problem hiding this comment.
@jimcadden Are we planning to do the decomposition of this in a follow-on PR? Do we have a tracking issue upstream?
rubambiza
left a comment
rubambiza
left a comment
There was a problem hiding this comment.
Vendors the doc-drift program into the repoman bundle from automation#124 (source_sha fa619090, verified merged on automation main). The vendoring is correctly extended for a non-shell engine file: vendor-engine.sh gains an EXTRA_FILES path for doc-drift-check.py, the shell manifest grows to 26, _vendor.json stamps the real source SHA with the .py in its file list, and test-vendor-engine.sh asserts the new count plus the .py copy and stamp. marketplace.json adds the doc-drift plugin and bundle entry and bumps to 0.1.1; both new SKILL.md files carry ### Requirements blocks; the entrypoint map, README, and CHANGELOG are updated.
I also inspected the vendored engine scripts for vendoring integrity (not their internal logic, which is reviewed upstream in automation): sibling resolution is self-relative ($SCRIPT_DIR/program-lib.sh, $SCRIPT_DIR/doc-drift-check.py) and every sourced sibling — including the .py — is in the vendored manifest, so nothing 404s or misresolves from the move into skills/repoman/scripts/. URLs are runtime fork-clone targets, and the worklist/.repoman paths are runtime-relative, not automation-only absolutes.
Author: jimcadden (MEMBER — maintainer). Areas reviewed: vendor tooling, marketplace manifest, new skill docs, CI, docs, and vendoring integrity of the engine scripts. Agent/IDE config (.claude/.vscode): none. Commits: 7, all signed-off. CI: passing (DCO, shellcheck, verify-pr-title).
No blocking issues; vendored-script internals are out of scope here and were verified merged upstream (#124).
Assisted-By: Claude Code
Vendors the deterministic doc-drift engine from rossoctl/automation and ships scanner/fixer skills, marketplace wiring, and shellcheck for the vendor tooling. Closes the docs-vs-code gap tracked in rossoctl#8. Signed-off-by: Jim Cadden <jcadden@ibm.com>
Syncs automation@fbf8f7b into the repoman plugin scripts (dashboard Delta, YAML list detection, bullet subcommands, real precheck test). Signed-off-by: Jim Cadden <jcadden@ibm.com>
Syncs automation@c4b55a6 into the plugin scripts. Signed-off-by: Jim Cadden <jcadden@ibm.com>
Sync the plugin scripts to the merged automation main so the stamp matches reviewed upstream code, including the pinned-SHA footer the entrypoint skill already calls. Signed-off-by: Jim Cadden <jcadden@ibm.com> Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
The program map and friendly name already include it. Orientation still listed only the five older programs. Signed-off-by: Jim Cadden <jcadden@ibm.com> Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
The vendored default path is removed on exit. The scanner skill now requires --worklist so the documented command keeps the file. Also name weekly-report in the allowlist sentence and drop unrelated gitignore entries. Signed-off-by: Jim Cadden <jcadden@ibm.com> Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Record the version in the marketplace manifest and changelog. The tag is cut after this pull request merges. Signed-off-by: Jim Cadden <jcadden@ibm.com> Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
8838271 to
3c7bba3
Compare
- Document set-requirements in repoman-setup.sh header subcommand list. - Cite merged PRs rossoctl#12 and rossoctl#9 in the [0.1.1] changelog. Automated update by RepoMan Doc-Drift Fixer (2026-11-05). Signed-off-by: Clawgenti <clawgenti@users.noreply.github.com>
Summary
doc-drift-check.py, scanner, fixer, precheck) at mergedfa619090.doc-drift-scanner/doc-drift-fixerskills, marketplace plugin, Doc-Drift-Manager friendly name, and README rows.vendor-engine.sh(26 scripts + Python checker) and adds shellcheck CI for vendor tooling.--workliston a persistent path. The vendored precheck default lives in a temp dir that is removed on exit, and this PR does not edit vendored scripts pastfa619090.Implements / closes #8.
Test plan
bash tests/test-vendor-engine.shbash skills/repoman/scripts/doc-drift-scanner.sh --helppython3 skills/repoman/scripts/doc-drift-check.py --help_vendor.jsonlistsdoc-drift-*.shanddoc-drift-check.pyscripts/vendor-engine.sh/tests/test-vendor-engine.shAssisted-By: Claude Code