Skip to content

fix(intake-triage): honor admission's comment window when judging re-triage - #10121

Merged
kyle-sexton merged 6 commits into
mainfrom
fix/refine-exact-refinement-retriage
Oct 25, 2026
Merged

kyle-sexton merged 6 commits into
mainfrom
fix/refine-exact-refinement-retriage

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Oct 25, 2026 •

Copy link
Copy Markdown
Member

No related issue: owner goal (2026-10-24) "The only thing that should ever stop an item ... is if there's a change in intent and vision"; Issue Triage measured this defect parking 12 of 17 trusted-criteria refusals on NO-QUESTION rows (e.g. #6930, #9018).

Spec

  1. An exact refinement of a listed-human-filed issue with no body task list sets redispatch true and lifts needs-decision, counting the refinement being posted.
  2. A body already refined writes nothing.
  3. A refinement past admission's comment window, or of an App-filed body, keeps the labels.

Summary

On an issue a listed human filed, the lanes App's newest unedited refinement is the spec admission reads (owner decision Q78, admit.mjs refinementSpec). refine.mjs decided whether an exact refinement asks for re-triage by reading the spec before posting that refinement. On an issue with no task list in its body the check found no criteria, posted without re-triage, and the App's earlier status: needs-decision stayed for good. Intake-triage run 40206908072 shows this on #9016: admission refused at 15:27:57Z, then apply-refinement logged "no listed human's text holds a task list yet" even though the refinement held 10 unedited - [ ] criteria. #6885 and #6908 have the same shape.

Fix

applyRefinement's untrusted() in .github/scripts/intake-triage/refine.mjs now appends the refinement it is about to post (rendered by render, authored by the App's Bot id, unedited) as the newest comment, then passes that spec to admit.mjs hasTrustedCriteria. Admission's own refinementSpec and author rules make the call, with no copy of them. These properties are unchanged:

  • one spec read before any label write
  • needs-triage is added before needs-decision is removed
  • the marker comment is posted last
  • a person's needs-decision stays (decisionStays)
  • a refinement that is not exact still waits
  • an issue the App or another bot filed still does not redispatch, because countedRefinement counts a refinement only on a body a listed human wrote

When an issue already has at least admission's comment window of comments (SPEC_COMMENTS, now exported from admit.mjs), the new refinement would fall outside what admission reads, and admission refuses such an issue anyway. In that case no triage is asked. The header, docstring and test comments now describe the new rule.

Verification

  • New tests in refine.test.mjs cover both branches (the App's needs-decision, and status: ready awaiting admission):
    • A human-filed issue with no task list and no earlier App comment now re-triages. The writes are, in order, needs-triage, the needs-decision removal (that branch only), then the comment with the re-triage footer.
    • An App-filed issue of the same shape keeps its labels, and the comment is the only write.
    • An issue with an older unedited App refinement and no task list re-triages, so the new refinement counts as the newest.
    • An issue with 100 comments keeps its labels, and the comment is the only write.
  • Fail-first: on unmodified refine.mjs, both positive tests fail (redispatch: false) and 101 of 103 pass.
  • I checked that each guard has a test that fails without it:
    • Counting the refinement whatever the issue's author fails 4 App-filed negative tests.
    • Adding the refinement as the oldest comment instead of the newest fails 2 tests.
    • Dropping the comment-window check fails 2 tests.
  • node --test in .github/scripts/intake-triage: 446 of 446 pass. The pr-verify and work-unblock suites, which import admit.mjs: 302 of 302 pass.
  • No changelog fragment: .github/scripts/ is not a shipped plugin path (scripts/check-changelog-fragments.sh).

Related

🤖 Generated with Claude Code

kyle-sexton and others added 3 commits October 25, 2026 12:56
…-triage

On an issue a listed human filed, the lanes App's newest unedited
refinement is the spec admission reads (owner decision Q78). refine.mjs
judged hasTrustedCriteria on the spec read before posting that
refinement, so an issue with no body task list never re-triaged and the
App's earlier status: needs-decision stayed (issue #9016, intake-triage
run 40206908072).

untrusted() now appends the about-to-post refinement as the newest
unedited App comment and passes that spec to admit.mjs
hasTrustedCriteria, so admission's own refinementSpec and author rules
decide. Still one spec read before any label write, needs-triage before
needs-decision is removed, and the comment last.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…triage

Past admission's 100-comment window the refinement being posted is unread
and admission refuses, so untrusted() asks for no triage there; admit.mjs
now exports SPEC_COMMENTS for it. New tests pin that the refinement counts
as the newest over an older unedited one with no task list, and the window
case, on both the needs-decision and status: ready branches. Comments now
match the updated header.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review October 25, 2026 21:33
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 25, 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-10-25T21:34:28.807787Z 9b84369 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 commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review reviewed fb3f077905e89bfb66058f972e782e557f338a31: 0 finding(s), 0 as inline comments.

No security issues found in this pull request.

critical-findings: 0

The change makes applyRefinement's untrusted() in .github/scripts/intake-triage/refine.mjs judge the spec as it will read once the refinement is posted. It appends a synthetic App-authored, unedited comment to the spec read and passes the result to hasTrustedCriteria. It also exports SPEC_COMMENTS from admit.mjs.

I read the whole change and what it gates, including the admission helpers in admit.mjs (refinementSpec, countedRefinement, writtenBy, trustedSpecTexts).

  • Trust boundary: the synthetic comment is counted only through admission's own rules. countedRefinement requires the issue body to be written by a listed human, with any last editor also listed. refinementSpec requires a Bot author whose id equals appId, a body starting with REFINE_MARKER, and no edit. An issue filed by the App or another bot therefore does not count, and the new tests cover that case.
  • What the pre-check can do: it only decides whether to add needs-triage and set redispatch. It grants no admission, label authority or approval. The re-triage runs admission again against the real GitHub data, so a wrong answer from the pre-check cannot admit anything admission would refuse. The worst case is one extra triage run, or a missed one.
  • Comment window: spec.commentCount < SPEC_COMMENTS fails closed. At 100 or more comments the refinement would fall outside what admission reads, so no triage is asked.
  • Unchanged guarantees: there is still one spec read before any label write, a person's needs-decision still stays, and a refinement that is not exact still waits.
  • Trusted paths: none of the three changed files matches the listed trusted globs. check-trusted-trigger and select-trusted-text are imported but not modified.
  • Manifests and instruction surfaces: the diff touches neither a plugin manifest nor a standing instruction.

The change adds no new spend. At most it makes a re-triage run slightly more likely, which draws only on existing prepaid or subscription usage.

@github-actions

github-actions Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

claude-review reviewed fb3f077905e89bfb66058f972e782e557f338a31: 0 finding(s), 0 as inline comments.

No findings. I read the full diff and the PR context, and checked the change against admit.mjs and the refine.mjs call sites. I did not run the tests (no shell in this lane), so the 446/446 and fail-first results in the PR body are author-claimed and unverified.

What I checked

  • Synthetic refinement matches admission's rule. refinementSpec (admit.mjs:691) counts a comment only if its author is a Bot with databaseId === appId, its body starts with REFINE_MARKER, and lastEditedAt is null. render() begins with REFINE_MARKER (refine.mjs:485), so the object built in untrusted() (refine.mjs:634) satisfies all three conditions. Appending it as the newest comment puts it where findLast looks first.
  • Trust gate unchanged. countedRefinement still counts a refinement only when a listed human wrote the body (admit.mjs:710). An issue the App or another bot filed therefore still gets no re-triage, and the new App-filed negative test covers that.
  • Window boundary is right. Admission refuses when commentCount > comments.length (admit.mjs:462), that is, above 100 comments. With commentCount at 99, posting makes 100, which admission still reads. With 100, posting makes 101, which it refuses. So spec.commentCount < SPEC_COMMENTS (refine.mjs:635) matches admission exactly.
  • Ordering unchanged. There is still one spec read before any label write, needs-triage is added before needs-decision is removed, the marker comment is posted last, and decisionStays still runs before untrusted().
  • Probe and posted body agree. The probe renders with retriage: true and the posted comment uses post(true). The task-list lines are the same in both, so the probe's answer holds for what gets posted.
  • Tests exercise the claimed paths. The new cases cover both label branches, using the helpers specOf, actor and fakeGitHub as defined in refine.test.mjs. The cases are a human-filed issue with no task list, an older unedited refinement, 100 comments, and an App-filed issue.

Gap (not a finding): no test pins the 99-comment case, where a refinement should still be counted. The < SPEC_COMMENTS bound is correct as written, so I left it unflagged.

Trusted paths: none of the three changed files match the listed globs.

Cost: nothing here can cause a real charge.

…mment

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread .github/scripts/intake-triage/refine.mjs
@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 25, 2026
Merged via the queue into main with commit a31dc4a Oct 25, 2026
36 checks passed
@kyle-sexton
kyle-sexton deleted the fix/refine-exact-refinement-retriage branch October 25, 2026 23:46
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