[kyle-sexton]

Refs: #12634

Summary

On a pull request a trusted person opened, publish-feedback answers review-bot threads but cannot resolve them, because its publish-artifact lane token is no writer there. The owner then resolves them by hand. This adds resolve-feedback-threads, a pr-unblock activity that runs in its own job after publish-feedback and resolves the answered or outdated non-CRITICAL bot threads. CRITICAL findings, security findings a person decides, and every thread a person wrote or joined stay open. This is option (B) from the decision record, as amended, on the token the probe proved.

Fix

• Job (.github/workflows/pr-unblock.yml): resolve-feedback-threads calls the shared pr-run-activity-write.yml. It runs only when publish-feedback succeeded with no skip reason, so a kill-switch stop still stops it. Its job permissions, concurrency group and trigger-gate inputs mirror publish-feedback's. It takes only the trigger gate's two heads from other jobs, never a thread id or a read-runner output.

• Grant: mutate-branch (contents: write, pull-requests: read, issues: read), the grant the probe showed resolves a thread. It is added in lane-grants.json, activity-rules.json and pr-pipeline.yaml. No pull-requests: write is requested. publish-feedback keeps publish-artifact and its job permissions byte for byte.

• Base-only checkout, set in config with no new reusable input: a new optional leaf key, checkout: head|base (schema, resolve.mjs, and a checkout output from resolve-config for every script). On base, the write runner skips "Check out the PR head" and its SHA assert, and gives the script TRUSTED_ACTORS the way it does for a review-replies script. The job then holds only the default-branch checkout at .base, with persist-credentials: false. The broker reads the schema from the caller's main tip, so the key and its first use land together. No .github/standards/ or runner-policy change was needed: local reusable calls are checked against the workflow's own inputs.

• Filter (.github/scripts/pr-unblock/resolve-threads.mjs, a script with no model): the script reads threads, review comments, reviews and issue comments from GitHub. It acts only on a PR whose author is a User on the base trusted-actors list (anything else skips not-applicable). It resolves a thread when all of these hold:

  • every comment is by a review bot or the lanes App, and none was edited by another account;
  • the root is a review finding that no reviewer comment rates CRITICAL (isCritical, adjudicate-feedback's rule) and that adjudicable() allows, so an unrated security-review finding, or a listed bot's finding on a held or unreviewed head, stays open;
  • it is answered (its last comment is the lane's VALID reply or a finding-answered adjudication: isResolvingAnswer, defined next to renderReply) or outdated, with no lane reply that left it open (INCORRECT, UNCERTAIN or VALID-defer, an escalation, or finding-stands).

  Before each resolve the script re-reads the thread and resolves it only if it is unchanged. pr-verify notes stay with the pass-only path. The reviewer and verifier predicates were pulled out of threadAuthors, so one definition serves both paths.

• Hold floor: resolve-threads.mjs is added as an enforcement path. The copies are regenerated with scripts/sync-hold-floor.py, and the count and digest pins are updated in test_floor_paths.py and test_babysit_risk.py.

• Docs: the pr-pipeline README (Owner pull requests, Resolve), pr-run-activity.md (script activities), and changelog fragments for pr-pipeline (minor) and source-control (none, tests only).

For the person merging: the contract and this job use "outdated or answered". babysit's worker tier is stricter: it resolves only a thread that is outdated and answered by a lane login. A bulk autonomous resolve there is refused because a worker's own push marks threads outdated (guard-contract.md, resolve.autonomous-bulk-refused). Here, outdated alone resolves only a bot-only, non-CRITICAL, adjudicable thread that no lane reply left open.

Verification

Criterion	Status
1. On an owner-login PR the lane resolves an outdated or answered, non-CRITICAL bot thread	Built; resolve-threads.test.mjs covers answered (VALID, finding-answered), outdated, and an owner PR resolving only those. The live demo happens after merge: a new activity's grant mints only once it is on main. trusted-base: default-tip reads the config at main's tip, so any owner PR's next address-feedback round shows it.
2. A CRITICAL bot finding and any human-authored thread stay unresolved; a test covers both	resolve-threads.test.mjs: CRITICAL root (answered or outdated), CRITICAL follow-up, unrated security-review, a listed bot on a held or unreviewed head, a human root, a human reply, a bot comment edited by a person
Non-owner-login PR skipped	resolve-threads.test.mjs: a stranger's PR, the lanes App's PR, and a Bot-typed author all skip not-applicable with no resolve call
3. Owner thread resolves per day, 7 days before and after	Post-merge: the issue stays open for this measurement
Own job	callers.test.mjs "resolve-feedback-threads is its own base-only script job…"
Base-only checkout, never the PR head	the same test (head checkout and SHA assert skip on checkout: base and still run on head or for a skill), write-runner-never-rules.test.sh, live-config.test.mjs, resolve-config.test.mjs (output, and the schema refusing the key on a skill or with another value)
Credentials not persisted	the same callers test asserts the base checkout's persist-credentials: false
No model step	leaf is a script with no skill; the write runner's skill step is gated on kind == 'skill' (callers test)
The only job holding the added write	the new activity is the one grant this change adds (lane-grants.json, live-config.test.mjs); the job token's permissions equal publish-feedback's. apply-feedback-patch already held mutate-branch before this change.
publish-feedback unchanged	callers test pins its job permissions; live-config.test.mjs pins its publish-artifact grant
Probe posted	probe result (resolveReviewThread HTTP 200 with contents: write, pull-requests: read)

Local runs:

• node --test: resolve-config's live-config, lane-grants, resolve-config, resolve and broker-vendoring suites (290 pass); callers.test.mjs (235 pass); resolve-threads.test.mjs (9 pass); publish-feedback, adjudicate-feedback, address-feedback, feedback-dispatch and workflow-gates (653 pass); log-api-budget (67 pass).
• scripts/write-runner-never-rules.test.sh (76 pass) and scripts/act-turns-denied.test.sh (63 pass).
• The babysit-prs Python tests via unittest discover (OK, 16 skipped), and sync-hold-floor.py --check (clean).
• scripts/check-affected.sh --base origin/main --no-fetch after merging main.

Related

• feat(pr-pipeline): let the lane resolve answered non-CRITICAL bot threads on owner-login PRs #12634, pr-pipeline: static friction audit of the unattended merge flow and the 2026-10-18 gaps checklist (child of #7531) #12284 (row 15 of its findings)
• Probe PRs: pr-pipeline-sandbox#278, #281

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-03T18:04:47.284087Z	60aa4b6	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 60aa4b6f86

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[github-actions]

claude-security-review reviewed 83a66e6d9f7ff8a2cd7665a5c5134c4afc45f3cb: read 29 of 29 changed file(s); 0 finding(s), 0 as inline comments.

Security review: PR #12893, head 83a66e6d

I found no security issues. I read every hunk of all 29 changed files in .claude-lane/pr.diff. I also read the parts of pr-run-activity-write.yml that the new checkout: base path changes or depends on.

critical-findings: 0

Trust controls checked

The lane token grant is wider, but no untrusted code or text can use it.

• What widens. The new resolve-feedback-threads activity gets mutate-branch, which means contents: write, pull-requests: read and issues: read. The grant is added in lane-grants.json, activity-rules.json and pr-pipeline.yaml. This is a real increase in what the broker mints, so I checked what still stops untrusted code from reaching the token.
• Only base code runs with the token. The script runs as "$RUNNER_TEMP/base-script/$(basename "$SCRIPT")" (pr-run-activity-write.yml#L2168), so its imports come from the trusted base.
• No PR head is checked out. With checkout: base, the head checkout and its SHA assert are skipped (L1520, L1539). The only checkout left is .base, with persist-credentials: false (L739).
• The PR head cannot turn this mode on. The checkout value comes from the base config, read at the default tip (trusted-base: default-tip). The schema allows the key only on a script leaf.
• Nothing else in the job uses the write. No model step runs: the skill step is gated on kind == 'skill'. The lane-branch, push and promote steps are skill-only. The follow-up job adds a signed-commit check because can-commit is true, which is stricter, not looser.
• Untrusted text is only matched, never run. Review comment bodies are only tested against regexes (isCritical, isResolvingAnswer, laneOf). Thread ids come from GitHub's GraphQL, and the script's only write is resolveReviewThread.
• The job token is not wider. The job's permissions: equal publish-feedback's (contents: read).
• Which PRs it acts on. ownerPull limits it to PRs opened by a User on the base trusted-actors list. applies-when still keeps out PRs labeled security: outside-actor or do-not-merge.

The kill switch still stops the lane. The job's if admits only an empty skip reason or one of lane-stopped, awaiting-human, finding-stands or no-model-turn. It never admits kill-switch, and callers.test.mjs now asserts that. The write runner's own broker mint still consults the switch. This settles the earlier thread on pr-unblock.yml.

The merge hold on review threads is narrowed only on owner PRs, by design. The script resolves only bot-only threads that no other account edited, that nothing rates CRITICAL, and that adjudicable() allows. Before each resolve it re-reads the thread and judges it again on the current bodies and edits; that settled the earlier thread on resolve-threads.mjs. Only an owner-opened PR reaches this path, so no outside actor gets a PR through because of it. Under the owner threat model this is a narrowed review-thread friction hold on owner PRs, and I hold nothing for it. Adding resolve-threads.mjs to the hold floor widens the holds.

Other checks

• Instruction surfaces: the diff removes or softens no standing instruction (CLAUDE.md, skill or agent text, hook lists).
• Plugin manifests: none changed.
• Embedded instructions: the PR context file has no text telling the lane to pass the PR, skip a check or drop a finding.

Spend (not a hold): one extra GitHub Actions job per address-feedback round on PRs where it applies. It runs a script with no model, so it spends no model credits.

[github-actions]

claude-review: 1 inline finding(s) on 60aa4b6f860f3d377fe87622ca14d783b8cb2e78; the summary is in this lane's pull request comment.

claude-lane: claude-review

[github-actions]

claude-review reviewed 83a66e6d9f7ff8a2cd7665a5c5134c4afc45f3cb: read 29 of 29 changed file(s); 0 finding(s), 0 as inline comments.

Code review: PR #12893, resolve-feedback-threads (a base-only job that resolves non-CRITICAL bot threads)

I read every hunk of all 29 changed files in .claude-lane/pr.diff. For context I also read the code the new script and job depend on:

• the write runner's head re-read (git/ref), head checkout and SHA assert, trusted-actors copy, and script step in pr-run-activity-write.yml;
• adjudicable and listedState in adjudicate-feedback.mjs;
• keepThreads' comment tagging, and the edit fields that listThreads and the changed THREAD_QUERY fetch, in address-feedback.mjs;
• the live-config test that pins report-escalation's needs.

No findings this round.

What holds up (from reading the code; I ran no tests):

• Tagging matches keepThreads. judgeThread tags each comment's author, lane, listed and the root's security the way address-feedback.mjs:966-967 does, so adjudicable and isCritical judge a thread the same way on both paths.
• Edit detection fails closed. listThreads already fetches lastEditedAt and editor, and the extended THREAD_QUERY re-reads both with each body. An edit whose editor is not a User or Bot, or is missing, reads as edited by another account, so the thread stays open.
• Re-read. The script judges each thread again just before resolving it, on the re-read bodies, edits and resolved/outdated state, after checking that the thread's comment ids and PR number are unchanged. A failed read or resolve is logged, the remaining threads are still tried, and the job then fails.
• Write-runner gating. checkout: base skips only "Check out the PR head" and its SHA assert. No later step reads that assert's outputs. The script runs from $RUNNER_TEMP/base-script/, and the new file is committed 100755. TRUSTED_ACTORS is copied before the point where the head checkout would run.
• Job if. The condition admits '', lane-stopped, awaiting-human, finding-stands and no-model-turn. It never admits kill-switch or a gate skip. This addresses the earlier advisory thread, which is settled with lines unchanged since. The callers test evaluates every one of these cases.
• Config plumbing. resolve.mjs passes checkout through, resolve-config.mjs outputs it for scripts only (head by default), and the schema rejects it on a skill or with any other value. report-escalation's needs leave the new activity out, which fits a job that escalates nothing.
• Hold floor. The new path is added in merge-holds.mjs, pr-pipeline.yaml, pr-require-checks.yml and both hosted review workflows. The count pins move by one. I did not recompute the digest pins; they come from the author's run of sync-hold-floor.py.

For the person merging (a design choice, not a finding; the PR raises it itself): on an owner PR, an outdated, non-CRITICAL, adjudicable bot thread with no lane reply at all is resolved. That clears its conversation-resolution hold without checking that the finding was fixed. The same goes for a reviewer's follow-up after a VALID answer once the lines move (resolve-threads.test.mjs "Still wrong." case). This matches the "outdated or answered" design the PR states, which is looser than babysit's "outdated and answered".

Closing issue: pr-context.md has no ## Closing issue section. The PR says Refs: #12634, so there are no closing-issue criteria to check. Of the criteria the PR quotes in its own table, criterion 2 (CRITICAL and human-authored threads stay open) has tests in resolve-threads.test.mjs, and criteria 1 (the live demo) and 3 (resolves per day) are measured after merge. The "Local runs" results are the author's claims; I did not verify them.

Other checks:

• Manifests: no plugin manifest changed, so there are no manifest lines.
• Path-scoped rules: cost-claims.md matches the two docs files, and they make no cost claims. ruff-pin.md matches the Python test pin edits, which are only count and digest changes.
• Embedded instructions: none in the PR context or the settled threads.
• Security scope: questions about the new contents: write grant and the trust boundary are left to the security lane, which runs on this repository.

Spend (not a hold): one more short, model-free write-runner job per address-feedback round that publish-feedback finishes, on a standard runner.