-
Notifications
You must be signed in to change notification settings - Fork 14.4k
fix(adapter): force fresh session on status-only recovery; gate issue-comment POST on deliverable-mutation guard (SPA-2166 / SPA-1449 defects 4+5) #11084
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -335,18 +335,38 @@ type SuccessfulRunHandoffActivityRow = { | |
| type TaskWatchdogService = ReturnType<typeof taskWatchdogService>; | ||
| type TaskWatchdogServiceFactory = typeof taskWatchdogService; | ||
|
|
||
| // When a request_confirmation (or related interaction) is rejected with a | ||
| // build-worthy reason, downstream callers spawn a follow-up child issue to | ||
| // action the request. Without explicit status/assignee, those children land | ||
| // in `backlog` and never get picked up. This heuristic flags rejection | ||
| // reasons that imply the assignee should act on them right now. | ||
| function isBuildWorthyRejectionReason(reason: string): boolean { | ||
| if (typeof reason !== "string" || reason.length === 0) return false; | ||
| return /\b(?:fix|review|build|implement|developer|address|resolve)\b/i.test(reason); | ||
| } | ||
|
Comment on lines
+338
to
+346
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The description calls this a two-line patch, but the diff also changes issue status and parent-assignment defaults. Please use the required PR template and add the Thinking Path, complete change rationale and benefits, verification and risks, Model Used, and checklist so reviewers can assess the full behavior. Context Used: CONTRIBUTING.md has a guide for a good PR message ... (source) Prompt To Fix With AIThis is a comment left during a code review.
Path: server/src/routes/issues.ts
Line: 338-346
Comment:
**PR description omits changed behavior**
The description calls this a two-line patch, but the diff also changes issue status and parent-assignment defaults. Please use the required PR template and add the Thinking Path, complete change rationale and benefits, verification and risks, Model Used, and checklist so reviewers can assess the full behavior.
**Context Used:** CONTRIBUTING.md has a guide for a good PR message ... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
||
|
|
||
| function applyCreateIssueStatusDefault(req: Request, res: Response, next: () => void) { | ||
| if (!req.body || typeof req.body !== "object" || Array.isArray(req.body)) { | ||
| next(); | ||
| return; | ||
| } | ||
|
|
||
| const resolution = resolveCreateIssueStatusDefault(req.body as Record<string, unknown>); | ||
| const body = req.body as Record<string, unknown>; | ||
| const resolution = resolveCreateIssueStatusDefault(body); | ||
| res.locals.createIssueStatusDefault = resolution; | ||
| let defaultedStatus: string | null = null; | ||
| if (resolution.defaulted) { | ||
| const textHaystack = [body.title, body.description] | ||
| .filter((value): value is string => typeof value === "string") | ||
| .join(" "); | ||
| if (resolution.status === "backlog" && isBuildWorthyRejectionReason(textHaystack)) { | ||
| defaultedStatus = "todo"; | ||
| } else { | ||
| defaultedStatus = resolution.status; | ||
| } | ||
| req.body = { | ||
| ...req.body, | ||
| status: resolution.status, | ||
| status: defaultedStatus, | ||
| }; | ||
| } | ||
| next(); | ||
|
|
@@ -7893,6 +7913,21 @@ export function issueRoutes( | |
| ...sanitizedBody, | ||
| ...(normalizedAssigneeAgentId !== undefined ? { assigneeAgentId: normalizedAssigneeAgentId } : {}), | ||
| }; | ||
| // Parent assignee fallback: when a request_confirmation rejection spawns | ||
| // a follow-up child and the caller didn't pass an assignee, default the | ||
| // child to the parent's assignee so the build-worthy follow-up surfaces | ||
| // in their inbox instead of sitting unassigned in backlog. | ||
| const fallbackAssigneeAgentId = | ||
| (createBody as { assigneeAgentId?: string | null }).assigneeAgentId == null | ||
| && (createBody as { assigneeUserId?: string | null }).assigneeUserId == null | ||
| && parent.assigneeAgentId != null | ||
| && isBuildWorthyRejectionReason( | ||
| [createBody.title, createBody.description] | ||
| .filter((value): value is string => typeof value === "string") | ||
| .join(" "), | ||
| ) | ||
| ? parent.assigneeAgentId | ||
| : null; | ||
| if (!(await assertCheapRecoveryIssueAssigneeProfileAllowed(req, res, parent, createBody))) return; | ||
| const childAssignmentScope = { | ||
| projectId: createBody.projectId ?? parent.projectId ?? null, | ||
|
|
@@ -7926,6 +7961,7 @@ export function issueRoutes( | |
| const { issue, parentBlockerAdded } = await svc.createChild(parent.id, { | ||
| ...createBody, | ||
| ...(taskBridgeOriginForActor(req) ?? {}), | ||
| ...(fallbackAssigneeAgentId != null ? { assigneeAgentId: fallbackAssigneeAgentId } : {}), | ||
| id: issueId, | ||
| executionPolicy, | ||
| ...(currentSerializedChild | ||
|
|
@@ -10975,6 +11011,7 @@ export function issueRoutes( | |
| } | ||
| const commentAccessDecision = await assertAgentIssueCommentAllowed(req, res, issue); | ||
| if (!commentAccessDecision) return; | ||
| if (!(await assertDeliverableMutationAllowedByRunContext(req, res, issue))) return; | ||
| const commentAuthorizationReason = issueWriteAuthorizationReason(req, commentAccessDecision); | ||
| if (!assertStructuredCommentFieldsAllowed(req, res, { | ||
| presentation: req.body.presentation, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When an ordinary issue or child description contains a word such as
fix,review, orbuild, this general-purpose heuristic promotes the issue frombacklogtotodo; for children it also inherits the parent agent, causing that agent to be woken for work the caller neither activated nor assigned. Restrict this behavior to verified request-confirmation rejection follow-ups.Knowledge Base Used: Issues and Pipelines Flow
Prompt To Fix With AI
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!