fix(ci): stop enrichment bot commits racing content validation

.github/workflows/enrich-tool-assets.yml

@@ -14,7 +14,7 @@ on:
       - submitted
 
 permissions:
-  contents: write
+  contents: read
   pull-requests: write
 
 concurrency:
@@ -28,8 +28,6 @@ jobs:
 
     env:
       PR_NUMBER: ${{ github.event.pull_request.number }}
-      PR_HEAD_REF: ${{ github.event.pull_request.head.ref }}
-      PR_HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }}
       PR_BASE_SHA: ${{ github.event.pull_request.base.sha }}
       PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }}
       PR_AUTHOR: ${{ github.event.pull_request.user.login }}
@@ -131,21 +129,12 @@ jobs:
             echo "changed=true" >> "$GITHUB_OUTPUT"
           fi
 
-      - name: Commit enrichment to PR branch
-        if: ${{ steps.tool-diff.outputs.changed == 'true' && github.event.pull_request.head.repo.full_name == github.repository }}
-        run: |
-          git -C pr config user.name "github-actions[bot]"
-          git -C pr config user.email "41898282+github-actions[bot]@users.noreply.github.com"
-          git -C pr add tools tool-submitters.json
-          git -C pr commit -m "Auto-fill tool media and submitter metadata"
-          git -C pr remote set-url origin "https://x-access-token:${GH_TOKEN}@github.com/${PR_HEAD_REPO}.git"
-          git -C pr push origin "HEAD:${PR_HEAD_REF}"
-
-      - name: Comment when fork PR needs enrichment
-        if: ${{ steps.tool-diff.outputs.changed == 'true' && github.event.pull_request.head.repo.full_name != github.repository }}
+      - name: Comment when PR needs enrichment
+        if: ${{ steps.tool-diff.outputs.changed == 'true' }}
         env:
           CHANGED_TOOLS_JSON: ${{ steps.changed-tools.outputs.json }}
           SUBMITTER_TOOLS_JSON: ${{ steps.submitter-tools.outputs.json }}
+          REVIEW_DECISION: ${{ steps.review-decision.outputs.decision }}
         run: |
           node --input-type=module <<'NODE'
           import { writeFileSync } from "node:fs";
@@ -155,26 +144,35 @@ jobs:
           const baseRoot = `../agentfirst-base-${process.env.PR_NUMBER}`;
           const upstreamUrl = `https://github.com/${process.env.GITHUB_REPOSITORY}.git`;
           const submitterFlags = flags(process.env.SUBMITTER_TOOLS_JSON);
-          const enrichmentFlags = flags(process.env.CHANGED_TOOLS_JSON);
+          const enrichmentFlags = process.env.REVIEW_DECISION === "APPROVED"
+            ? flags(process.env.CHANGED_TOOLS_JSON)
+            : "";
           const enrichmentCommand = enrichmentFlags
             ? `npm run enrich:tool-assets -- --write ${enrichmentFlags}\n`
             : "";
-          const body = `I found missing media metadata and/or repo-managed submitter metadata that should be written back to the PR branch. I can't push directly to forked PR branches, but you can generate the same changes locally with:
+          const body = `<!-- agentfirst-enrichment-preflight -->
+          I found missing media metadata and/or repo-managed submitter metadata. This preflight never commits or pushes to same-repository and fork PRs. Generate the changes locally with:
 
           \`\`\`bash
           git fetch ${shellQuote(upstreamUrl)} ${shellQuote(process.env.PR_BASE_SHA)}
           git worktree add --detach ${shellQuote(baseRoot)} FETCH_HEAD
           npm run sync:tool-submitters -- --base-root-dir ${shellQuote(baseRoot)} --submitted-by ${shellQuote(process.env.PR_AUTHOR)} ${submitterFlags}
           ${enrichmentCommand}git worktree remove ${shellQuote(baseRoot)}
+          npm run validate:content -- --require-submitters
           \`\`\`
 
           Then commit the updated files to this PR branch.`;
           writeFileSync("/tmp/enrichment-comment.md", body);
           NODE
-          gh pr comment "${PR_NUMBER}" --repo "${GITHUB_REPOSITORY}" --body-file /tmp/enrichment-comment.md
+          mapfile -t COMMENT_IDS < <(gh api --paginate "repos/${GITHUB_REPOSITORY}/issues/${PR_NUMBER}/comments" --jq '.[] | select(.user.type == "Bot" and (.body | contains("<!-- agentfirst-enrichment-preflight -->"))) | .id')
+          if (( ${#COMMENT_IDS[@]} > 0 )); then
+            gh api --method PATCH "repos/${GITHUB_REPOSITORY}/issues/comments/${COMMENT_IDS[0]}" -f body="$(cat /tmp/enrichment-comment.md)" --silent
+          else
+            gh pr comment "${PR_NUMBER}" --repo "${GITHUB_REPOSITORY}" --body-file /tmp/enrichment-comment.md
+          fi
 
-      - name: Require fork PR enrichment changes
-        if: ${{ steps.tool-diff.outputs.changed == 'true' && github.event.pull_request.head.repo.full_name != github.repository }}
+      - name: Require PR enrichment changes
+        if: ${{ steps.tool-diff.outputs.changed == 'true' }}
         run: |
-          echo "::error::Commit the generated enrichment and submitter metadata changes to the fork PR branch."
+          echo "::error::Commit the generated enrichment and submitter metadata changes to this PR branch; see the preflight comment for commands."
           exit 1

README.md

@@ -305,9 +305,23 @@ This section should usually be a short bullet list of concrete agent outcomes. A
 
 ## Behind The Scenes
 
+Before opening an internal tool PR, generate repo-managed submitter metadata using the actual GitHub PR author and the exact new tool slug (repeat `--slug` for multiple tools):
+
+```bash
+git fetch origin main
+git worktree add --detach ../agentfirst-base origin/main
+npm run sync:tool-submitters -- --base-root-dir ../agentfirst-base --submitted-by ACTUAL_PR_AUTHOR --slug EXACT_NEW_SLUG
+git worktree remove ../agentfirst-base
+npm run validate:content -- --require-submitters
+```
+
+Commit `tool-submitters.json` along with the tool file before opening the PR. Do not put `submittedBy` in tool frontmatter. The trusted base preserves attribution for existing tools; new tools are attributed to the actual PR author.
+
+For both same-repository and fork PRs, enrichment is a branch-read-only preflight: it never commits or pushes. Missing generated metadata produces one updated correction comment and a failing preflight until the contributor commits the changes. Media generation runs only after an approved review; use the comment's exact local commands, validate, and commit any generated media changes. Fork validation still uses GitHub's normal workflow-approval rules.
+
 After a tool PR is approved, the repo handles a few things automatically:
 
-- author attribution is derived from the PR author
+- author attribution is checked against the trusted base and the PR author
 - missing `logoUrl` defaults to a Google favicon URL based on `websiteUrl`
 - missing `ogImageUrl` is discovered from the tool website's social metadata when available
 - D1 receives authored editorial, evidence, entity, and provenance fields

docs/d1-publish-pipeline.md

@@ -28,7 +28,7 @@ The manifest currently uses schema version `1`. Each entry has `sourcePath`, `de
 
 Manifest paths are canonical absolute same-site paths, not full URLs, and cannot contain a query or hash. Sources must be unique and cannot shadow a current tool or category. Redirect chains and cycles are invalid. A `/tools/<slug>` or `/category/<slug>` destination must resolve to a current source record; any future static destination must be added to the explicit validator allowlist.
 
-Submitter attribution is stored in `tool-submitters.json`, not in tool frontmatter. The approval workflow must derive each changed tool's submitter from the PR author and write that mapping back to the PR branch before merge.
+Submitter attribution is stored in `tool-submitters.json`, not in tool frontmatter. The preflight derives new tools' submitters from the PR author and preserves existing attribution from the trusted base. Contributors must commit the generated mapping before merge; the workflow never commits or pushes to PR branches. See the README for the internal pre-PR generation commands.
 
 This repo is the only authoring source of truth. D1 is a published runtime mirror, not the place where content is edited.
 
@@ -47,12 +47,15 @@ On `pull_request`:
 - validate content only
 - fail on invalid schema or invalid references
 
-On approved tool PRs:
+On all tool PRs (same-repository and fork):
 
-- sync changed tool slugs into `tool-submitters.json` using the PR author login
-- enrich missing `logoUrl` and `ogImageUrl` values
-- commit those generated changes back to same-repo PR branches
-- comment with the exact commands for fork PRs when the workflow cannot push
+- preflight changed tool slugs in a disposable checkout using trusted-base scripts and attribution; new tools use the PR author login
+- only after an approved review, enrich missing `logoUrl` and `ogImageUrl` values
+- never commit or push generated changes; contents permission is read-only
+- create or update a bot-owned correction comment with exact local generation and `--require-submitters` validation commands
+- fail clearly while generated changes still need to be committed by the contributor
+
+The required `validate` check remains on `pull_request`, including normal fork workflow approvals. Privileged enrichment executes only the existing trusted-base automation, never PR-provided scripts.
 
 On `push` to `main` with category, tool, or redirect-manifest changes:
 

test/enrich-preflight.test.mjs

@@ -0,0 +1,119 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+import { readFile, mkdtemp, rm } from "node:fs/promises";
+import { spawnSync } from "node:child_process";
+import { tmpdir } from "node:os";
+import path from "node:path";
+
+const workflowPath = new URL("../.github/workflows/enrich-tool-assets.yml", import.meta.url);
+const workflow = await readFile(workflowPath, "utf8");
+const changedCondition = "if: ${{ steps.tool-diff.outputs.changed == 'true' }}";
+
+function step(name) {
+  const value = workflow.split(`      - name: ${name}\n`)[1];
+  assert.ok(value, `missing step: ${name}`);
+  return value.split("      - name:")[0];
+}
+
+function comment(env) {
+  const source = workflow.match(/node --input-type=module <<'NODE'\n([\s\S]*?)\n\s+NODE/)[1]
+    .replace('import { writeFileSync } from "node:fs";', "const writeFileSync = (_path, body) => process.stdout.write(body);");
+  const result = spawnSync(process.execPath, ["--input-type=module", "--eval", source], {
+    encoding: "utf8",
+    env: {
+      ...process.env,
+      PR_NUMBER: "123",
+      GITHUB_REPOSITORY: "bradvin/agentfirst.directory",
+      PR_BASE_SHA: "abc123",
+      PR_AUTHOR: "actual-author",
+      SUBMITTER_TOOLS_JSON: '["exact-new-slug"]',
+      CHANGED_TOOLS_JSON: '["exact-new-slug"]',
+      REVIEW_DECISION: "REVIEW_REQUIRED",
+      ...env,
+    },
+  });
+  assert.equal(result.status, 0, result.stderr);
+  return result.stdout;
+}
+
+test("enrichment cannot commit/push a PR branch or request contents write", () => {
+  assert.match(workflow, /^permissions:\n  contents: read\n  pull-requests: write$/m);
+  assert.doesNotMatch(workflow, /contents: write|git[^\n]*\b(?:commit|push)\b|x-access-token/);
+});
+
+test("same-repository and fork PRs share correction and failure steps", () => {
+  for (const name of ["Comment when PR needs enrichment", "Require PR enrichment changes"]) {
+    const body = step(name);
+    assert.ok(body.includes(changedCondition));
+    assert.doesNotMatch(body, /head\.repo\.full_name|github\.repository/);
+  }
+  assert.match(step("Require PR enrichment changes"), /::error::.*PR branch[\s\S]*exit 1/);
+});
+
+test("correction comment updates an existing bot-owned marker instead of spamming", () => {
+  assert.match(workflow, /<!-- agentfirst-enrichment-preflight -->/);
+  const body = step("Comment when PR needs enrichment");
+  assert.match(body, /gh api --paginate/);
+  assert.match(body, /\.user\.type == "Bot"/);
+  assert.match(body, /contains\("<!-- agentfirst-enrichment-preflight -->"\)/);
+  assert.match(body, /gh api --method PATCH/);
+  assert.match(body, /gh pr comment/);
+});
+
+test("repeat correction runs update the same comment; only the first run creates one", async () => {
+  const root = await mkdtemp(path.join(tmpdir(), "enrichment-comments-"));
+  const logPath = path.join(root, "gh-calls.log");
+  const body = step("Comment when PR needs enrichment");
+  const commands = body.slice(body.indexOf("          mapfile"));
+  const mocks = `
+    gh() {
+      printf '%s\\n' "$*" >> "$CALL_LOG"
+      if [[ "$1" == api && "$2" == --paginate ]]; then
+        printf '%s' "$EXISTING_COMMENT_ID"
+      fi
+    }
+    cat() { printf 'generated comment body'; }
+  `;
+  try {
+    for (const id of ["", "456", "456"]) {
+      const result = spawnSync("bash", ["-euo", "pipefail", "-c", mocks + commands], {
+        encoding: "utf8",
+        env: {
+          ...process.env,
+          CALL_LOG: logPath,
+          EXISTING_COMMENT_ID: id,
+          PR_NUMBER: "123",
+          GITHUB_REPOSITORY: "bradvin/agentfirst.directory",
+        },
+      });
+      assert.equal(result.status, 0, result.stderr);
+    }
+    const calls = await readFile(logPath, "utf8");
+    assert.equal(calls.match(/^pr comment /gm)?.length, 1);
+    assert.equal(calls.match(/^api --method PATCH repos\/bradvin\/agentfirst.directory\/issues\/comments\/456 /gm)?.length, 2);
+  } finally {
+    await rm(root, { recursive: true, force: true });
+  }
+});
+
+test("both PR origins receive exact author/slug, trusted-base and validation instructions", () => {
+  for (const repo of ["bradvin/agentfirst.directory", "contributor/agentfirst.directory"]) {
+    const body = comment({ PR_HEAD_REPO: repo });
+    assert.match(body, /same-repository and fork PRs/);
+    assert.match(body, /--submitted-by 'actual-author' --slug 'exact-new-slug'/);
+    assert.match(body, /--base-root-dir '\.\.\/agentfirst-base-123'/);
+    assert.match(body, /git fetch 'https:\/\/github.com\/bradvin\/agentfirst.directory\.git' 'abc123'/);
+    assert.match(body, /npm run validate:content -- --require-submitters/);
+    assert.doesNotMatch(body, /npm run enrich:tool-assets/);
+  }
+  assert.match(comment({ REVIEW_DECISION: "APPROVED" }), /npm run enrich:tool-assets -- --write --slug 'exact-new-slug'/);
+});
+
+test("privileged preflight executes only trusted-base scripts and gates media on approval", () => {
+  assert.match(workflow, /ref: \$\{\{ github\.event\.pull_request\.base\.ref \}\}\n\s+path: automation/);
+  assert.match(workflow, /PR_AUTHOR: \$\{\{ github\.event\.pull_request\.user\.login \}\}/);
+  assert.match(step("Checkout PR head"), /persist-credentials: false/);
+  assert.match(step("Sync tool submitters"), /automation\/scripts\/sync-tool-submitters\.mjs --root-dir pr --base-root-dir automation --submitted-by "\$\{PR_AUTHOR\}"/);
+  assert.match(step("Enrich changed tool files"), /review-decision\.outputs\.decision == 'APPROVED'/);
+  assert.doesNotMatch(workflow, /(?:node|npm|npx) pr\/|working-directory: pr/);
+});