Merge pull request #2211 from deepseek-harness/worktree/event-driven-issue-review-status-20260810
feat: direct Issue status from PR review events
This commit is contained in:
@@ -3,6 +3,7 @@
|
||||
"repository": "deepseek-harness",
|
||||
"projectNumber": 1,
|
||||
"projectTitle": "DSH Issue Management",
|
||||
"lifecycleActor": "dsh-issue-management",
|
||||
"priorityField": "Priority",
|
||||
"allowUnassignedOwner": true,
|
||||
"statuses": [
|
||||
|
||||
@@ -37,10 +37,21 @@ const LEGACY_LABELS = new Set([
|
||||
])
|
||||
const TERMINAL_STATUSES = new Set(['Done', 'No action'])
|
||||
const ACTIVE_STATUS_ORDER = config.statuses.filter((status) => !TERMINAL_STATUSES.has(status))
|
||||
const IMPLEMENTATION_PULL_REQUEST_ACTIONS = new Set([
|
||||
'opened',
|
||||
'edited',
|
||||
'synchronize',
|
||||
'reopened',
|
||||
'labeled',
|
||||
'unlabeled',
|
||||
])
|
||||
|
||||
for (const status of ['In progress', 'In review']) {
|
||||
if (!ACTIVE_STATUS_ORDER.includes(status)) throw new Error(`config.statuses 缺少 ${status}`)
|
||||
}
|
||||
if (typeof config.lifecycleActor !== 'string' || !config.lifecycleActor) {
|
||||
throw new Error('config.lifecycleActor 未设置')
|
||||
}
|
||||
|
||||
/**
|
||||
* Return Markdown outside balanced details elements.
|
||||
@@ -159,18 +170,48 @@ export function requiresPullRequestPolicy({
|
||||
}
|
||||
|
||||
/**
|
||||
* Derive a forward-only Issue status from the current PR phase.
|
||||
* @param {string|null} currentStatus Current Project status.
|
||||
* @param {{isDraft: boolean, reviewRequestCount: number, reviewCount: number}} pull PR phase.
|
||||
* @returns {string|null} Status to write, or null when no forward transition exists.
|
||||
* Translate a repository event into one resolving-Issue lifecycle command.
|
||||
* @param {string} eventName GitHub event name.
|
||||
* @param {{action?: string, review?: {state?: string}}} event GitHub event payload.
|
||||
* @returns {'implementation'|'review-requested'|'changes-requested'|null} Lifecycle command.
|
||||
*/
|
||||
export function nextResolvingIssueStatus(currentStatus, pull) {
|
||||
const target =
|
||||
!pull.isDraft && (pull.reviewRequestCount > 0 || pull.reviewCount > 0)
|
||||
? 'In review'
|
||||
: 'In progress'
|
||||
export function resolvingIssueStatusCommand(eventName, event) {
|
||||
if (eventName === 'pull_request') {
|
||||
if (event.action === 'review_requested') return 'review-requested'
|
||||
return IMPLEMENTATION_PULL_REQUEST_ACTIONS.has(event.action) ? 'implementation' : null
|
||||
}
|
||||
if (
|
||||
eventName === 'pull_request_review' &&
|
||||
event.action === 'submitted' &&
|
||||
event.review?.state?.toLowerCase() === 'changes_requested'
|
||||
) {
|
||||
return 'changes-requested'
|
||||
}
|
||||
return null
|
||||
}
|
||||
|
||||
/**
|
||||
* Plan one event-directed resolving-Issue status transition.
|
||||
* @param {string|null} currentStatus Current Project status.
|
||||
* @param {'implementation'|'review-requested'|'changes-requested'} command Lifecycle command.
|
||||
* @param {string|null} currentStatusActor Actor that last set the current Project status.
|
||||
* @returns {string|null} Status to write, or null when no permitted transition exists.
|
||||
*/
|
||||
export function nextResolvingIssueStatus(currentStatus, command, currentStatusActor = null) {
|
||||
let target
|
||||
if (command === 'review-requested') target = 'In review'
|
||||
else if (command === 'implementation' || command === 'changes-requested') target = 'In progress'
|
||||
else throw new Error(`未知 lifecycle command:${command}`)
|
||||
|
||||
const currentIndex = ACTIVE_STATUS_ORDER.indexOf(currentStatus)
|
||||
const targetIndex = ACTIVE_STATUS_ORDER.indexOf(target)
|
||||
if (
|
||||
command === 'changes-requested' &&
|
||||
currentStatus === 'In review' &&
|
||||
currentStatusActor === config.lifecycleActor
|
||||
) {
|
||||
return target
|
||||
}
|
||||
return currentIndex >= 0 && currentIndex < targetIndex ? target : null
|
||||
}
|
||||
|
||||
@@ -396,9 +437,15 @@ async function issueSnapshot(number, status = undefined) {
|
||||
}
|
||||
}
|
||||
|
||||
async function projectContext(number) {
|
||||
async function projectContext(number, includeStatusActor = false) {
|
||||
const data = await graphql(
|
||||
`query($organization: String!, $repository: String!, $number: Int!, $project: Int!) {
|
||||
`query(
|
||||
$organization: String!
|
||||
$repository: String!
|
||||
$number: Int!
|
||||
$project: Int!
|
||||
$includeStatusActor: Boolean!
|
||||
) {
|
||||
organization(login: $organization) {
|
||||
projectV2(number: $project) {
|
||||
id
|
||||
@@ -413,6 +460,16 @@ async function projectContext(number) {
|
||||
repository(owner: $organization, name: $repository) {
|
||||
issue(number: $number) {
|
||||
id
|
||||
timelineItems(last: 100, itemTypes: [PROJECT_V2_ITEM_STATUS_CHANGED_EVENT])
|
||||
@include(if: $includeStatusActor) {
|
||||
nodes {
|
||||
... on ProjectV2ItemStatusChangedEvent {
|
||||
actor { login }
|
||||
project { id }
|
||||
status
|
||||
}
|
||||
}
|
||||
}
|
||||
projectItems(first: 20, includeArchived: true) {
|
||||
nodes {
|
||||
id
|
||||
@@ -430,6 +487,7 @@ async function projectContext(number) {
|
||||
repository: config.repository,
|
||||
number,
|
||||
project: config.projectNumber,
|
||||
includeStatusActor,
|
||||
},
|
||||
)
|
||||
const project = data.organization?.projectV2
|
||||
@@ -439,7 +497,14 @@ async function projectContext(number) {
|
||||
const statusField = project.fields.nodes.find((field) => field?.name === 'Status')
|
||||
if (!statusField) throw new Error('Project 缺少 Status 字段')
|
||||
const item = issue.projectItems.nodes.find((candidate) => candidate.project.id === project.id)
|
||||
return { project, issue, statusField, item }
|
||||
const latestStatusEvent = issue.timelineItems?.nodes
|
||||
?.filter((event) => event?.project?.id === project.id)
|
||||
.at(-1)
|
||||
const statusActor =
|
||||
latestStatusEvent?.status === item?.fieldValueByName?.name
|
||||
? (latestStatusEvent.actor?.login ?? null)
|
||||
: null
|
||||
return { project, issue, statusField, item, statusActor }
|
||||
}
|
||||
|
||||
async function projectStatus(number) {
|
||||
@@ -530,12 +595,7 @@ async function auditIssue(number, extraErrors = [], status = undefined) {
|
||||
return errors
|
||||
}
|
||||
|
||||
async function pullRequestSnapshot(number) {
|
||||
const pull = await api(`/repos/${config.organization}/${config.repository}/pulls/${number}`)
|
||||
const [reviewRequests, reviews] = await Promise.all([
|
||||
api(`/repos/${config.organization}/${config.repository}/pulls/${number}/requested_reviewers`),
|
||||
api(`/repos/${config.organization}/${config.repository}/pulls/${number}/reviews?per_page=100`),
|
||||
])
|
||||
async function resolvingReferencesSnapshot(number, pull) {
|
||||
const references = parseReferences({
|
||||
body: pull.body ?? '',
|
||||
repository: `${config.organization}/${config.repository}`,
|
||||
@@ -547,20 +607,41 @@ async function pullRequestSnapshot(number) {
|
||||
}
|
||||
return {
|
||||
number,
|
||||
isDraft: pull.draft,
|
||||
authorType: pull.user?.type ?? 'User',
|
||||
reviewRequestCount: reviewRequests.users.length + reviewRequests.teams.length,
|
||||
reviewCount: reviews.length,
|
||||
labels: pull.labels.map((label) => label.name),
|
||||
references: retainIssueReferences(references, issues),
|
||||
issues,
|
||||
}
|
||||
}
|
||||
|
||||
async function advanceResolvingIssues(pull) {
|
||||
async function pullRequestSnapshot(number) {
|
||||
const [pull, reviewRequests, reviews] = await Promise.all([
|
||||
api(`/repos/${config.organization}/${config.repository}/pulls/${number}`),
|
||||
api(`/repos/${config.organization}/${config.repository}/pulls/${number}/requested_reviewers`),
|
||||
api(`/repos/${config.organization}/${config.repository}/pulls/${number}/reviews?per_page=100`),
|
||||
])
|
||||
const resolving = await resolvingReferencesSnapshot(number, pull)
|
||||
return {
|
||||
...resolving,
|
||||
isDraft: pull.draft,
|
||||
authorType: pull.user?.type ?? 'User',
|
||||
reviewRequestCount: reviewRequests.users.length + reviewRequests.teams.length,
|
||||
reviewCount: reviews.length,
|
||||
labels: pull.labels.map((label) => label.name),
|
||||
}
|
||||
}
|
||||
|
||||
async function lifecyclePullRequestSnapshot(number) {
|
||||
const pull = await api(`/repos/${config.organization}/${config.repository}/pulls/${number}`)
|
||||
return resolvingReferencesSnapshot(number, pull)
|
||||
}
|
||||
|
||||
async function transitionResolvingIssues(pull, command) {
|
||||
for (const number of pull.references.resolving) {
|
||||
const context = await projectContext(number)
|
||||
const target = nextResolvingIssueStatus(context.item?.fieldValueByName?.name ?? null, pull)
|
||||
const context = await projectContext(number, command === 'changes-requested')
|
||||
const target = nextResolvingIssueStatus(
|
||||
context.item?.fieldValueByName?.name ?? null,
|
||||
command,
|
||||
context.statusActor,
|
||||
)
|
||||
if (!target) continue
|
||||
// TODO: Replace this latest-state guard with per-Issue serialization or a
|
||||
// conditional ProjectV2 update; GraphQL currently has no compare-and-swap.
|
||||
@@ -598,8 +679,10 @@ async function runLifecycle(eventName, event) {
|
||||
}
|
||||
|
||||
if (eventName === 'pull_request' || eventName === 'pull_request_review') {
|
||||
const pull = await pullRequestSnapshot(event.pull_request.number)
|
||||
await advanceResolvingIssues(pull)
|
||||
const command = resolvingIssueStatusCommand(eventName, event)
|
||||
if (!command) return
|
||||
const pull = await lifecyclePullRequestSnapshot(event.pull_request.number)
|
||||
await transitionResolvingIssues(pull, command)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -6,6 +6,7 @@ import {
|
||||
nextResolvingIssueStatus,
|
||||
parseReferences,
|
||||
retainIssueReferences,
|
||||
resolvingIssueStatusCommand,
|
||||
requiresPullRequestPolicy,
|
||||
validateBody,
|
||||
validateIssue,
|
||||
@@ -243,32 +244,73 @@ test('requires policy only after a human PR enters review', () => {
|
||||
)
|
||||
})
|
||||
|
||||
test('advances resolving Issues to the live PR phase', () => {
|
||||
const draft = { isDraft: true, reviewRequestCount: 1, reviewCount: 4 }
|
||||
const open = { isDraft: false, reviewRequestCount: 0, reviewCount: 0 }
|
||||
const requestedReview = { isDraft: false, reviewRequestCount: 1, reviewCount: 0 }
|
||||
const submittedReview = { isDraft: false, reviewRequestCount: 0, reviewCount: 1 }
|
||||
|
||||
for (const status of ['Inbox', 'Backlog', 'Ready']) {
|
||||
assert.equal(nextResolvingIssueStatus(status, draft), 'In progress')
|
||||
assert.equal(nextResolvingIssueStatus(status, open), 'In progress')
|
||||
assert.equal(nextResolvingIssueStatus(status, requestedReview), 'In review')
|
||||
assert.equal(nextResolvingIssueStatus(status, submittedReview), 'In review')
|
||||
test('maps only explicit review handoffs to review status commands', () => {
|
||||
assert.equal(
|
||||
resolvingIssueStatusCommand('pull_request', {
|
||||
action: 'review_requested',
|
||||
}),
|
||||
'review-requested',
|
||||
)
|
||||
assert.equal(
|
||||
resolvingIssueStatusCommand('pull_request_review', {
|
||||
action: 'submitted',
|
||||
review: { state: 'changes_requested' },
|
||||
}),
|
||||
'changes-requested',
|
||||
)
|
||||
for (const state of ['approved', 'commented']) {
|
||||
assert.equal(
|
||||
resolvingIssueStatusCommand('pull_request_review', {
|
||||
action: 'submitted',
|
||||
review: { state },
|
||||
}),
|
||||
null,
|
||||
)
|
||||
}
|
||||
assert.equal(nextResolvingIssueStatus('In progress', requestedReview), 'In review')
|
||||
assert.equal(nextResolvingIssueStatus('In progress', submittedReview), 'In review')
|
||||
assert.equal(
|
||||
resolvingIssueStatusCommand('pull_request_review', {
|
||||
action: 'dismissed',
|
||||
review: { state: 'changes_requested' },
|
||||
}),
|
||||
null,
|
||||
)
|
||||
})
|
||||
|
||||
test('never regresses or reopens a resolving Issue', () => {
|
||||
const implementation = { isDraft: false, reviewRequestCount: 0, reviewCount: 0 }
|
||||
const review = { isDraft: false, reviewRequestCount: 0, reviewCount: 1 }
|
||||
test('keeps ordinary pull request events as forward-only implementation signals', () => {
|
||||
for (const action of ['opened', 'edited', 'synchronize', 'reopened', 'labeled', 'unlabeled']) {
|
||||
assert.equal(resolvingIssueStatusCommand('pull_request', { action }), 'implementation')
|
||||
}
|
||||
assert.equal(
|
||||
resolvingIssueStatusCommand('pull_request', { action: 'review_request_removed' }),
|
||||
null,
|
||||
)
|
||||
})
|
||||
|
||||
assert.equal(nextResolvingIssueStatus('In progress', implementation), null)
|
||||
assert.equal(nextResolvingIssueStatus('In review', implementation), null)
|
||||
assert.equal(nextResolvingIssueStatus('In review', review), null)
|
||||
assert.equal(nextResolvingIssueStatus('Done', review), null)
|
||||
assert.equal(nextResolvingIssueStatus('No action', review), null)
|
||||
assert.equal(nextResolvingIssueStatus(null, review), null)
|
||||
test('toggles automation-owned work on request changes and repeated review request', () => {
|
||||
for (const status of ['Inbox', 'Backlog', 'Ready']) {
|
||||
assert.equal(nextResolvingIssueStatus(status, 'implementation'), 'In progress')
|
||||
assert.equal(nextResolvingIssueStatus(status, 'review-requested'), 'In review')
|
||||
assert.equal(nextResolvingIssueStatus(status, 'changes-requested'), 'In progress')
|
||||
}
|
||||
let status = nextResolvingIssueStatus(
|
||||
'In review',
|
||||
'changes-requested',
|
||||
'dsh-issue-management',
|
||||
)
|
||||
assert.equal(status, 'In progress')
|
||||
status = nextResolvingIssueStatus(status, 'review-requested')
|
||||
assert.equal(status, 'In review')
|
||||
})
|
||||
|
||||
test('preserves human review status and terminal Issues', () => {
|
||||
assert.equal(nextResolvingIssueStatus('In progress', 'implementation'), null)
|
||||
assert.equal(nextResolvingIssueStatus('In review', 'implementation'), null)
|
||||
assert.equal(nextResolvingIssueStatus('In review', 'review-requested'), null)
|
||||
assert.equal(nextResolvingIssueStatus('In review', 'changes-requested', 'tianyicui'), null)
|
||||
assert.equal(nextResolvingIssueStatus('In review', 'changes-requested'), null)
|
||||
assert.equal(nextResolvingIssueStatus('Done', 'review-requested'), null)
|
||||
assert.equal(nextResolvingIssueStatus('No action', 'changes-requested'), null)
|
||||
assert.equal(nextResolvingIssueStatus(null, 'review-requested'), null)
|
||||
})
|
||||
|
||||
test('keeps lifecycle projection independent of PR metadata enforcement', () => {
|
||||
@@ -283,7 +325,7 @@ test('keeps lifecycle projection independent of PR metadata enforcement', () =>
|
||||
}
|
||||
|
||||
assert.ok(validatePullRequest(pull).length > 0)
|
||||
assert.equal(nextResolvingIssueStatus('Inbox', pull), 'In review')
|
||||
assert.equal(nextResolvingIssueStatus('Inbox', 'review-requested'), 'In review')
|
||||
})
|
||||
|
||||
test('exempts Draft, Bot, and App PRs', () => {
|
||||
|
||||
@@ -36,6 +36,7 @@ concurrency:
|
||||
jobs:
|
||||
lifecycle:
|
||||
name: Issue lifecycle
|
||||
if: ${{ github.event_name != 'pull_request_review' || (github.event.action == 'submitted' && github.event.review.state == 'changes_requested') }}
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- name: Check out trusted policy
|
||||
|
||||
Reference in New Issue
Block a user