diff --git a/.github/scripts/factory-pr-auth.test.cjs b/.github/scripts/factory-pr-auth.test.cjs new file mode 100644 index 0000000000..de411c8730 --- /dev/null +++ b/.github/scripts/factory-pr-auth.test.cjs @@ -0,0 +1,50 @@ +const assert = require('node:assert/strict'); +const { readFileSync } = require('node:fs'); +const { spawnSync } = require('node:child_process'); +const { test } = require('node:test'); + +const source = readFileSync('.github/scripts/free-model-factory-worker-primitives.sh', 'utf8'); +const start = source.indexOf('persist_issue_pr() {'); +const body = source.slice(start, source.indexOf('\n}\n', start) + 3); + +function run(credential) { + return spawnSync('bash', ['-c', ` + set -Eeuo pipefail + EXPECTED_HEAD=base DISPLAY=test MODEL=test SOURCE=test WORKER_ID=test WORKER=11 + GH_TOKEN=workflow-token + PR_REBASE_TOKEN="$1" + reject_out_of_scope_diff() { return 0; } + git() { + case "$1" in + status) printf changed ;; + rev-parse|ls-remote) printf head ;; + esac + } + gh() { + if [[ "$1 $2" == 'issue view' ]]; then + printf title + elif [[ "$1 $2" == 'pr create' ]]; then + printf 'create:%s\\n' "$GH_TOKEN" >&2 + elif [[ "$1 $2" == 'pr list' ]]; then + [[ "$GH_TOKEN" == workflow-token ]] || exit 9 + fi + } + ${body} + persist_issue_pr 2952 branch + printf 'after:%s\\n' "$GH_TOKEN" >&2 + `, 'test', credential], { encoding: 'utf8' }); +} + +test('PR creation uses trusted credential without changing controller authentication', () => { + const result = run('trusted-token'); + assert.equal(result.status, 0, result.stderr); + assert.match(result.stderr, /create:trusted-token/); + assert.match(result.stderr, /after:workflow-token/); +}); + +test('PR creation fails closed when the trusted credential is missing', () => { + const result = run(''); + assert.notEqual(result.status, 0); + assert.match(result.stderr, /PR_REBASE_TOKEN is required for trusted PR creation/); + assert.doesNotMatch(result.stderr, /create:/); +}); diff --git a/.github/scripts/factory-visibility-enrollment.test.cjs b/.github/scripts/factory-visibility-enrollment.test.cjs new file mode 100644 index 0000000000..900f8761a3 --- /dev/null +++ b/.github/scripts/factory-visibility-enrollment.test.cjs @@ -0,0 +1,88 @@ +const assert = require('node:assert/strict'); +const test = require('node:test'); +const { reconcileMissingPrLabels } = require('./factory-visibility.cjs'); + +const sha = 'a'.repeat(40); +const pr = (number = 1) => ({ + number, state: 'open', draft: false, user: { login: 'JoshCLWren' }, + head: { sha, repo: { full_name: 'JoshCLWren/comic-pile' } }, +}); + +async function run({ prs = [pr()], labels = [], comments = [], event = 'schedule' } = {}) { + const writes = []; + const reads = []; + const issues = { + listLabelsOnIssue() {}, listComments() {}, + async setLabels(input) { writes.push(input); }, + }; + const pulls = { list() {} }; + const github = { + rest: { issues, pulls }, + async paginate(operation, params) { + reads.push(params); + if (operation === pulls.list) return prs; + if (operation === issues.listComments) return comments; + return labels.map(name => ({ name })); + }, + }; + await reconcileMissingPrLabels({ github, context: { + eventName: event, repo: { owner: 'JoshCLWren', repo: 'comic-pile' }, + payload: { pull_request: prs[0] }, + } }); + return { writes, reads }; +} + +test('open event enrolls a manual PR and preserves unrelated labels atomically', async () => { + const { writes } = await run({ labels: ['bug'], event: 'pull_request_target' }); + assert.equal(writes.length, 1); + assert.deepEqual(new Set(writes[0].labels), + new Set(['bug', 'factory', 'factory:review', 'factory:unowned'])); +}); + +test('scheduled sweep paginates all open PRs', async () => { + const { writes, reads } = await run({ prs: [pr(1), pr(2)] }); + assert.deepEqual(writes.map(input => input.issue_number), [1, 2]); + assert.equal(reads[0].state, 'open'); + assert.equal(reads[0].per_page, 100); +}); + +test('missing factory label preserves current owner and advanced stage', async () => { + const { writes } = await run({ labels: ['factory:59', 'factory:ready', 'bug'] }); + assert.deepEqual(new Set(writes[0].labels), + new Set(['bug', 'factory', 'factory:59', 'factory:ready'])); +}); + +test('complete metadata is left untouched', async () => { + const { writes } = await run({ labels: ['factory', 'factory:ci', 'factory:unowned'] }); + assert.deepEqual(writes, []); +}); + +test('forks, dependency bots, drafts, and closed PRs are excluded', async () => { + const fork = pr(); + fork.head.repo.full_name = 'someone/comic-pile'; + const { writes } = await run({ prs: [fork, { ...pr(), draft: true }, + { ...pr(), state: 'closed' }, { ...pr(), user: { login: 'dependabot[bot]' } }, + { ...pr(), user: { login: 'renovate[bot]' } }] }); + assert.deepEqual(writes, []); +}); + +test('only trusted current-head review verdicts recover a missing stage', async () => { + const comment = (head, verdict, association, day) => ({ + body: ``, + author_association: association, created_at: `2026-09-${day}T00:00:00Z`, + }); + const { writes } = await run({ comments: [ + comment(sha, 'pass', 'OWNER', 20), + comment('b'.repeat(40), 'changes-required', 'OWNER', 21), + comment(sha, 'changes-required', 'NONE', 22), + ] }); + assert.ok(writes[0].labels.includes('factory:ci')); +}); + +test('current-head changes-required verdict requests repair', async () => { + const { writes } = await run({ comments: [{ + body: ``, + author_association: 'OWNER', created_at: '2026-09-20T00:00:00Z', + }] }); + assert.ok(writes[0].labels.includes('factory:changes-requested')); +}); diff --git a/.github/scripts/factory-visibility.cjs b/.github/scripts/factory-visibility.cjs index 9b77d67f16..4980bd972e 100644 --- a/.github/scripts/factory-visibility.cjs +++ b/.github/scripts/factory-visibility.cjs @@ -193,6 +193,46 @@ async function ownerFromLinkedIssue(github, context, pullRequest) { return 'factory:unowned'; } +async function reconcileMissingPrLabels({ github, context }) { + const candidates = context.eventName === 'pull_request_target' + ? [context.payload.pull_request] + : await withRetry(() => github.paginate(github.rest.pulls.list, { + ...context.repo, state: 'open', per_page: 100, + })); + for (const pr of candidates) { + if (pr.state !== 'open' || pr.draft + || pr.head.repo?.full_name !== `${context.repo.owner}/${context.repo.repo}` + || /^(dependabot|renovate)(\[bot\])?$/.test(pr.user?.login || '')) continue; + const current = await currentLabels(github, context, pr.number); + const owners = OWNER_LABELS.filter(label => current.has(label)); + const stages = STAGE_LABELS.filter(label => current.has(label)); + if (current.has('factory') && owners.length === 1 && stages.length === 1) continue; + + // Repair missing metadata without taking an existing lease or downgrading + // review/CI state. Never manufacture readiness from green checks alone. + let stage = stages.length === 1 ? stages[0] : 'factory:review'; + if (stages.length === 0) { + const comments = await withRetry(() => github.paginate(github.rest.issues.listComments, { + ...context.repo, issue_number: pr.number, per_page: 100, + })); + for (const comment of [...comments].sort( + (left, right) => new Date(right.created_at) - new Date(left.created_at), + )) { + if (!TRUSTED_ASSOCIATIONS.has(comment.author_association)) continue; + const match = (comment.body || '').match( + /comic-pile-factory-review-v\d+:([a-f0-9]{40}):(pass|changes-required)/, + ); + if (match?.[1] !== pr.head.sha) continue; + stage = match[2] === 'pass' ? 'factory:ci' : 'factory:changes-requested'; + break; + } + } + await reconcileLabels(github, context, pr.number, { + owner: owners.length === 1 ? owners[0] : 'factory:unowned', stage, + }); + } +} + async function reconcile({ github, context }) { await ensureLabels(github, context); @@ -281,6 +321,7 @@ async function reconcile({ github, context }) { } module.exports = reconcile; +module.exports.reconcileMissingPrLabels = reconcileMissingPrLabels; module.exports._test = { durablePrOwner, ownerFor, diff --git a/.github/scripts/free-model-factory-worker-primitives.sh b/.github/scripts/free-model-factory-worker-primitives.sh index 105797d503..8fe819d13d 100755 --- a/.github/scripts/free-model-factory-worker-primitives.sh +++ b/.github/scripts/free-model-factory-worker-primitives.sh @@ -481,7 +481,11 @@ persist_issue_pr() { title="$(gh issue view "$number" --json title --jq .title)" body="$(printf 'Closes #%s.\n\nModel: %s\nSource: %s\nWorker: %s\n\nProduced by fixed-model Factory %s (%s). Normal ComicPile exact-head factory merge gates apply.\n' \ "$number" "$MODEL" "$SOURCE" "$WORKER_ID" "$WORKER" "$DISPLAY")" - gh pr create --base main --head "$branch" --title "$title" --body "$body" >/tmp/factory-pr-url + # PR creation must use the same trusted actor as pushes. The workflow + # token makes the author github-actions[bot], which can require approval + # for Actions and causes CodeRabbit to skip the initial review. + GH_TOKEN="${PR_REBASE_TOKEN:?PR_REBASE_TOKEN is required for trusted PR creation}" \ + gh pr create --base main --head "$branch" --title "$title" --body "$body" >/tmp/factory-pr-url pr="$(gh pr list --state open --head "$branch" --json number --jq '.[0].number')" fi echo "$pr" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cbfbcce704..4417766342 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,6 +22,18 @@ env: IMAGE_NAME: ${{ github.repository_owner }}/${{ github.event.repository.name }} jobs: + factory-pr-auth: + name: Factory PR authentication + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - uses: actions/checkout@v7 + - uses: actions/setup-node@v6 + with: + node-version: '24' + - run: node --test .github/scripts/factory-pr-auth.test.cjs .github/scripts/factory-visibility*.test.cjs + build: name: Build Docker Image runs-on: ubuntu-latest diff --git a/.github/workflows/factory-issue-pr-state-reconciler.yml b/.github/workflows/factory-issue-pr-state-reconciler.yml index 623ba76bbe..334352740a 100644 --- a/.github/workflows/factory-issue-pr-state-reconciler.yml +++ b/.github/workflows/factory-issue-pr-state-reconciler.yml @@ -2,7 +2,7 @@ name: Factory Issue PR State Reconciler on: pull_request_target: - types: [opened, reopened, closed] + types: [opened, reopened, ready_for_review, closed, unlabeled] workflow_dispatch: schedule: # PR events cannot repair issues whose last canonical PR was closed before @@ -32,6 +32,13 @@ jobs: ref: ${{ github.event.repository.default_branch }} persist-credentials: false + - name: Enroll repository PRs missing factory labels + uses: actions/github-script@v8 + with: + script: | + const { reconcileMissingPrLabels } = require('./.github/scripts/factory-visibility.cjs'); + await reconcileMissingPrLabels({ github, context }); + - name: Reconcile linked issue from trusted canonical PR history env: GH_TOKEN: ${{ github.token }} diff --git a/.github/workflows/free-model-factory-run.yml b/.github/workflows/free-model-factory-run.yml index 6a255b6584..8320e1d5a6 100644 --- a/.github/workflows/free-model-factory-run.yml +++ b/.github/workflows/free-model-factory-run.yml @@ -51,8 +51,8 @@ jobs: timeout-minutes: 30 env: # Workflow token handles GitHub API, labels, comments, and backlog-zero - # workflow dispatch. A dedicated PAT is used only for git fetch/push so - # pull_request workflows are attributed to a trusted actor. + # workflow dispatch. Git fetch/push and PR creation use the dedicated + # PAT so pull_request workflows and initial review use a trusted actor. GH_TOKEN: ${{ github.token }} PR_REBASE_TOKEN: ${{ secrets.PR_REBASE_TOKEN }} NVIDIA_API_KEY: ${{ secrets.NVIDIA_API_KEY }} diff --git a/docs/AUTONOMOUS_FACTORY_POLICY.md b/docs/AUTONOMOUS_FACTORY_POLICY.md index d44f54d8cb..d302ae11ad 100644 --- a/docs/AUTONOMOUS_FACTORY_POLICY.md +++ b/docs/AUTONOMOUS_FACTORY_POLICY.md @@ -230,6 +230,12 @@ Apply these states exactly. Reconcile each target with one full label-set replac Rules: +- The issue/PR state reconciler enrolls open, non-draft repository PRs missing + factory metadata on open/reopen/unlabel events and every 15 minutes. Forks, + Dependabot, and Renovate are excluded. It preserves unrelated labels and an + existing owner/stage, defaults missing ownership to `factory:unowned`, and uses + exact-head trusted review markers when recovering a missing stage; otherwise + it requests `factory:review`. Green CI alone never establishes readiness. - `factory:building`, `factory:review`, `factory:changes-requested`, `factory:ci`, `factory:ready`, and `factory:blocked` are mutually exclusive workflow states. - `factory:unowned`, `factory:local`, and every `factory:` owner label are mutually exclusive next-action owners. - Never leave a factory-produced or factory-managed open PR without `factory`, one truthful workflow-state label, and one truthful owner label. @@ -240,6 +246,10 @@ Rules: ## Repository safety +- Fixed-model PR creation must use `PR_REBASE_TOKEN`, like git pushes. Keep the + workflow token for controller comments and labels; creating the PR with that + token attributes it to `github-actions[bot]`, which can leave Actions awaiting + approval and causes CodeRabbit to skip initial review. - Never push directly to `main`. - Never create or convert a draft PR unless Josh explicitly requests a draft. - Never enable auto-merge.