diff --git a/.github/workflows/enrich-tool-assets.yml b/.github/workflows/enrich-tool-assets.yml index 455489f..1c85097 100644 --- a/.github/workflows/enrich-tool-assets.yml +++ b/.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 = ` + 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(""))) | .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 diff --git a/README.md b/README.md index 712bb5a..eefe44e 100644 --- a/README.md +++ b/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 diff --git a/docs/d1-publish-pipeline.md b/docs/d1-publish-pipeline.md index 271d69f..4e461fd 100644 --- a/docs/d1-publish-pipeline.md +++ b/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/` or `/category/` 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: diff --git a/test/enrich-preflight.test.mjs b/test/enrich-preflight.test.mjs new file mode 100644 index 0000000..311d214 --- /dev/null +++ b/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, //); + const body = step("Comment when PR needs enrichment"); + assert.match(body, /gh api --paginate/); + assert.match(body, /\.user\.type == "Bot"/); + assert.match(body, /contains\(""\)/); + 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/); +});