Skip to content

feat(pr-pipeline): admit marked decided issues by the admission rules and re-fire stranded ones - #9566

Merged
kyle-sexton merged 12 commits into
mainfrom
feat/hitl-admission-mark-stranded-check
Oct 24, 2026
Merged

kyle-sexton merged 12 commits into
mainfrom
feat/hitl-admission-mark-stranded-check

Conversation

@kyle-sexton

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

Copy link
Copy Markdown
Member

No related issue: owner decision Q75 (2026-10-24)

Summary

The mark: add the admission-deferred label to a decided issue that has status: ready; the hourly work-unblock sweep checks it against the admission rules and admits it when there is room.

  1. HITL admission mark (Q75). Rerunning intake on an issue a person decided sends it to needs-decision, because apply-verdict keeps any value a person set. With this change, when the newest admission-deferred add is by a listed human and the issue has status: ready, work-unblock's deferred pass reads the issue's own work-class: label and its Size, Complexity and Uncertainty fields. It runs admit.mjs decide() on them and admits within the room only on an admit verdict. Intake's own deferrals (added by the lanes App) keep the room-only path.
  2. Trusted-human acceptance criteria (Q9, feat(source-control): adopt idle owner pull requests in the pr-merge lane #8999). pr-verify judges a lane PR only against the body and comments a listed human wrote and last edited (resolve-pr.mjs written()). decide()'s task-list check (criterion 7) now reads only those texts.
    • The lanes App's refinement comment, or an App-filed body, no longer satisfies it. A person who confirms such criteria posts them as their own comment.
    • An App-drafted task list plus an owner comment with no task list is refused.
    • There is one refusal: "no trusted-human acceptance criteria: a listed human must put the confirmed - [ ] criteria in the body or post them as their own comment (owner Q9, feat(source-control): adopt idle owner pull requests in the pr-merge lane #8999)". The issue moves to status: needs-decision.
    • This applies to intake (apply-verdict to admit) and to the mark path.
  3. Stranded agent-ready issues. An agent-ready issue with no hold, no open lane PR and no outcome marker holds queue room forever (for example fix(planning): name the replacement in an archived, superseded interview question's banner #8087) when its newest label has no work-resolve-issue run, or only one that ended over 3 h ago with no report. The sweep now re-fires such an issue once per version of the lane workflow files. On a second strand it posts one comment and adds needs-human.

Fix

  • admit.mjs:
    • Export admissionFacts() (the fact reads admit() did inline, now shared), sayWhy, READY and NEEDS_DECISION.
    • One GraphQL read returns the body and the first 100 comments, each with its author, editor and edit time. It replaces the body-edit-time read; an unexpected shape throws before decide(), so nothing is written.
    • trustedSpecTexts() keeps only written() texts, and the task-list and unrunnable-item checks read those.
    • refinementsOf is removed. The refusal comment reuses main's refusalText (feat(pr-pipeline): file the split for a lane-unrunnable done-when at refine #9439).
    • openPullsNaming and openPullsFor are unchanged.
  • label-unblocked.mjs:
    • The marked path (admitMarked, issueVerdict) reads fields the way the lanes do (REST issue-field-values plus resolve-config fieldSetters), counting only values a listed human or the App set. An unset field reads unset, never the lanes' default.
    • On refuse it posts the admission comment, swaps status: ready for status: needs-decision, then removes admission-deferred. A wait on an open PR stays deferred; a wait on a blocker drops the deferral.
    • resolve-config is loaded lazily with a dynamic import (default base path .base), so the sweep needs no workflow change: the job's resolve-config step already installs its packages.
  • The stranded check reuses the existing re-fire record, <!-- work-unblock refired <key> -->, and its once-per-key counter, so no issue is re-fired by two rules. The hand-off has its own <!-- work-unblock stranded <key> --> marker, posted once per label add. Both share limits.restarts-per-run.
  • refine.mjs: the header comment now says its task list counts only once a listed human posts it.
  • .github/workflows/ is not touched.

Verification

  • node --test in .github/scripts/work-unblock: 143 pass, 0 fail.
    • New cases: admissible mark admitted within room; inadmissible mark refused with the comment, needs-decision and the deferral removed; App-only criteria on a mark refused; a bad-shape text read writes nothing; mark held by room left deferred; mark held while a PR closes or Refs the issue, admitted when one only mentions it; stranded issue re-fired; stranded issue already re-fired gets the comment and needs-human; a recent run or label left alone, with a 3 h boundary pair.
    • A refusal whose needs-decision write fails keeps status: ready, and the next sweep admits nothing.
    • A mark whose field values read in an unexpected shape writes nothing. (The same fail-closed fix for resolve-config fieldSetters was taken out because resolve-config is in pr-verify's takeover set; it moves to fix(pr-pipeline): apply-verdict readFields reads a malformed field-values answer as no fields set #9598.)
    • A mark admits only when a listed human or the App made the newest add of status: ready and of the work-class label.
    • A mark waiting on an open blocker loses admission-deferred with the admission comment.
    • A mark the rules skip loses admission-deferred with a comment saying why.
    • The stranded no-report clock starts when the run ended, not when it started.
    • A stranded issue whose re-fired run failed again over 3 h ago gets the comment and needs-human.
  • node --test in .github/scripts/intake-triage: 233 pass, 0 fail.
    • Q9 cases: App-only body refused; App task list plus an owner comment with no task list refused; an owner comment carrying the task list admits; a human body or comment last edited by the App does not count; App refinement alone refused; no spec fails closed.
    • admit() refuses to needs-decision, and writes nothing on a bad-shape text read.
    • admit() refusal whose needs-decision write fails removes no label.
    • An issue with more comments than the 100 the text read covers is refused, in decide() and in admit().
  • intake-triage + work-unblock + resolve-config suites together: 1063 pass, 0 fail (resolve-config unchanged by this PR).
  • scripts/check-affected.sh --tests: 10 passed, 4 skipped.

Left out: no live sweep run against GitHub; the admission and stranded paths are covered by the fake-API tests only.

Related

🤖 Generated with Claude Code

kyle-sexton and others added 7 commits October 24, 2026 00:09
… and re-fire stranded ones

A listed human adding admission-deferred to an issue with status: ready
now has work-unblock's hourly sweep run admit.mjs decide() on the issue's
own work-class label and Size, Complexity and Uncertainty fields, and
admit it within the room only on an admit verdict (owner decision Q75).

The sweep also re-fires an agent-ready issue with no run and no report
for 3 hours, once per version of the lane workflow files with the
existing re-fire record, then hands it to a person.

Admission now refuses an issue with no trusted-human acceptance criteria,
the only text pr-verify judges against (owner decision Q9, #8999).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The task-list check read the body plus the lanes App's refinement
comments, while the trusted-spec check passed on any listed human's
text. An App-drafted task list plus a short owner comment passed both,
and pr-verify then never saw the criteria.

decide() now reads the task list only from the body and comments a
listed human wrote and last edited (resolve-pr.mjs written()), under one
refusal. The spec read carries each text's body; refinement comments no
longer count for criterion 7.

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

# Conflicts:
#	.github/scripts/intake-triage/admit.mjs
#	.github/scripts/intake-triage/admit.test.mjs
…s open-PR check

Merges origin/main (c5a82db, #9560). The marked admission path and
the deferred sweep both read open pull requests through openPullsFor:
one GitHub says closes the issue, or whose body has its Refs line. A
pull request that only mentions the issue no longer holds a marked
admission; a closing one keeps it waiting and deferred.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- A refusal posts status: needs-decision before it removes status: ready,
  in the sweep and in intake, so a failed write never leaves an issue the
  sweep admits.
- A field-values or field-setters read of an unexpected shape throws
  instead of reading every dimension as unset.
- Intake refuses an issue with more comments than the 100 pr-verify reads.
- A marked issue's status: ready and work-class label count only when a
  listed human or the lanes App added them.
- The stranded no-report and failed-again clocks start when the run ended.
- A skip on a marked issue tells the person why with a comment.
- admissionFacts drops its unused appId; docs and comments match Q9.

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

chatgpt-codex-connector Bot commented Oct 24, 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-24T05:11:42.088994Z 78dc077 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 24, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review needs no new review of 84dd899676c3ae125d14a415fa232f986a031ebe: no file of this pull request changed since the review of dde7e3e.

I found no security issues in this pull request. I read the full diff (10 files) and the PR context, and checked .github/workflows/work-unblock.yml and the refireRefusal caller in label-unblocked.mjs. I did not run any code or tests. docs/conventions/instruction-exception-register/README.md was not consulted. The only removed instruction is the old rule that let the lanes App's refinement comment satisfy the task-list criterion, and that change narrows trust.

What I checked

  • Listed-human mark on admission-deferred: the new path runs only when the newest add of the label is by a listed human (by user id), the issue is open with status: ready, and no hold label is present. Every input to the admit verdict must be a trusted add or set.
    • Labels: the newest add of status: ready and of each work-class: label must come from a listed human or the lanes App. If not, the issue is skipped or the class is marked untrusted.
    • Fields: Size, Complexity and Uncertainty count only when a listed human or the App set them. Otherwise they read unset and the rules refuse. A malformed answer throws before any write.
    • Blockers: a closed blocker counts only through trustedClose.
    • Acceptance criteria: they must be in a listed human's own text. Text last edited by a non-listed actor or a bot does not count. An issue with more than 100 comments, or with no spec read, is refused.
  • Ordering of label writes on refusal: status: needs-decision is added first, then status: ready is removed, and admission-deferred goes last. A failed write leaves the issue held or still marked, never in a state the sweep would admit.
  • Re-firing stranded issues: the stranded path sits behind the existing trustedLabeler(lastAdd) check. It runs only when no lane PR is open, once per workflow key, and then hands off to needs-human. It shares the restart cap.
  • Config loading: the workflow runs .base/.github/scripts/work-unblock/label-unblocked.mjs from the trusted default-branch checkout. The lazy resolve-config import and the admission rules resolve from BASE_PATH (default .base), not from PR-controlled files.
  • fieldSetters change: it now throws on a missing issue or event list instead of reading it as unset, which fails closed.

manifest: not applicable. The diff changes no plugin.json or marketplace.json.

Trust-gate paths (trusted-actors/**, check-trusted-trigger/**, CODEOWNERS, .gitmodules) are not touched.

critical-findings: 0

@github-actions

github-actions Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

claude-review needs no new review of 84dd899676c3ae125d14a415fa232f986a031ebe: no file of this pull request changed since the review of dde7e3e.

Code review: PR #9566 (head dde7e3e)

No findings. I read the full 1650-line diff and the PR context. I also read the surrounding code in admit.mjs (decide), label-unblocked.mjs (refusal, readiness, HOLDS, refireRefusal) and refine.mjs, the createGitHub signature, and the resolve-config exports. I did not run any tests; the pass counts in the PR description are author-claimed and unverified by me.

What I checked, and found sound

  • Q9 trusted-criteria rule (admit.mjs).
    • trustedSpecTexts counts a text only when a listed User wrote it and any later editor is also a listed User.
    • Over 100 comments is refused.
    • A failed or malformed spec read throws before decide(), so nothing is written.
  • Marked path (admitMarked).
    • It is reached only when a listed human made the newest admission-deferred add, the issue has status: ready, and it carries no hold. HOLDS includes status: needs-decision, so a half-finished refusal leaves the issue held, not re-admitted.
    • fieldSetters now throws on a missing issue or event list, so a bad answer is never read as "unset".
    • createGitHub accepts the per-call headers that FIELD_API uses.
    • issueDimensionsFromBase, issueLaneWebHostsFromBase and fieldSetters are all exported from resolve-config.
  • Refusal write order. Both admit() and admitMarked now add needs-decision, then remove status: ready, then remove the deferral. A failure partway never leaves admission-deferred standing alone, and both orderings have tests.
  • refine.mjs. humans is in scope at the applyRefinement call site. The spec is read before any label write on both exact branches, and the new retriage flag only changes the footer text.
  • Stranded re-fire. It reuses the existing refired record and once-per-workflow-key bound. It checks for an open lane PR before acting, starts the no-report clock at run end, and shares restarts-per-run.
  • Tests. The new cases cover the 3 h boundary, the already-re-fired hand-off, a mark held by room, a mark waiting on a PR or blocker, and App-only criteria.

Manifests: no plugin.json or marketplace.json change; nothing to report. The change fragment is bump: minor for added behaviour.

Closing issue: the PR states no related issue (owner decision Q75). #8999, #8087, #9560 and #9439 appear only as Refs, with no closing-issue section, so I had no acceptance criteria to check.

Hold (cost): this PR does not add a model call directly, but it adds two automatic paths in the hourly work-unblock sweep that can start new work-resolve-issue agent runs:

  • A listed human's admission-deferred mark now admits an issue.
  • The stranded re-fire removes and re-adds agent-ready on an issue with no run for 3 hours.

Both are bounded by the admission room and limits.restarts-per-run, and the per-run bound is only as tight as the configured limit. A person should confirm that is the intended spend. This is not a defect finding.

kyle-sexton and others added 2 commits October 24, 2026 01:30
comment-hygiene read "issue 25" in a label-unblocked test comment as a
tracker reference and failed lint-repo.

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

@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 5dbb5eb6f92ba336eb5d86b551ddb27470697e14; the summary is in this lane's pull request comment.

claude-lane: claude-review

Comment thread .github/scripts/intake-triage/refine.mjs Outdated
Comment thread .github/actions/resolve-config/resolve-config.mjs
Comment thread .github/scripts/intake-triage/admit.mjs
Comment thread .github/scripts/work-unblock/label-unblocked.mjs
kyle-sexton and others added 2 commits October 24, 2026 01:41
Admission criterion 7 reads the task list only from a listed human's text
(owner decision Q9), so re-triaging an exact refinement whose only task
list is the lanes App's comment always refused again and re-added
status: needs-decision. refine.mjs now asks for the re-triage, on both
the App-needs-decision and the status: ready branches, only when
admit.mjs hasTrustedCriteria (the test decide() uses) finds one in the
spec specOf reads; otherwise the labels stay and the comment asks a
listed human to post the criteria as their own decision comment. A
malformed spec read throws before any label write.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added the needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. label Oct 24, 2026
@github-actions

Copy link
Copy Markdown
Contributor

claude-review holds this pull request for a person (cost) at dde7e3e3d12ee136c72a5545412d46ec307ea582.

.github/scripts/work-unblock/label-unblocked.mjs adds two new automatic triggers in the hourly sweep that can start work-resolve-issue agent runs. A listed human's admission-deferred mark now admits a decided issue (admitMarked). A stranded agent-ready issue with no run, or no report, for 3 hours (STRANDED_HOURS) is re-fired once per workflow-file version. Both are bounded by the admission room and limits.restarts-per-run, but they raise agent-run spend, so a person should confirm the intended cost.

claude-lane: claude-review

…k change

pr-verify fails any change to the resolve-config action, which is in its
takeover set, whatever the spec says. Restore resolve-config.mjs and
models.test.mjs to main; the fieldSetters fail-closed tightening moves to
its own issue. The label-unblocked field-values check stays.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton

Copy link
Copy Markdown
Member Author

Cost hold lifted (Issue Triage session). Basis: the owner's Q75 flow design (2026-10-24), recorded by the GitHub Support hub. The owner's mark (admission-deferred on a status: ready issue) is admitted by work-unblock's hourly sweep under the Q51 admission rules, within check-wip-cap room. A stranded agent-ready issue gets one re-fire (the shared #9537 counter), then a comment and needs-human.

The two triggers claude-review named are that design. They admit within the existing WIP 12 / held 10 caps and replace the manual admissions admit.sh does today, so they add no new kind of charge. The stranded re-fire is capped at one per issue per version of the lane workflow files. The decision reached this session through the hub's relay, which the owner's merge policy accepts ("a peer's report of that decision suffices").

@kyle-sexton kyle-sexton removed the needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. label Oct 24, 2026
@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 24, 2026
Merged via the queue into main with commit 5589b26 Oct 24, 2026
89 of 93 checks passed
@kyle-sexton
kyle-sexton deleted the feat/hitl-admission-mark-stranded-check branch October 24, 2026 06:12
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