Skip to content

feat: GitHub Action — audit changed markdown in PRs#8

Closed
conorbronsdon wants to merge 1 commit into
mainfrom
feat/github-action
Closed

feat: GitHub Action — audit changed markdown in PRs#8
conorbronsdon wants to merge 1 commit into
mainfrom
feat/github-action

Conversation

@conorbronsdon
Copy link
Copy Markdown
Owner

Implements the GitHub Action half of avoid-ai-writing-app#110.

Summary

A composite GitHub Action at .github/actions/avoid-ai-writing/ that auto-runs on PRs touching markdown, audits each changed file via the live API at avoidaiwriting.com, and posts a single summary review comment back to the PR. Three changed files, ~270 lines of bash + yaml, no node_modules to bundle.

Why composite + bash (not a JS action)

The skill repo is intentionally zero-build (markdown only). A JS action means committing a bundled dist/index.js, which is review noise + a maintenance surface that isn't worth it for this action's narrow scope. curl + gh + jq cover the full path cleanly.

Failure-open posture

Every error path exits 0 so a flaky audit can't block a merge:

  • API key missing → ::warning:: + skip
  • API key rejected (401) → halt run, log, exit 0
  • File too large (>60KB or >5000 words, mirroring server caps) → skip + note in summary
  • Insufficient credits mid-run (402) → partial summary + credit note
  • Per-file network error → that file marked errored, others continue

Pre-flight size check matches the server's MAX_TEXT_LENGTH so we don't burn a credit just to come back as 400.

Example workflow location

docs/example-workflow.yml, not .github/workflows/. The skill repo doesn't have an AVOID_API_KEY secret set yet; an active workflow here would fail on every PR until that's configured. Move to .github/workflows/ to enable.

Also in this PR

  • Backfills the missing [3.3.1] CHANGELOG entry for the "hit differently" Tier 1 addition (commit 088c4ad from 4/17, never made it into CHANGELOG).
  • README "Installation & Usage" gains a "GitHub Action — audit docs PRs" subsection with a copy-paste workflow snippet.

Test plan

  • Manual smoke test: create a test repo with AVOID_API_KEY secret, copy docs/example-workflow.yml to .github/workflows/, open a PR with a doc change → verify summary comment posts
  • Edge case: PR with only a deleted .md file → action posts "no files to audit" and exits 0
  • Edge case: oversize file (>60KB) → action skips with a "too large" note
  • Edge case: invalid API key → halts cleanly

… app repo)

Composite action at .github/actions/avoid-ai-writing/. Detects changed

*.md files in a PR (filtered by the user's paths globs), POSTs each to

the audit API at avoidaiwriting.com, posts a single summary review

comment back to the PR.

Composite + bash (not a JS action) on purpose: the skill repo is

intentionally zero-build (markdown only). curl + gh + jq cover the

full path without committing a bundled dist/index.js.

Fails open across the board — missing API key, rejected key, oversized

file, exhausted credits, network error per file all return exit 0 so a

flaky audit can't block a merge. Per-file caps mirror the server's

60KB / 5000-word limits so we don't burn a credit just to get a 400.

Example workflow lives in docs/example-workflow.yml (not

.github/workflows/) so it stays a reference template — the skill repo

doesn't yet have an AVOID_API_KEY secret, and a live workflow here

would fail on every PR until that's configured.

Also backfills the missing [3.3.1] CHANGELOG entry for the

'hit differently' Tier 1 addition (commit 088c4ad).
@conorbronsdon
Copy link
Copy Markdown
Owner Author

@copilot review

Copy link
Copy Markdown

Copilot AI commented May 8, 2026

@copilot review

Reviewed the current PR changes at b595602 and ran validation/checks; no additional changes are needed at this time.

@conorbronsdon
Copy link
Copy Markdown
Owner Author

Five-agent review (code, silent-failure, API-contract, security, UX-adopter)

Two hard blockers prevent first-run success. After those, ~5 high-priority items before this is comfortable to ship publicly. Implementation logic is sound and the API wiring is correct against current /api/v1/audit; the gaps are in shell-options choice, comment hygiene, and adopter-facing docs.


🔴 Blockers (will fail first run or violate stated invariants)

  1. run.sh is not executable in git (git ls-files --stage shows 100644). Composite action invokes the script directly with no chmod, so first run is "Permission denied" on Linux runners. Fix: git update-index --chmod=+x .github/actions/avoid-ai-writing/run.sh && commit, OR change action.yml to run: bash ${{ github.action_path }}/run.sh (more robust — bypasses exec bit entirely).
  2. $last_credits_ interpolation bug at run.sh:164. Trailing _ is a valid bash identifier char, so bash parses $last_credits_ (unset). With set -u this aborts the script just before posting the comment — audit work is lost. Fix: "_API credits remaining: ${last_credits}_".
  3. set -euo pipefail violates the fail-open invariant. Multiple paths abort red instead of exit 0:
    • gh pr comment (line 185) — if auth/network fails, audit ran fine but action goes red
    • jq on $GITHUB_EVENT_PATH (lines 38-40) — payload-shape failure dies before the null guard at line 42
    • wc/cat per-file (lines 108-109, 121) — non-curl per-file failure violates the "others continue" invariant
    • Recommendation: replace with set -uo pipefail (drop -e) and explicitly guard gh pr comment and every jq parse with || { echo "::warning::..."; }.
  4. 200-status with malformed JSON silently passes as clean audit. jq -r '.result // empty' returns nothing on parse failure, file counts as audited-clean. A poisoned 200 (HTML body, cached upstream error) = false negative across the whole PR. Fix: check .result was non-empty AND well-formed before treating as a successful audit.
  5. Markdown injection in PR comment. API result (line 139) is interpolated raw into the comment. <details>, <img src=x onerror=...>, [click](javascript:...), fenced-block-breaking backticks, and HTML comments colliding with the <!-- avoid-ai-writing-action --> sentinel all render. Real risk: a prompt-injected markdown PR plants a fake "✅ no issues" block burying actual findings. Fix server-side: strip HTML tags from result before returning. Or client-side: fence-wrap the audit output in a code block.

🟠 High

  1. Security: script accepts pull_request_target trigger (run.sh:32). Example uses safe pull_request, but the script silently allows the dangerous variant. A user "improving" the example to support fork PRs (the canonical reason to use pull_request_target) hands fork authors the ability to modify run.sh itself in their PR + leak AVOID_API_KEY and github.token (write). Fix: reject pull_request_target outright with a clear error message. The script has no reason to need it; fork PRs without secrets should simply skip with a warning (as they do today).
  2. No comment upsert. The <!-- avoid-ai-writing-action --> marker is written but never used to find+update a prior comment. Every push to a PR appends a new comment; a 6-commit PR shows 6 audit comments. Either wire up the marker-based update (gh api /repos/{owner}/{repo}/issues/{pr}/comments → filter for marker → PATCH) or remove the misleading marker.
  3. No 65KB comment-length guard. With max-files=10 and verbose audit output per file, flagged_list can exceed GitHub's ~65,536-byte cap. gh pr comment --body-file fails and the whole step exits non-zero (because of set -e + no || true on line 185). Truncate comment_body to ~60K with a "(output truncated)" footer.
  4. 401/402 break leaves remaining files invisible. The break-on-error loop never appends "N files not attempted" to errored_list. Summary says "Audited 3 of 10 matched" with no note about the unaudited 7. Fix: track + report unattempted count.
  5. Cost is completely undocumented. README and action description say "each file = 1 API credit" but nowhere does it say how much is a credit. A docs-repo maintainer evaluating this for 40 PRs/month needs a number. Add a one-liner: "X credits per $Y, first N free, see /developers."

🟡 Medium

  1. @main pin in example. First rule of consuming third-party actions: pin to a tag. Cut a v1 and reference it.
  2. mapfile + process substitution + || true can silently treat a git diff failure as "no files to audit." Add an explicit git rev-parse --verify "$base_sha" check before the diff.
  3. Workflow paths: (trigger) vs action paths: input (audit scope) conflation. Example doesn't distinguish them. A user copying the example and setting the workflow-level filter to content/**/*.md still gets the action auditing every diffed .md (default globs *.md,**/*.md). Add a worked example with both set + a callout explaining the difference.
  4. Glob matching is bash standard. No ! negation, no {a,b} braces, no recursive *.md without globstar. The **/*.md second default pattern catches the recursive case but a user supplying just *.md expecting recursive will be surprised. Document the limitation OR enable globstar at the top of the script.
  5. Clean PRs still comment. "Audited 3; 0 flagged" with no findings is noise. Either suppress on flagged=0 && skipped=0 && errored=0, or downgrade to a check-run status instead of a comment.
  6. Bot PRs unfiltered. Dependabot bumping a package that touches any .md (some bumps include CHANGELOG changes) burns credits. Add to example: if: github.actor != 'dependabot[bot]'.
  7. File-cap truncation invisible to PR author. When >10 files match, the script truncates with ::warning:: but the truncation isn't surfaced in the PR comment. Add "Capped at 10; M additional files not audited" line.
  8. API comment is misleading. "Pre-flight size check matches the server's MAX_TEXT_LENGTH" — true for /api/audit but /api/v1/audit only enforces the 5000-word cap, not 60KB. Local check is stricter than the server (safe); fix the comment.

🟢 Low / informational

  1. git fetch --depth=1 for base_sha can fail silently on force-push scenarios. The example workflow's fetch-depth: 0 makes the action's own fetches redundant; document fetch-depth: 0 as a prereq.
  2. API response usage field (words, context) isn't surfaced in the summary. Minor missed-opportunity, not a bug.
  3. Detector v2/v3.4 classification fields (document_classification, class_probabilities) aren't returned by /api/v1/audit — only by the burn-path /api/audit. If you want the action to surface them, the API needs to expose them on v1 first.

✅ Clean

  • API endpoint (https://www.avoidaiwriting.com/api/v1/audit) — correct, uses www (apex would drop POST body)
  • Auth header (Authorization: Bearer ...) — matches v1 handler
  • Request body shape ({text, context}) — matches; context from PR #398 already wired
  • Error codes (401, 402, default) — all match v1 handler
  • API key handling — no set -x, no curl -v, key never echoed, GitHub auto-masks correctly
  • Command injection from PR-controlled content — properly defended (jq --arg, all paths quoted, no eval)
  • GITHUB_TOKEN scope in example — minimal and correct (pull-requests: write, contents: read)

Priority order to ship

  1. Blockers 1-5 (must)
  2. Security Add worth-verbing and reader-steering frame patterns #6 (reject pull_request_target)
  3. UX/operational Add 'hit differently' to Tier 1 word table #7, feat: GitHub Action — audit changed markdown in PRs #8 (upsert, 65KB cap)
  4. Adoption gates feat: position-pivot preface + Cursor rule port #10, feat: AI-tool fingerprint sections (placeholders, citation markup, UTM) #11 (cost docs, version pin)
  5. Everything else as follow-up issues

5/5 agents converged on overlapping findings; the blockers above were independently surfaced by at least two lenses each.

🤖 Reviewed by: pr-review-toolkit:code-reviewer, pr-review-toolkit:silent-failure-hunter, general-purpose × 3 (API contract, security, UX adoption)

@conorbronsdon
Copy link
Copy Markdown
Owner Author

Superseded by avoid-ai-writing-app#405.

After the sub-agent review on this PR surfaced 5 blockers + a security HIGH + several UX gaps, decided to move the Action out of the OSS skill repo entirely. The OSS skill is a free local-LLM tool; the Action is a paid-API consumer. They're different surfaces and shouldn't share a README.

All 5 blockers + the pull_request_target security gate + the highest-impact UX fixes integrated in the migration. See the new PR description for the full list. Branch on this repo deleted.

@conorbronsdon conorbronsdon deleted the feat/github-action branch May 17, 2026 06:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants