Repository navigation
docs-truth: post PR comment with ready-to-copy ledger rows on missing_ledger_row - #29
Conversation
…_ledger_row Recurring stall observed this week: autonomous agents add tracked markdown to a subject repo (docs_for_team, mindburn-infra, ...) without opening the paired ledger PR in Mindburn-Labs/docs. The push-time whole-estate scan then goes red on that subject repo's main until someone opens the ledger PR by hand. This happened twice in the last 24h (Mindburn-Labs/docs #27 and #28) and burned real session time each round. Add a post-hoc step to the reusable workflow that, on pull_request events whose docs-truth-report.json contains missing_ledger_row failures, upserts a PR comment listing: * exactly which tracked files are missing rows, * a copy-paste-ready CSV block using the standard subject-repo schema (repo, path, active, owner=<repo>, repo_docs, four owning-repo source refs, ...), one line per orphan, * an alphabetical-insertion hint into the repo block, * a rerun command for once the ledger PR merges. Uses actions/github-script@60a0d83 (v7.0.1, verified) with a marker string so the comment is edited on re-runs and deleted when the report is clean — never leaves stale advice on a PR that's since been fixed. Runs with if: always() so it fires whether or not the Ruby step exit code was non-zero (a clean report also triggers the deletion path). Also declare pull-requests: write in the reusable permissions block. Caller workflows must additionally bump their own permissions (contents: read + pull-requests: write) for the token to actually carry that permission — reusable declarations alone do not elevate. Bump instructions land in the follow-up PRs against subject repos. Longer-term refactor of the two-repo ledger flow is HELM-41; this is a stop-gap until that lands.
Strix Security ReviewAll previously reported security findings have been resolved. 1 resolved finding
Review summaryReviewed the reusable workflow change that adds a post-run PR comment for Updated for Reviewed by Strix |
![strix-security[bot]](https://avatars.githubusercontent.com/in/2749804?s=60&v=4){kind=small}
Address Strix Security Review LOW-severity finding on #29 (CWE-116): attacker-controlled report.failures[].path values were embedded directly into both a markdown code-span and a fenced CSV row. A PR author could smuggle backticks or CR/LF through a crafted markdown filename and either close the inline-code span (injecting arbitrary markdown / HTML into the trusted bot comment) or break the CSV framing (bumping downstream fields). Adopt Strix's suggested fix, with one tightening: RFC 4180-quote every CSV column, not just the untrusted ones. Consequences: - escapeMarkdownInline: backslash-escape backticks and backslashes (order matters — backslash first), visualize CR/LF as literal '\r'/'\n' so the file listing stays one bullet per line. - escapeCsvField: double any embedded quote, visualize CR/LF, wrap in double quotes. Applied uniformly across all ten columns. Empty-string columns become "" instead of nothing — semantically identical under the ledger's CSV reader, and the round-trip is cleaner if a reader copies the block straight into audit/markdown-estate-ledger.csv. The 'subject' repo name is not user-controlled (comes from steps.subject.outputs.repo which is derived from GITHUB_REPOSITORY / inputs.repo), but running it through the same escape keeps the code uniform and avoids a future review-round if someone widens the source. Verification: - actionlint clean. - Manual trace for a crafted path like 'foo`bar\nbaz.md': in markdown it renders as literal backticks inside the code span (backslash escapes), in CSV it renders as "foo`bar\\nbaz.md" with visible \\n rather than a real line break. - Empty-report / no-orphans branches unchanged.
There was a problem hiding this comment.
Pull request overview
Adds a post-processing step to the reusable docs-truth-public workflow to (on PR runs) upsert or delete a marker-tagged PR comment when docs-truth-report.json contains missing_ledger_row failures, including a copy/paste CSV block for the ledger.
Changes:
- Grants additional workflow token permissions intended to allow PR comment upserts.
- Uploads the docs-truth report artifact and then conditionally creates/updates/deletes a PR comment based on
missing_ledger_rowfailures. - Implements escaping for markdown/CSV output to reduce filename-based injection risks.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
peycheff-com
left a comment
peycheff-com
left a comment
There was a problem hiding this comment.
I can't approve this revision as-is.
Blocking wiring issue: .github/workflows/docs-truth.yml still calls ./.github/workflows/docs-truth-public.yml under caller permissions that only grant contents: read. This PR makes the called workflow request pull-requests: write for issue-comment upsert/delete, but GitHub does not allow a reusable workflow to elevate the caller token permissions. The active Docs Truth run on this PR already failed at startup (28668939087) before any job logs were created.
Minimal fix: add pull-requests: write to the caller workflow permissions in .github/workflows/docs-truth.yml (or the reusable-call job permissions) alongside contents: read, then rerun Docs Truth. The inline script itself and escaping pass my review once that caller permission is wired.
peycheff-com
left a comment
peycheff-com
left a comment
There was a problem hiding this comment.
Approved after the caller workflow permission fix in 4667457 and refreshed passing checks.
peycheff-com
left a comment
peycheff-com
left a comment
There was a problem hiding this comment.
Approved latest head 1209798 after caller permission wiring, marker-comment ownership guard, malformed-report cleanup, and passing refreshed checks.
Review round summary — ready to mergeStrix Security Review (updated for Copilot PR Reviewer left four inline comments on
Checks
Note on the CODEOWNERS blockMerging via admin override. CODEOWNERS references Stale CODEOWNERS in this repo tracked separately in HELM-44 — mirror of Ivan's helm-ai-kernel#493 fix. Follow-up post-mergeBump the pinned SHA in the six subject-repo |
Summary
Adds a post-hoc step to the reusable `docs-truth-public` workflow that, on `pull_request` runs whose report contains `missing_ledger_row` failures, upserts a PR comment listing the offending files and a copy-paste-ready CSV block for `Mindburn-Labs/docs/audit/markdown-estate-ledger.csv`.
Motivation
Recurring pattern this week: autonomous agents (`codex/*`, and manual pushes) add tracked markdown to a subject repo without opening the paired ledger PR. Whole-estate scan on push then goes red on `main` in that subject repo until a human notices and opens the ledger PR by hand. Happened twice in the last 24h (Mindburn-Labs/docs #27 and #28) and burned real session time each time.
This bot doesn't fix the underlying two-repo ledger design (that's HELM-41) — it removes the "how do I fix this?" friction by putting the exact rows to add directly on the PR.
What the comment looks like
Comment is marker-tagged (``) so the same step edits it on re-runs and deletes it when the report goes clean. A PR that fixed the drift after review will not carry stale advice.
Permissions
Reusable now declares `pull-requests: write`. Caller workflows must also carry `pull-requests: write` for the token to actually elevate — reusable-side declarations alone are not enough. Follow-up: bump the caller workflows in `docs_for_team`, `mindburn-infra`, `helm-ai-enterprise`, `helm-ai-kernel`, `gitops-platform`, `docs` (only the subject repos that call this reusable). Small PR per repo, single-line change plus a permissions bump.
Verification
Test plan
References