feat(pr-pipeline): resolve answered non-CRITICAL bot threads in a base-only job

.changes/pr-pipeline/fix-resolve-only-job-9649f6d2.md

@@ -0,0 +1,22 @@
+---
+bump: minor
+---
+
+### Added
+
+- pr-unblock's `resolve-feedback-threads` activity resolves review threads on a pull request a
+  person on the trusted-actors list opened, where `publish-feedback`'s `publish-artifact` token is
+  no writer and leaves its answers open. It resolves a thread whose comments are all a review
+  bot's or the lanes App's, whose finding no reviewer comment rates CRITICAL and an adjudicator
+  could judge, and that the lane answered (its VALID reply or a `finding-answered` adjudication is
+  the last comment) or GitHub reports outdated with no lane reply that left it open. A CRITICAL
+  finding, a person-decided security finding and any thread a person wrote or joined stay open.
+  It runs in its own job after `publish-feedback`: a script with no model, granted
+  `mutate-branch` (`contents: write`), reading the threads from GitHub itself
+  (`resolve-threads.mjs`, now on the hold floor as an enforcement path), and judging each thread
+  again on its re-read comments, edits included, before resolving it. It also runs after
+  `publish-feedback`'s own stops (an escalation, `finding-stands`, `no-model-turn`), never after
+  a kill-switch or gate skip. `publish-feedback`'s job and grant do not change.
+- A script leaf's `checkout: base` has the write runner skip the PR head checkout and its SHA
+  assert, so the job holding the lane token holds only the trusted base checkout, credentials not
+  persisted. resolve-config outputs `checkout` (`head` by default) for every script activity.

.changes/source-control/fix-resolve-only-job-9649f6d2.md

@@ -0,0 +1,5 @@
+---
+bump: none
+---
+
+Tests only: the hold-floor pins count the pr-unblock resolve script the floor now holds.

.github/actions/resolve-config/action.yml

@@ -188,6 +188,9 @@ outputs:
   prepare:
     description: "The leaf's prepare mode (`merge-base`), empty when it sets none."
     value: ${{ steps.resolve.outputs.prepare }}
+  checkout:
+    description: "A script activity's checkout in the write runner: `head` (the default) or `base` (no head checkout); empty for a skill."
+    value: ${{ steps.resolve.outputs.checkout }}
   contents:
     description: The App grant's `contents` permission for the activity's effect.
     value: ${{ steps.resolve.outputs.contents }}

.github/actions/resolve-config/activity-rules.json

@@ -27,6 +27,10 @@
     "forbidden-effects": ["mutate-branch", "mutate-tracker", "merge"],
     "never": "Writes to the branch, or posts anything but the recorded replies"
   },
+  "resolve-feedback-threads": {
+    "forbidden-effects": ["publish-artifact", "mutate-tracker", "merge"],
+    "never": "Runs a model or head code, pushes, or posts: it only resolves review threads"
+  },
   "review-dependency": {
     "forbidden-effects": ["publish-artifact", "mutate-tracker", "mutate-branch", "merge"],
     "never": "Holds a write token, posts to the pull request, merges or closes: its fix leaves only as a patch"

.github/actions/resolve-config/callers.test.mjs

@@ -7738,8 +7738,9 @@ test("every caller of a reusable holding model legs passes the transcript key",
 // request-lane-token's kill-switch-ok ends a kill-switch refusal green with no
 // token, so nothing after it may run on success() alone: every later step is
 // gated, a step that reads the token runs only when it is set, and a job that
-// needs the stopped job's success and reads none of its outputs also needs its
-// skip output empty (an output only a token step sets already stops it). A
+// needs the stopped job's success and reads none of its outputs also tests its
+// skip output for empty (an output only a token step sets already stops it);
+// it may also admit named skip reasons, never kill-switch. A
 // reusable workflow's caller job stands for its job, so the same holds for
 // the caller's dependents, through the skip output the workflow exposes.
 test("every kill-switch-ok caller runs no token step and no dependent job on a kill-switch stop", () => {
@@ -7752,6 +7753,7 @@ test("every kill-switch-ok caller runs no token step and no dependent job on a k
       if (cond.replaceAll(`needs.${id}.outputs.${skip} `, "").includes(`needs.${id}.outputs.`)) continue;
       assert.ok(skip, `${name}:${id} exposes the token step's skip-reason for ${otherId}`);
       assert.ok(cond.includes(`needs.${id}.outputs.${skip} == ''`), `${name}:${otherId} skips on ${id}'s ${skip}`);
+      assert.ok(!cond.includes(`needs.${id}.outputs.${skip} == 'kill-switch'`), `${name}:${otherId} never runs on ${id}'s kill-switch`);
     }
   };
   const names = readdirSync(new URL("../../workflows/", import.meta.url)).filter((f) => /\.ya?ml$/.test(f));
@@ -7826,6 +7828,7 @@ test("every kill-switch-ok caller runs no token step and no dependent job on a k
     "pr-unblock.yml:apply-patch-rung-up -> pr-run-activity-write.yml",
     "pr-unblock.yml:apply-shared-copies -> pr-run-activity-write.yml",
     "pr-unblock.yml:publish-feedback -> pr-run-activity-write.yml",
+    "pr-unblock.yml:resolve-feedback-threads -> pr-run-activity-write.yml",
     "pr-unblock.yml:report-escalation -> pr-run-activity-write.yml",
     "work-resolve-issue.yml:apply-issue-patch -> pr-run-activity-write.yml",
     "work-resolve-issue.yml:report-outcome -> pr-run-activity-write.yml",
@@ -8013,11 +8016,12 @@ test(`${READ}: a broker kill-switch stop runs no model, lease, comment or label,
 
 // The caller jobs with a write grant that still start on a read call's stop
 // are gated only on their own select job, never on the read call's outputs:
-// they started on the red broker-check before too. The two write-runner calls
-// mint with kill-switch-ok, so the switch still on stops them green at the
-// broker; fix-ci-unreported posts its check only on a call's broker allow.
+// they started on the red broker-check before too. The write-runner calls
+// (resolve-feedback-threads follows publish-feedback) mint with
+// kill-switch-ok, so the switch still on stops them green at the broker;
+// fix-ci-unreported posts its check only on a call's broker allow.
 const GATED_ON_SELECT = {
-  "pr-unblock.yml": ["publish-feedback", "apply-dependency-verdict", "fix-ci-unreported"],
+  "pr-unblock.yml": ["publish-feedback", "resolve-feedback-threads", "apply-dependency-verdict", "fix-ci-unreported"],
 };
 test(`every caller of ${READ} runs no model, write token, push, comment or label off a read call's kill-switch stop`, () => {
   const { state } = readRun(STOP, "7");
@@ -8400,3 +8404,67 @@ test("work-unblock.yml: a finished work-resolve-issue run in this repository swe
   assert.equal(resolves("issues", "true"), true);
   assert.equal(resolves("workflow_run", "", "false"), false, "the kill switch stops it");
 });
+
+// #12634: the resolve of answered bot threads on an owner PR runs in its own
+// write-runner job after publish-feedback, a script (no model step) whose
+// leaf sets checkout: base, so the job holding its contents: write checks out
+// only the trusted base, credentials not persisted, and never the PR head. It
+// takes no thread ids from another job: the script reads them from GitHub.
+// publish-feedback's job and grant stay as they were.
+test("pr-unblock.yml: resolve-feedback-threads is its own base-only script job after publish-feedback, which is unchanged", () => {
+  const { jobs } = workflow("pr-unblock.yml");
+  const job = jobs["resolve-feedback-threads"];
+  const publish = jobs["publish-feedback"];
+  const tokenPermissions = { contents: "read", "pull-requests": "read", checks: "write", issues: "write", "id-token": "write" };
+  assert.deepEqual(publish.permissions, tokenPermissions, "publish-feedback's job permissions are unchanged");
+  assert.equal(publish.with.activity, "publish-feedback");
+  assert.deepEqual(job.permissions, tokenPermissions, "the job token holds no write beyond publish-feedback's");
+  assert.equal(String(job.uses).split(" ")[0], "./.github/workflows/pr-run-activity-write.yml");
+  assert.equal(job.with.activity, "resolve-feedback-threads");
+  assert.equal(job.with["trusted-base"], "default-tip");
+  assert.deepEqual(job.concurrency, publish.concurrency);
+  assert.deepEqual([job.needs].flat(), ["address-feedback-select", "apply-feedback-patch", "publish-feedback"]);
+  // Only the trigger gate's heads cross from other jobs; no read-runner output.
+  const crossed = [...JSON.stringify(job.with).matchAll(/needs\.([\w-]+)\.outputs\.([\w-]+)/g)].map((m) => `${m[1]}.${m[2]}`);
+  assert.deepEqual(crossed.sort(), ["address-feedback-select.head-sha", "apply-feedback-patch.pushed-commit"]);
+  // It runs on an acted round and on the reply script's own stops, which may
+  // still have answered other threads; never on a kill-switch or gate skip.
+  const runs = (result, skip) =>
+    evaluate(job.if.replace("!cancelled() && ", ""), { "needs.publish-feedback.result": result, "needs.publish-feedback.outputs.skip-reason": skip });
+  for (const skip of ["", "lane-stopped", "awaiting-human", "finding-stands", "no-model-turn"]) {
+    assert.equal(runs("success", skip), true, `runs on skip ${JSON.stringify(skip)}`);
+    assert.equal(runs("failure", skip), false, `not on a failed publish-feedback, skip ${JSON.stringify(skip)}`);
+  }
+  for (const skip of ["kill-switch", "superseded-sha", "not-applicable", "pass-only", "disabled-by-config"]) {
+    assert.equal(runs("success", skip), false, `not on skip ${skip}`);
+  }
+
+  // The leaf: a script, mutate-branch, checkout base; publish-feedback keeps
+  // publish-artifact and the head checkout.
+  const config = parse(read("../../../docs/conventions/pr-pipeline.yaml"));
+  const leaf = config.activities["resolve-feedback-threads"];
+  assert.equal(leaf.script, ".github/scripts/pr-unblock/resolve-threads.mjs");
+  assert.equal(leaf.skill, undefined, "no model step");
+  assert.equal(leaf.effect, "mutate-branch");
+  assert.equal(leaf.checkout, "base");
+  assert.equal(config.activities["publish-feedback"].effect, "publish-artifact");
+  assert.equal(config.activities["publish-feedback"].checkout, undefined);
+  const grants = JSON.parse(read("./lane-grants.json"))["1277265216"].lanes["pr-unblock"];
+  assert.equal(grants["resolve-feedback-threads"], "mutate-branch");
+  assert.equal(grants["publish-feedback"], "publish-artifact");
+
+  // The write runner: checkout base skips the head checkout and its assert;
+  // the base checkout persists no credentials; a script runs no skill step.
+  const steps = workflow("pr-run-activity-write.yml").jobs.run.steps;
+  const step = (name) => steps.find((s) => s.name === name);
+  assert.equal(step("Check out the default branch tip").with["persist-credentials"], false);
+  const on = (name, checkout) =>
+    evaluate(step(name).if, { "steps.lane-token.outputs.token": "t", "steps.config.outputs.checkout": checkout });
+  for (const name of ["Check out the PR head", "Assert the head is the gated SHA"]) {
+    assert.equal(on(name, "base"), false, `${name} skips on checkout: base`);
+    assert.equal(on(name, "head"), true, `${name} runs on checkout: head`);
+    assert.equal(on(name, ""), true, `${name} runs for a skill`);
+  }
+  assert.match(String(steps.find((s) => s.id === "act-skill").if), /kind == 'skill'/);
+  assert.match(String(steps.find((s) => s.id === "act-script").if), /kind == 'script'/);
+});

.github/actions/resolve-config/lane-grants.json

@@ -4,7 +4,7 @@
     "lanes": {
       "work-resolve-issue": { "resolve-issue": "read", "apply-issue-patch": "mutate-branch", "report-outcome": "publish-artifact" },
       "pr-refine": { "fix-docs": "read", "apply-patch": "mutate-branch" },
-      "pr-unblock": { "resolve-conflicts": "read", "apply-conflict-patch": "mutate-branch", "address-feedback": "read", "apply-feedback-patch": "mutate-branch", "publish-feedback": "publish-artifact", "review-dependency": "read", "apply-dependency-patch": "mutate-branch", "apply-dependency-verdict": "publish-artifact", "fix-ci": "read", "apply-patch": "mutate-branch", "sync-shared-copies": "read", "apply-shared-copies": "mutate-branch", "report-escalation": "publish-artifact" },
+      "pr-unblock": { "resolve-conflicts": "read", "apply-conflict-patch": "mutate-branch", "address-feedback": "read", "apply-feedback-patch": "mutate-branch", "publish-feedback": "publish-artifact", "resolve-feedback-threads": "mutate-branch", "review-dependency": "read", "apply-dependency-patch": "mutate-branch", "apply-dependency-verdict": "publish-artifact", "fix-ci": "read", "apply-patch": "mutate-branch", "sync-shared-copies": "read", "apply-shared-copies": "mutate-branch", "report-escalation": "publish-artifact" },
       "pr-merge": { "merge-pr": "merge", "enable-auto-merge": "merge" },
       "work-unblock": { "label-unblocked": "mutate-tracker" },
       "intake-triage": { "apply-verdict": "mutate-tracker", "apply-refinement": "mutate-tracker" },

.github/actions/resolve-config/live-config.test.mjs

@@ -158,6 +158,7 @@ test("live config: pr-unblock runs its slots in order", () => {
       "address-feedback",
       "apply-feedback-patch",
       "publish-feedback",
+      "resolve-feedback-threads",
       "review-dependency",
       "apply-dependency-patch",
       "apply-dependency-verdict",
@@ -345,6 +346,23 @@ test("live config: publish-feedback runs the reply script after address-feedback
   );
 });
 
+// #12634: resolving a thread on an owner PR needs contents: write, so it
+// runs in its own script slot after publish-feedback, which keeps
+// publish-artifact, with no PR head checkout and on a PR head it never reads.
+test("live config: resolve-feedback-threads resolves after publish-feedback with mutate-branch and no head checkout", () => {
+  const slot = selected("pr-unblock", "resolve-feedback-threads");
+  assert.equal(slot.applies, true);
+  assert.equal(slot.kind, "script");
+  assert.equal(slot.script, ".github/scripts/pr-unblock/resolve-threads.mjs");
+  assert.equal(slot.checkout, "base");
+  assert.equal(slot["reads-untrusted"], true);
+  assert.deepEqual(slot.needs, ["publish-feedback"]);
+  assert.deepEqual(
+    resolveGrant(FILES, { lane: "pr-unblock", activity: "resolve-feedback-threads", configPath: CONFIG_PATH }),
+    { effect: "mutate-branch", grant: { contents: "write", "pull-requests": "read", issues: "read" } },
+  );
+});
+
 test("live config: report-escalation follows every pr-unblock activity", () => {
   assert.deepEqual(selected("pr-unblock", "report-escalation").needs, [
     "resolve-conflicts",

.github/actions/resolve-config/resolve-config.mjs

@@ -278,6 +278,7 @@ function outputsFor({ lane, selected }) {
     ...(selected["applied-by"]?.patch === undefined ? {} : { "applied-by-patch": selected["applied-by"].patch }),
     ...(selected.prepare === undefined ? {} : { prepare: selected.prepare }),
     ...(selected.kind === "skill" ? { "permission-mode": selected["permission-mode"] ?? "dontAsk" } : {}),
+    ...(selected.kind === "script" ? { checkout: selected.checkout ?? "head" } : {}),
     ...(selected["issue-dimensions"] === undefined
       ? {}
       : { "issue-dimensions": selected["issue-dimensions"], "timeout-minutes": String(selected["timeout-minutes"]) }),

.github/actions/resolve-config/resolve-config.test.mjs

@@ -203,6 +203,7 @@ test("a valid config writes ResolvedConfig and the selected activity's outputs",
     issues: "read",
     actions: "",
     "can-commit": "false",
+    checkout: "head",
   });
 });
 
@@ -249,6 +250,37 @@ for (const mode of ["bypassPermissions", "acceptEdits", "default", "plan"]) {
   });
 }
 
+// A script's write-runner checkout: head unless the leaf sets base; a skill
+// has none, and setting one on a skill or to another value fails the schema.
+// The key is inserted after `after`, a line of the leaf.
+const withCheckout = (checkout, { activity = "run-tests", after = "    script: scripts/run-tests.sh\n" } = {}) => {
+  const { env } = workspace();
+  env.ACTIVITY = activity;
+  if (activity === "fix-prose") {
+    env.LANE = "pr-refine";
+  }
+  const path = join(env.BASE_PATH, CONFIG_PATH);
+  const config = readFileSync(path, "utf8");
+  const set = config.replace(after, (line) => `${line}    checkout: ${checkout}\n`);
+  assert.notEqual(set, config);
+  writeFileSync(path, set);
+  return env;
+};
+
+test("a script's checkout output is head unless the leaf sets base; a skill has none", async () => {
+  const env = withCheckout("base");
+  assert.equal(await main({ env, github: srcChanged() }), 0);
+  assert.equal(readOutputs(env.GITHUB_OUTPUT).checkout, "base");
+  const skill = withPermissionMode(undefined);
+  assert.equal(await main({ env: skill, github: noApi }), 0);
+  assert.equal(Object.hasOwn(readOutputs(skill.GITHUB_OUTPUT), "checkout"), false);
+});
+
+test("checkout fails the schema on a skill or with a value other than head or base", async (t) => {
+  await assertFailsRed(t, withCheckout("pr"), /checkout/);
+  await assertFailsRed(t, withCheckout("base", { activity: "fix-prose", after: /    reads-untrusted: false\n(?=lanes:)/ }), /checkout|script/);
+});
+
 test("without an activity only enabled and slots are output", async () => {
   const { env } = workspace();
   env.ACTIVITY = "";

.github/actions/resolve-config/resolve.mjs

@@ -903,6 +903,7 @@ function resolveSlot(
     ...(activity["web-access"] === undefined ? {} : { "web-access": activity["web-access"] }),
     ...(activity.tidy === undefined ? {} : { tidy: activity.tidy }),
     ...(activity["permission-mode"] === undefined ? {} : { "permission-mode": activity["permission-mode"] }),
+    ...(activity.checkout === undefined ? {} : { checkout: activity.checkout }),
     ...(activity["applied-by"] === undefined ? {} : { "applied-by": activity["applied-by"] }),
     effect: activity.effect,
     gating: activity.gating,

.github/scripts/pr-merge/merge-holds.mjs

@@ -351,6 +351,7 @@ export const HOLD_PATH_CLASSES = Object.freeze([
   [".github/scripts/pr-unblock/report-escalation.sh", "enforcement"],
   [".github/scripts/pr-unblock/adjudicate-feedback.mjs", "enforcement"],
   [".github/scripts/pr-unblock/publish-feedback.mjs", "enforcement"],
+  [".github/scripts/pr-unblock/resolve-threads.mjs", "enforcement"],
   [".github/scripts/pr-unblock/request-budget.mjs", "enforcement"],
   [".github/scripts/pr-verify/resolve-pr.mjs", "enforcement"],
   [".github/scripts/pr-verify/write-verdict.mjs", "enforcement"],

.github/scripts/pr-merge/refire-refused.mjs

@@ -97,6 +97,7 @@ const LANES = Object.freeze({
       "apply-conflict-patch",
       "apply-feedback-patch",
       "publish-feedback",
+      "resolve-feedback-threads",
       "apply-dependency-patch",
       "apply-dependency-verdict",
       "apply-patch",