Repository navigation
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.
| # Prefer tag as boundary truth when boundary=tags | ||
| pass | ||
| for doc_path, text in docs: | ||
| if os.path.basename(doc_path).upper() == "CHANGELOG.md": |
There was a problem hiding this comment.
must-fix: os.path.basename(doc_path).upper() == "CHANGELOG.md" can never be true, because .upper() produces CHANGELOG.MD. Every historical version in CHANGELOG.md is therefore emitted as a mechanical=True correction with replace_with=canon_ver. Separately, extract_versions_from_text (L696) flags every X.Y.Z in every doc (dependency/tool versions, foo==1.2.3 pins in requirements.txt, which matches DOC_EXTS) as drift from the plugin version. Suggest comparing against "CHANGELOG.MD", honouring ignore_paths, and only matching versions that are tied to the project's own name or manifest (or making this check non-mechanical).
| 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.
| # Rebase/extend existing rolling branch if present | ||
| if git fetch "$FORK_REMOTE" "$branch" 2>/dev/null; then | ||
| git checkout -B "$branch" "$FORK_REMOTE/$branch" | ||
| git rebase "$FORK_REMOTE/main" 2>/dev/null || true |
There was a problem hiding this comment.
must-fix: git rebase "$FORK_REMOTE/main" 2>/dev/null || true: a conflicting rebase leaves the shared clone under $REPOS_DIR in a mid-rebase state. The later git checkout main then fails silently, which breaks repo-sync and every other scanner reading that clone. Even a successful rebase rewrites history, so the plain git push at L389 is rejected as non-fast-forward once the fork's main has moved, and the rolling PR can never update again. Suggest git rebase --abort on failure (or rebuilding the branch fresh from $FORK_REMOTE/main) and --force-with-lease on the push, as create_fork_pr does.
| 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.
| code_text = git_show(clone, ref, cfile) | ||
| if code_text is None: | ||
| continue | ||
| m = re.search(pattern, code_text) |
There was a problem hiding this comment.
suggestion: sources_of_truth[].code.pattern (and stable_tag_pattern at L151) come from the scanned repo's .repoman/doc-drift.yaml and are passed straight to re.search. There is no try/except re.error, so an invalid pattern crashes the whole repo scan, which then triggers the issue-close bug in the scanner. There is also no guard against catastrophic backtracking. At minimum, catch re.error and emit a config finding.
|
|
||
| def key(self) -> str: | ||
| claim_n = re.sub(r"\s+", " ", self.claim.strip().lower()) | ||
| return "|".join( |
There was a problem hiding this comment.
suggestion: The dedupe key is repo|check|doc_path|claim|code_path, and claims like version is {canon} are identical for every occurrence in a file, so only the first finding per doc survives (the same happens with retired names and repeated flags). Consider including doc_line, or the old token, in the key.
| "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)
| nxt = _next_significant_line(lines, i + 1) | ||
| if nxt is not None: | ||
| nxt_indent = len(nxt) - len(nxt.lstrip(" ")) | ||
| if nxt_indent > indent and nxt.lstrip().startswith("- "): |
There was a problem hiding this comment.
must-fix (regression): nxt_indent > indent rejects YAML's compact sequence form, where - item sits at the same column as its key. That form is valid YAML and is what yaml.dump and many editors emit. Verified locally:
retired_names:
- old-name
ignore_paths:
- vendor/
sources_of_truth:
- name: version
code:
path: VERSION
This parses to {'retired_names': {}, 'ignore_paths': {}, 'sources_of_truth': {}, 'code': {'path': 'VERSION'}}. Every list is silently dropped, and nested keys leak to the root. Because the key now exists as {}, the cfg.setdefault("ignore_paths", ["CHANGELOG.md", ...]) default at L327 never applies either. CHANGELOG is then no longer ignored, which compounds the L815 / fixer L236 version-rewrite issue. The old hard-coded key list did handle this case. Please use nxt_indent >= indent when the next line starts with - , and add a test for the compact form.
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)
| tag_ver = boundary_label[1:] | ||
| if tag_ver != canon_ver: | ||
| canon_ver = tag_ver | ||
| canon_path = f"tag:{boundary_label}" |
There was a problem hiding this comment.
suggestion: The newest stable tag now overrides the declared (marketplace/manifest) version as canonical. On main, the declared version is usually ahead of the last tag: it gets bumped in the release PR before the tag is cut (this PR itself proposes 0.1.0 → 0.1.1). In that window, any doc that correctly mentions the upcoming version is emitted as a mechanical=True correction back to the older tag version. Because of the repo-wide replace in doc-drift-fixer.sh L236, the fixer would then downgrade every occurrence. Consider only preferring the tag when it is >= the declared version, or flagging the mismatch as a non-mechanical finding instead of rewriting it.
|
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.
| canon_path = f"tag:{boundary_label}" | ||
| canon_line = 1 | ||
| for doc_path, text in docs: | ||
| if os.path.basename(doc_path).upper() == "CHANGELOG.md": |
There was a problem hiding this comment.
must-fix: .upper() turns the basename into CHANGELOG.MD, which can never equal the mixed-case literal CHANGELOG.md — so the changelog is never skipped, and every historical X.Y.Z in it is emitted as a mechanical version correction. Drop .upper() (or compare to CHANGELOG.MD). Still present at head; flagged in every prior review.
| nxt = _next_significant_line(lines, i + 1) | ||
| if nxt is not None: | ||
| nxt_indent = len(nxt) - len(nxt.lstrip(" ")) | ||
| if nxt_indent > indent and nxt.lstrip().startswith("- "): |
There was a problem hiding this comment.
must-fix: Compact YAML sequences (key: followed by - item at the same indent) are dropped. The list-vs-map decision only accepts nxt_indent > indent, so parent[key] becomes {} and the following - lines are skipped by the isinstance(parent, list) guard. Handle same-indent sequence items (and keep the next-sibling-key case working).
| 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: Live mode runs a repo-wide git grep for the old version token and rewrites every hit in *.md/*.rst/*.txt with no path filter. apply_replace is substring-based, so bumping 1.2.3 also rewrites 1.2.30, changelog history, and dependency pins — including files the finding never referenced. Scope the rewrite to doc_path with exact-token boundaries, or keep this check report-only until a targeted fix exists.
| # Rebase/extend existing rolling branch if present | ||
| if git fetch "$FORK_REMOTE" "$branch" 2>/dev/null; then | ||
| git checkout -B "$branch" "$FORK_REMOTE/$branch" | ||
| git rebase "$FORK_REMOTE/main" 2>/dev/null || true |
There was a problem hiding this comment.
must-fix: git rebase ... || true swallows rebase failures, so a conflicting rebase leaves the shared clone stuck mid-rebase. The plain git push (L374) is then rejected as non-fast-forward, and the loop continues with the checkout still detached/mid-rebase. Check the rebase exit status and git rebase --abort (or hard-reset) on failure before proceeding.
| 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: repos_all unions in repos_prev unconditionally, including repos whose checker run failed this round (they continue before REPOS_SCANNED but are still in last report's PREV). Those repos end up with zero current findings and get their rollup issue closed — masking a broken scan as "fixed". Exclude REPOS_FAILED repos from the close path, or carry their previous findings forward.
| "source_repo": "rossoctl/automation", | ||
| "source_sha": "0311752eba5350c74b8c9650137d3e6da65bd547", | ||
| "vendored_at": "2026-10-23T03:46:40Z", | ||
| "source_sha": "c4b55a64f48d097216a021d7479f86da8820c227", |
There was a problem hiding this comment.
must-fix: source_sha records c4b55a6, which is unmerged code. rossoctl/automation#124 is still OPEN (head f668ded), and c4b55a6 is not an ancestor of that head (branches diverged). Land #124, then re-vendor from the merged main SHA so the stamp maps to reviewed upstream code.
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.
| "source_repo": "rossoctl/automation", | ||
| "source_sha": "0311752eba5350c74b8c9650137d3e6da65bd547", | ||
| "vendored_at": "2026-10-23T03:46:40Z", | ||
| "source_sha": "c4b55a64f48d097216a021d7479f86da8820c227", |
There was a problem hiding this comment.
must-fix: source_sha is c4b55a6, but automation#124 HEAD is 97254a5. That branch was rewritten after this stamp, so this snapshot is not an ancestor of the engine PR. Re-vendor before merge. The gap in the four doc-drift scripts is about checker +570/−164, fixer +54/−28, scanner +17, precheck +2. Later engine commits fix behavior this copy still has:
versions/retired_namesreplace the token in every markdown file (doc-drift-fixer.shgit grep), which rewrites changelog history and pins that contain the same token. HEAD edits onlydoc_path:doc_line, with a word boundary.- Every merged PR is stamped
unreleased: true. HEAD classifies frommergedAtvspublishedAt. - A checker failure drops that repo's rows, so the next scan treats them as fixed and can close the rollup. HEAD carries the previous findings forward.
- Flag and path checks bind a mention to the nearest real script on the line, and the checker module is registered before the dataclass exec.
automation-health-dashboard.sh, repoman-run.sh, and repoman-setup.sh already match HEAD.
|
|
||
| setup_workspace "doc-drift-precheck" | ||
| if [ -z "$WORKLIST" ]; then | ||
| WORKLIST="$PROGRAM_TMPDIR/worklist.json" |
There was a problem hiding this comment.
must-fix: The default worklist lives inside PROGRAM_TMPDIR, and setup_workspace deletes that directory on EXIT. A bare doc-drift-precheck.sh (the invocation in the scanner skill) prints worklist: /tmp/doc-drift-precheck-…/worklist.json and then removes the file. --worklist outside the temp dir is fine, which is why the precheck test stays green. This is still true on automation 97254a5, so re-vendoring the current HEAD does not fix it. Write the worklist outside the temp dir (or clear the trap before the successful exit) in the engine, then vendor that commit.
| | `pr-review` | `skills/github-pr-review/` | | ||
| | `automation-health` | `skills/automation-health-dashboard/` | | ||
| | `repo-sync` | `skills/repo-sync/` | | ||
| | `doc-drift` | `skills/doc-drift-scanner/` | |
There was a problem hiding this comment.
suggestion: This map row is right. Step 2 still tells the agent the authoritative allowlist is only link-health, dep-bump, pr-review, automation-health, and repo-sync (around line 149). Add doc-drift there so orientation matches KNOWN_PROGRAMS and the Doc-Drift-Manager row.
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>
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).
| "source_repo": "rossoctl/automation", | ||
| "source_sha": "0311752eba5350c74b8c9650137d3e6da65bd547", | ||
| "vendored_at": "2026-10-23T03:46:40Z", | ||
| "source_sha": "c4b55a64f48d097216a021d7479f86da8820c227", |
There was a problem hiding this comment.
must-fix: source_sha records c4b55a6, a pre-merge commit on the #124 branch. That PR is now merged (2026-11-03T21:03Z, merge commit fa61909, which is also main HEAD), and c4b55a6 is not an ancestor of main:
$ git merge-base --is-ancestor c4b55a6 origin/main # exit 1
So the stamp points at code no branch of main ever contained. Re-vendor from fa61909:
bash scripts/vendor-engine.sh --source <automation-clone> --dest skills/repoman/scriptsThat also refreshes github-api.sh, pr-review-* and weekly-report.sh (the REPORT_PY candidate list from #127 — this copy still hardcodes the OpenClaw path at weekly-report.sh:79).
| canon_path = f"tag:{boundary_label}" | ||
| canon_line = 1 | ||
| for doc_path, text in docs: | ||
| if os.path.basename(doc_path).upper() == "CHANGELOG.md": |
There was a problem hiding this comment.
must-fix: .upper() makes the compared value CHANGELOG.MD, which can never equal the mixed-case literal "CHANGELOG.md", so the changelog is never skipped and every historical X.Y.Z in it is emitted as a mechanical version correction. The merged upstream version drops .upper().
| nxt = _next_significant_line(lines, i + 1) | ||
| if nxt is not None: | ||
| nxt_indent = len(nxt) - len(nxt.lstrip(" ")) | ||
| if nxt_indent > indent and nxt.lstrip().startswith("- "): |
There was a problem hiding this comment.
must-fix: compact YAML sequences (key: followed by - item at the same indent) are dropped: the list-vs-map decision only accepts nxt_indent > indent, so parent[key] becomes {} and the following - lines are skipped by the isinstance(parent, list) guard. Merged upstream uses nxt_indent >= indent.
| 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: live mode rewrites the version token across every *.md/*.rst/*.txt in the clone, with no path or line scoping. apply_replace is substring-based, so bumping 1.2.3 also rewrites 1.2.30, changelog history and dependency pins — including files the finding never named. The merged upstream version replaces only doc_path at doc_line (replace_on_line) and refuses the edit when that line does not match.
| # Rebase/extend existing rolling branch if present | ||
| if git fetch "$FORK_REMOTE" "$branch" 2>/dev/null; then | ||
| git checkout -B "$branch" "$FORK_REMOTE/$branch" | ||
| git rebase "$FORK_REMOTE/main" 2>/dev/null || true |
There was a problem hiding this comment.
must-fix: git rebase … || true swallows rebase failures. A conflicting rebase leaves the shared clone (which repo-sync and the other scanners read) stuck mid-rebase, and the plain git push further down is then rejected as non-fast-forward while the loop continues. The merged upstream version aborts the rebase, detects an existing rebase-merge/rebase-apply state up front, and fails the repo instead of pushing from a half-applied state.
|
|
||
| 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.
must-fix: every merged PR is unconditionally stamped unreleased: true, so release drafts list work that has already shipped. Merged upstream leaves the field off and lets the checker classify unreleased PRs from mergedAt vs the newest published publishedAt (same change in doc-drift-precheck.sh:82).
| 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: repos_all unions repos_prev unconditionally, including repos whose checker run failed this round — they continue before contributing any findings, yet still appear via the previous report, so repo_findings is empty and their rollup issue is closed on a broken run (masking the failure as "fixed"). Merged upstream records failed repos and carries their previous findings forward.
| Merge {"enabled": true} into programs/<name>.json. | ||
| Known programs: link-health, dep-bump, pr-review, automation-health, | ||
| repo-sync, weekly-report. | ||
| repo-sync, weekly-report, doc-drift. |
There was a problem hiding this comment.
nit: this list was updated here, but the second copy of it — cmd_enable_program's --help at lines 321-324 — still reads (link-health, dep-bump, pr-review, automation-health, repo-sync, weekly-report). So enable-program --help tells the user doc-drift is unknown while the allowlist accepts it (and while the error message on line 356 lists it). Same defect exists upstream, so it will come back with the re-vendor unless fixed in rossoctl/automation first.
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>
Summary
doc-drift-check.py, scanner, fixer, precheck).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.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
Made with Cursor