Skip to content

fix(code-tidying): link upkeep PRs to a standing spec issue so pr-verify can judge them - #12898

Merged
kyle-sexton merged 4 commits into
mainfrom
fix/12828-upkeep-spec-link
Nov 3, 2026
Merged

kyle-sexton merged 4 commits into
mainfrom
fix/12828-upkeep-spec-link

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Nov 3, 2026 •

Copy link
Copy Markdown
Member

Refs: #12828

Summary

Every PR the daily code-tidying upkeep batch opened failed pr-verify with "no spec to judge against", because its body opened with No related issue:. The batch now opens each body with Refs: #<upkeep_spec_issue>, a standing spec issue (#12894) that holds the upkeep sweep contract as done-when checks, so pr-verify judges upkeep PRs on their content. pr-verify is unchanged.

Fix

  • docs/conventions/code-tidying.yaml: new upkeep_spec_issue: 12894, read through upkeep-setting.sh with no default (schema and reference/config.md updated). Replacing the spec issue is a one-line change.
  • upkeep.mjs and batch.mjs:
    • The batch writes the Refs: line itself; the model no longer writes it.
    • The body is checked with refsIssue, the reader pr-verify and pr-merge share. A body that links another issue or carries a closing keyword is refused.
    • If the setting is missing or invalid, the gate fails before the model runs, and scan and publish refuse to build a body, so no PR opens.
  • maintenance-tidy-code-upkeep.yml: the gate's spec-issue output reaches the scan and publish jobs.
  • Standing spec: code-tidying upkeep sweep contract #12894 stays out of every lane:
    • work-map keeps it off the /work-items:work frontier.
    • Without agent-ready the issue lane never runs it.
    • status: confirmed and the born-triaged marker keep intake from re-triaging it.
    • Neither label is a pr-merge issue hold (pr-pipeline.yaml:446).
    • Refs: never closes it.

Verification

  • Upkeep and batch node tests: 53/53 after the change, 3 failing before it. A mutant body without Refs: fails the "body opens with Refs:" test.
  • open-pr-count.test.sh 21/21, remove-comments settings 19/19, resolve-config callers and live config 286/286; 31 changelog fragments valid.
  • pr-verify accepts a Refs: issue whatever its labels; only the body author's trust counts (resolve-pr.mjs:609-621,687).
  • Left out: the live check runs on the next daily batch after merge (see Remaining). No upkeep PRs are open now, so the cap does not block it.

Remaining

Related

🤖 Generated with Claude Code

kyle-sexton and others added 3 commits November 3, 2026 13:06
…issue

The code-tidying upkeep batch opened every pull request with "No related
issue", so pr-verify had no spec and failed each one, and the batch stalled
at upkeep_max_open. Each body now opens with `Refs: #<upkeep_spec_issue>`,
read from docs/conventions/code-tidying.yaml (set to #12894, the sweep
contract). The batch writes the line itself and refuses a body that links
another issue; with the setting missing or invalid the gate fails before
the model runs, and scan and publish open nothing.

Refs: #12828

Co-Authored-By: Claude Opus 5.5 <[email protected]>
check-born-triaged.mjs matches the marker with endsWith, so a marker
anywhere else in the body does not skip intake; the README said
"contains".

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@kyle-sexton
kyle-sexton marked this pull request as ready for review November 3, 2026 18:43
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

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:45:57.886578Z dd563b9 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.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

claude-lane: claude-review

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

claude-review reviewed 344107408e7fb1fbf561f794e5adff128b45787d: read 2 of 2 changed file(s); 0 finding(s), 0 as inline comments.

Code review: #12898, incremental (dd563b9 → 3441074)

I read every hunk of the two files changed since the last review: batch.mjs and batch.test.mjs. I found no new problems.

What changed

  • The full-URL closing link is now refused (last round's inline finding).
    • CLOSING (batch.mjs:119) now matches a closing keyword followed by #n, owner/repo#n, or http(s)://github.com/<owner>/<repo>/issues/<n>.
    • prBody refuses any of these, so pr-verify can no longer switch from the spec issue to a URL-linked issue.
    • Two new cases in the refusal test cover the change: Fixes https://…/issues/123, and closed: http://…/issues/9, which uses the colon and http forms.
    • This answers the earlier thread, which the owner resolved and nothing has changed since.
  • The closing-issue finding from last round's summary is addressed.

Notes

  • Tests: I ran nothing. The 53/53 result is the author's claim, not something I checked.
  • Other files: none of the other changed files changed since dd563b9. No plugin manifest is in this increment.
  • Spend: nothing beyond current usage.
  • Embedded instructions: none found in the PR context, settled-threads or prior-findings files.
  • Round cap: this is round 2 of 3, so the cap did not apply.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

claude-lane: claude-security-review

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 344107408e7fb1fbf561f794e5adff128b45787d: read 2 of 2 changed file(s); 0 finding(s), 0 as inline comments.

Security review (incremental): PR #12898, dd563b91..34410740

No security issues found in this push. I read every hunk of both files in scope: .github/scripts/maintenance-tidy-code-upkeep/batch.mjs and batch.test.mjs.

What changed

This push fixes the SUGGESTION I raised last round (discussion_r4459135907). CLOSING (batch.mjs#L119) now also matches a closing keyword followed by a full issue URL: https?://github.com/<owner>/<repo>/issues/<n>.

What I checked

  • The fix is complete for the case raised. The regex is case-insensitive and still allows the optional : after the keyword. prBody refuses any body that matches it (L128), so a model-written body like Fixes https://github.com/.../issues/123 can no longer:
    • close an unrelated issue on merge, or
    • replace the Refs: spec issue that pr-verify judges against.
  • The new tests cover both new forms: a Fixes URL over https and a closed: URL over http.
  • Nothing else changed. The refsIssue check, the spec-issue validation and the error messages (which name only a sweep index) are the same as before.
  • No trust-control widening, no merge-hold change and no manifest change in this push.

Prior findings and settled threads

  • The one inline finding from the earlier round is fixed by this push, so I haven't reposted it.
  • The earlier review and summary comments carried no other finding.
  • No settled thread applies to these lines.
  • Round 2 of the cap of 3; the cap did not apply.

Embedded text: none found in the PR context or the prior-findings file.

Spend: none beyond the existing daily upkeep batch.

critical-findings: 0

…n upkeep bodies

GitHub links "Fixes https://github.com/<owner>/<repo>/issues/<n>" as a
closing reference too, so the upkeep body check let a model-written body
close an unrelated issue on merge and replace the spec pr-verify judges.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_015te8CrNfM6v83JDbLRFDA9
@kyle-sexton

Copy link
Copy Markdown
Member Author
# Finding (claude-review, closing #12828) Classification Evidence
2 Closes #12828 would close the issue before its live done-when (an upkeep PR reaching the merge queue without a session) can run VALID (fix now) That criterion needs a daily batch run after merge.

Fixed: the body now links with Refs: #12828 and lists the live check under ## Remaining; the issue closes once a live upkeep PR reaches the merge queue.

@kyle-sexton
kyle-sexton added this pull request to the merge queue Nov 3, 2026
Merged via the queue into main with commit 262b03b Nov 3, 2026
78 checks passed
@kyle-sexton
kyle-sexton deleted the fix/12828-upkeep-spec-link branch November 3, 2026 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant