From 200c3828f8053419fd7ec9a0c1881b6f85c1cae0 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Fri, 13 Nov 2026 17:43:17 +0000 Subject: [PATCH 1/4] feat(github): Keep drafts with code owners until the requester reviews Co-Authored-By: Burak Yigit Kaya <126780+BYK@users.noreply.github.com> --- .../evals/coding/subscriptions.eval.ts | 28 +++- packages/junior-github/package.json | 1 + .../junior-github/skills/github-code/SKILL.md | 2 +- .../junior-github/skills/github-code/SPEC.md | 2 +- .../skills/github-code/references/workflow.md | 2 +- packages/junior-github/src/code-owners.ts | 132 ++++++++++++++++++ .../junior-github/src/events/pull-request.ts | 8 +- .../junior-github/src/graphql-documents.ts | 28 ++++ .../src/tools/get-pull-request.ts | 18 ++- .../tests/get-pull-request.test.ts | 72 ++++++++++ .../junior-testing/src/http/github-drafts.ts | 20 ++- pnpm-lock.yaml | 3 + 12 files changed, 305 insertions(+), 11 deletions(-) create mode 100644 packages/junior-github/src/code-owners.ts diff --git a/packages/junior-evals/evals/coding/subscriptions.eval.ts b/packages/junior-evals/evals/coding/subscriptions.eval.ts index d455100f68..855640608e 100644 --- a/packages/junior-evals/evals/coding/subscriptions.eval.ts +++ b/packages/junior-evals/evals/coding/subscriptions.eval.ts @@ -83,13 +83,14 @@ function checkSuiteWebhook(conclusion: "failure" | "success") { // The shared GitHub HTTP fixture serves these drafts: #692 with every // required check passing, #693 with an open decision in its body, #694 -// with another required check failing, and #695 with no required check -// reported yet. +// with another required check failing, #695 with no required check +// reported yet, and #696 with code owners for its changed file. const draftHeadShas = { 692: "6920000000000000000000000000000000000692", 693: "6930000000000000000000000000000000000693", 694: "6940000000000000000000000000000000000694", 695: "6950000000000000000000000000000000000695", + 696: "6960000000000000000000000000000000000696", }; type DraftNumber = keyof typeof draftHeadShas; @@ -371,6 +372,29 @@ describe("Watches", () => { expect(delivery.replies).toHaveLength(1); }); + test("when the checks of a Junior draft with code owners pass, keep it a draft and tell the requester", async ({ + run, + }) => { + const conversation = await startDraftThread(run, 696); + + const delivery = await conversation.continue(draftChecksPassedWebhook(696)); + await expect(delivery).toSatisfyJudge( + RubricJudge, + rubric({ + pass: [ + "The reply says the required checks of getsentry/junior#696 pass and the draft waits for the requester's review.", + "The reply names @getsentry/cache-owners as the code owners that marking it ready asks to review.", + ], + fail: [ + "Do not say that getsentry/junior#696 is ready for review or was marked ready.", + ], + }), + ); + + expect(markReadyCalls(delivery)).toEqual([]); + expect(delivery.replies).toHaveLength(1); + }); + test("when a Junior draft names an open decision, keep it a draft", async ({ run, }) => { diff --git a/packages/junior-github/package.json b/packages/junior-github/package.json index b4d185ca69..30dc22f1f9 100644 --- a/packages/junior-github/package.json +++ b/packages/junior-github/package.json @@ -41,6 +41,7 @@ "@sentry/junior-plugin-api": "workspace:*", "@sinclair/typebox": "^0.34.49", "drizzle-orm": "catalog:", + "ignore": "^7.0.5", "marked": "^18.1.2", "mdast-util-from-markdown": "^2.0.3", "mdast-util-gfm": "^3.1.0", diff --git a/packages/junior-github/skills/github-code/SKILL.md b/packages/junior-github/skills/github-code/SKILL.md index 7ada02f981..d77182d902 100644 --- a/packages/junior-github/skills/github-code/SKILL.md +++ b/packages/junior-github/skills/github-code/SKILL.md @@ -37,7 +37,7 @@ Use `git` and `gh` for repository work. - When an error names a tool, use it. Change permissions only when GitHub rejected the request. - Stop for an unclear target, missing access, destructive work, or a GitHub permission failure. - Code changes always end in a pushed branch and a PR. Do not ask whether to open one. Skip it only when the user opts out or asks for investigation only. -- Open the PR ready for review when the work is complete and the checks pass. Open a draft when work, checks, or decisions remain. A scope or follow-up question is not a reason to stop: open the PR, and name the question in the PR body and the report. Name each open decision of a draft in its PR body: when the required checks pass, the Watch marks a draft with no open decision ready for review. +- Open the PR ready for review when the work is complete and the checks pass. Open a draft when work, checks, or decisions remain, or when the repo has a `CODEOWNERS` file in `.github/`, the root, or `docs/` and the requester did not ask for a ready PR: opening it ready asks the code owners to review. A scope or follow-up question is not a reason to stop: open the PR, and name the question in the PR body and the report. Name each open decision of a draft in its PR body: when the required checks pass, the Watch marks a draft with no open decision ready for review. A draft whose changed files have code owners stays a draft, and the Watch tells the requester that it waits for their review. - Link the PR. Mention checks only when they failed or were skipped. - The runtime adds the requester `via` line. Do not write one. - Credit other people, such as a reporter, by GitHub `@login` so GitHub notifies them. Get the login from `userLookup` (`linked_identities` with provider `github`, else a GitHub link in `profile_fields`). Pass the Slack user ID when known. With no login, write the name. Never guess a login. When you delegate the write, put the `@login` in the brief. diff --git a/packages/junior-github/skills/github-code/SPEC.md b/packages/junior-github/skills/github-code/SPEC.md index 73885aff0f..89bc73b687 100644 --- a/packages/junior-github/skills/github-code/SPEC.md +++ b/packages/junior-github/skills/github-code/SPEC.md @@ -10,7 +10,7 @@ Guide evidence-first GitHub repository work from inspection through a reviewable - Preserve unrelated work; reject destructive Git operations except `--force-with-lease` pushes to branches Junior pushed or the requester named. - Treat shallow clones as inspection checkouts; deepen before history work; never force-push around missing ancestry. - Install dependencies with the lockfile frozen mode before verification when needed. -- Finish every code change with a pushed PR, without asking, unless the user opts out or asked for investigation only. Open it ready when work and checks are done; open a draft when work, checks, or decisions remain. An open scope question goes in the PR body, not in a stop. The Watch of a draft marks it ready when its required checks pass and no decision is open, so open decisions of a draft go in its PR body. +- Finish every code change with a pushed PR, without asking, unless the user opts out or asked for investigation only. Open it ready when work and checks are done; open a draft when work, checks, or decisions remain, or when the repo has a CODEOWNERS file and the requester did not ask for a ready PR. An open scope question goes in the PR body, not in a stop. The Watch of a draft marks it ready when its required checks pass and no decision is open, so open decisions of a draft go in its PR body. A draft whose changed files have CODEOWNERS owners stays a draft unless the requester said to mark it ready, and Junior tells the requester that it waits for their review. - After each push to a branch with an open PR, check mergeability. Resolve conflicts without asking, by merging the PR base branch (usually the default branch). Never rebase for this, except in a stack: a stack merges only with a linear history, so stacked PRs restack with `gh stack`. GitHub sends no event for a new conflict, and conflicted PRs get no new checks. The plugin publishes `pull_request.merge_conflict` for a watched PR only when another webhook event arrives for it, so the post-push check stays. - Write conventional PR titles and short plain-English bodies; keep check results out of the PR body. - Report exact validation and permission failures. diff --git a/packages/junior-github/skills/github-code/references/workflow.md b/packages/junior-github/skills/github-code/references/workflow.md index 6625f43757..564df9cdc3 100644 --- a/packages/junior-github/skills/github-code/references/workflow.md +++ b/packages/junior-github/skills/github-code/references/workflow.md @@ -42,7 +42,7 @@ Open this when making repository edits, not for read-only inspection. - Not in a stack: GitHub checks mergeability against the PR base (`baseRefName`), which is usually the default branch. Fetch the base and merge `origin/BASE` into the PR branch. Do not rebase. Resolve each conflict so that both the PR change and the base branch change stay intact. Run the checks again and push. - Then check mergeability again. 4. Resolve the actual default branch. -5. Update an existing PR for the branch with `github_updatePullRequest`, or create with `github_createPullRequest` (`draft: true` only when work, checks, or decisions remain; otherwise ready for review). +5. Update an existing PR for the branch with `github_updatePullRequest`, or create with `github_createPullRequest` (`draft: true` when work, checks, or decisions remain, or when the repo has a `CODEOWNERS` file and the requester did not ask for a ready PR; otherwise ready for review). 6. PR title matches the current dominant change, same conventional form as commits. 7. PR body: short, plain English, what changed and why. Only context the diff cannot show. No empty sections, file lists, commit logs, or Checks/Verification/Test plan blocks. 8. After material follow-up commits, refresh title and body against the current diff. diff --git a/packages/junior-github/src/code-owners.ts b/packages/junior-github/src/code-owners.ts new file mode 100644 index 0000000000..dc88766855 --- /dev/null +++ b/packages/junior-github/src/code-owners.ts @@ -0,0 +1,132 @@ +/** + * The code owners that GitHub asks to review a pull request. + * + * GitHub reads the first CODEOWNERS file of the base branch, in `.github/`, + * the repository root, or `docs/`. For each changed file the last matching + * rule wins, and a matching rule with no owners leaves the file without + * owners. GitHub asks the owners to review when the pull request is ready for + * review, so a draft has not asked them yet. + * + * CODEOWNERS uses gitignore patterns, which the `ignore` package matches. + * One difference: GitHub matches `docs/*` only to files directly in `docs/`, + * and gitignore also matches the files below. That error finds too many + * owners, never too few, so Junior does not ping owners it did not expect. + */ +import type { PluginEgress } from "@sentry/junior-plugin-api"; +import ignore from "ignore"; +import * as z from "zod"; +import { PULL_REQUEST_CODE_OWNERS } from "./graphql-documents.js"; +import { githubApi } from "./tool-support/github-api.js"; + +interface CodeOwnersRule { + matches(path: string): boolean; + owners: string[]; +} + +/** Parse the rules of one CODEOWNERS file, in file order. */ +function parseCodeOwners(text: string): CodeOwnersRule[] { + const rules: CodeOwnersRule[] = []; + for (const line of text.split(/\r?\n/)) { + const [pattern, ...tokens] = line.trim().split(/\s+/); + if (!pattern || pattern.startsWith("#")) continue; + const comment = tokens.findIndex((token) => token.startsWith("#")); + const matcher = ignore().add(pattern); + rules.push({ + matches: (path) => matcher.ignores(path), + owners: comment === -1 ? tokens : tokens.slice(0, comment), + }); + } + return rules; +} + +/** Return the sorted owners of the given paths. */ +function codeOwnersOf(rules: CodeOwnersRule[], paths: string[]): string[] { + const owners = new Set(); + for (const path of paths) { + let last: CodeOwnersRule | undefined; + for (const rule of rules) if (rule.matches(path)) last = rule; + for (const owner of last?.owners ?? []) owners.add(owner); + } + return [...owners].sort(); +} + +// GitHub uses the first of these files that exists. +const CODE_OWNERS_PATHS = { + githubExpression: ".github/CODEOWNERS", + rootExpression: "CODEOWNERS", + docsExpression: "docs/CODEOWNERS", +} as const; + +const blobSchema = z.object({ text: z.string().nullish() }).nullable(); +const responseSchema = z.object({ + data: z.object({ + repository: z.object({ + github: blobSchema, + root: blobSchema, + docs: blobSchema, + pullRequest: z.object({ + files: z + .object({ + nodes: z.array(z.object({ path: z.string() })), + pageInfo: z.object({ + endCursor: z.string().nullable(), + hasNextPage: z.boolean(), + }), + }) + .nullable(), + }), + }), + }), +}); + +/** + * Read the code owners of the files that one pull request changes, from the + * CODEOWNERS file of its base branch. + */ +export async function readPullRequestCodeOwners( + egress: PluginEgress, + repo: { name: string; owner: string }, + pullRequest: { base: string; number: number }, +): Promise { + const expressions = Object.fromEntries( + Object.entries(CODE_OWNERS_PATHS).map(([key, path]) => [ + key, + `${pullRequest.base}:${path}`, + ]), + ); + const paths: string[] = []; + let codeOwners: string | undefined; + let after: string | null = null; + do { + const response = await githubApi(egress, { + operation: "github.pull.code_owners.get", + method: "POST", + path: "/graphql", + body: { + ...PULL_REQUEST_CODE_OWNERS, + variables: { + ...expressions, + after, + name: repo.name, + number: pullRequest.number, + owner: repo.owner, + }, + }, + }); + if (!response.ok) { + throw new Error( + `GitHub pull request code owners lookup failed with HTTP ${response.status}`, + ); + } + const repository = responseSchema.parse(response.body).data.repository; + codeOwners ??= + repository.github?.text ?? + repository.root?.text ?? + repository.docs?.text ?? + undefined; + const files = repository.pullRequest.files; + paths.push(...(files?.nodes.map((file) => file.path) ?? [])); + after = files?.pageInfo.hasNextPage ? files.pageInfo.endCursor : null; + } while (after); + return codeOwners ? codeOwnersOf(parseCodeOwners(codeOwners), paths) : []; +} diff --git a/packages/junior-github/src/events/pull-request.ts b/packages/junior-github/src/events/pull-request.ts index feb8e38b1f..67f44a4e29 100644 --- a/packages/junior-github/src/events/pull-request.ts +++ b/packages/junior-github/src/events/pull-request.ts @@ -139,11 +139,13 @@ const GITHUB_PULL_REQUEST_OUTCOME_GUIDANCE = { /** * Junior finishes its own drafts. Passing checks start no other step, so the * Watch of a draft also gets them. Marking the pull request ready sends the - * review request. Each passing suite wakes the Watch, so a suite that leaves - * the required checks unfinished must stay silent, or every suite posts. + * review request. GitHub also asks the code owners then, so a draft with code + * owners waits for the person who asked for it, unless they said otherwise. + * Each passing suite wakes the Watch, so a suite that leaves the required + * checks unfinished must stay silent, or every suite posts. */ const DRAFT_CHECKS_RECOVERED_GUIDANCE = - "If the pull request is still a draft, call github_getPullRequest and read its `checks`. When `checks.required` is `passing` and no decision is open, in the pull request body or in an open review item that waits on the person who asked for it, mark it ready with github_updatePullRequest and draft false, and report that in the originating thread. When it is `failing`, fix the failing checks like a failed check. When it is `pending` or `not_reported`, stay silent: a later passing suite wakes you again. `not_reported` means the required checks may not have started on the latest commit yet, not that none are required."; + "If the pull request is still a draft, call github_getPullRequest and read its `checks` and `code_owners`. When `checks.required` is `passing` and no decision is open, in the pull request body or in an open review item that waits on the person who asked for it: if `code_owners` is empty, or the person who asked for it said to mark it ready, mark it ready with github_updatePullRequest and draft false, and report that in the originating thread. Otherwise keep it a draft, because marking it ready asks those code owners to review. Tell the person who asked for it in the originating thread that the checks pass and the draft waits for their review, and name the code owners that marking it ready asks. If you already told them for this head commit, stay silent. When it is `failing`, fix the failing checks like a failed check. When it is `pending` or `not_reported`, stay silent: a later passing suite wakes you again. `not_reported` means the required checks may not have started on the latest commit yet, not that none are required."; /** * Build the Watch created after Junior creates a pull request. Only a draft diff --git a/packages/junior-github/src/graphql-documents.ts b/packages/junior-github/src/graphql-documents.ts index 48b19b262e..9cd0582cd2 100644 --- a/packages/junior-github/src/graphql-documents.ts +++ b/packages/junior-github/src/graphql-documents.ts @@ -156,6 +156,34 @@ export const PULL_REQUEST_CHECKS = githubGraphql(` } `); +/** + * Read the CODEOWNERS files of a base branch and one page of the files that + * a pull request changes. Each `*Expression` is `:`. + */ +export const PULL_REQUEST_CODE_OWNERS = githubGraphql(` + query PullRequestCodeOwners( + $owner: String! + $name: String! + $number: Int! + $githubExpression: String! + $rootExpression: String! + $docsExpression: String! + $after: String + ) { + repository(owner: $owner, name: $name) { + github: object(expression: $githubExpression) { ... on Blob { text } } + root: object(expression: $rootExpression) { ... on Blob { text } } + docs: object(expression: $docsExpression) { ... on Blob { text } } + pullRequest(number: $number) { + files(first: 100, after: $after) { + pageInfo { hasNextPage endCursor } + nodes { path } + } + } + } + } +`); + /** Read one page of a pull request search, with mergeability. */ export const OPEN_PULL_REQUEST_MERGEABILITY = githubGraphql(` query OpenPullRequestMergeability($query: String!, $after: String) { diff --git a/packages/junior-github/src/tools/get-pull-request.ts b/packages/junior-github/src/tools/get-pull-request.ts index bb08861969..c0303fe5ee 100644 --- a/packages/junior-github/src/tools/get-pull-request.ts +++ b/packages/junior-github/src/tools/get-pull-request.ts @@ -1,4 +1,5 @@ import { githubObjectAnnotation } from "../annotations.js"; +import { readPullRequestCodeOwners } from "../code-owners.js"; import { githubObjectFacts } from "../object-facts.js"; import { type PluginEgress, @@ -57,6 +58,12 @@ type Checks = z.output; const pullRequestSchema = z.object({ base: z.string(), checks: checksSchema, + code_owners: z + .array(z.string()) + .optional() + .describe( + "Draft only: the CODEOWNERS owners of the changed files. GitHub asks them to review when the pull request is marked ready for review.", + ), draft: z.boolean(), head: z.string(), head_sha: commitShaSchema, @@ -205,7 +212,7 @@ export function createGitHubGetPullRequestTool(ctx: { readOnlyHint: true, }, description: - "Get a GitHub pull request with its required checks and merge state. Use this when an existing PR may need event monitoring; the result includes a subscribable hint when GitHub webhooks are configured.", + "Get a GitHub pull request with its required checks and merge state, and for a draft, the code owners that GitHub asks to review when it is marked ready. Use this when an existing PR may need event monitoring; the result includes a subscribable hint when GitHub webhooks are configured.", inputSchema, outputSchema, async execute(input): Promise { @@ -239,6 +246,14 @@ export function createGitHubGetPullRequestTool(ctx: { repo, providerResult.number, ); + // Only a draft can still ask its code owners by being marked ready. + const codeOwners = + providerResult.draft && providerResult.state === "open" + ? await readPullRequestCodeOwners(ctx.egress, repo, { + base: providerResult.base.ref, + number: providerResult.number, + }) + : undefined; const subscribable = gitHubPullRequestSubscribable({ canSubscribe: ctx.events.canSubscribe, number: providerResult.number, @@ -247,6 +262,7 @@ export function createGitHubGetPullRequestTool(ctx: { const data: PullRequest = { base: providerResult.base.ref, checks, + ...(codeOwners ? { code_owners: codeOwners } : undefined), draft: providerResult.draft, head: providerResult.head.ref, head_sha: providerResult.head.sha.toLowerCase(), diff --git a/packages/junior-github/tests/get-pull-request.test.ts b/packages/junior-github/tests/get-pull-request.test.ts index 578de7e4b5..9a6539916e 100644 --- a/packages/junior-github/tests/get-pull-request.test.ts +++ b/packages/junior-github/tests/get-pull-request.test.ts @@ -288,6 +288,78 @@ describe("getPullRequest", () => { }); }); + it("returns the code owners that marking a draft ready asks", async () => { + const draft = pullRequestResponse(); + const codeOwners = ( + files: string[], + hasNextPage: boolean, + root: string | null = null, + ) => ({ + body: { + data: { + repository: { + github: null, + root: root === null ? null : { text: root }, + docs: null, + pullRequest: { + files: { + nodes: files.map((path) => ({ path })), + pageInfo: { + endCursor: hasNextPage ? "page-2" : null, + hasNextPage, + }, + }, + }, + }, + }, + }, + }); + const { adapter, tool } = toolContext(true, [ + { body: { ...draft.body, draft: true } }, + checksResponse({ contexts: [] }), + codeOwners( + ["docs/guide.txt", "docs/generated/api.txt"], + true, + [ + "# Everything else", + "* @getsentry/owners", + "/docs/ @getsentry/docs", + "/docs/generated/", + "*.md @alice # changelog owner", + ].join("\n"), + ), + codeOwners(["CHANGELOG.md"], false), + ]); + + const result = await tool.execute?.( + { repo: "getsentry/junior", number: 691 }, + { toolCallId: "get-draft-code-owners" }, + ); + + // The last matching rule wins, and a rule with no owners clears them. + expect(result).toMatchObject({ + code_owners: ["@alice", "@getsentry/docs"], + draft: true, + }); + const pages = await Promise.all( + adapter + .requests() + .slice(2) + .map((request) => request.request.clone().json()), + ); + expect(pages).toMatchObject([ + { + operationName: "PullRequestCodeOwners", + variables: { + after: null, + githubExpression: "main:.github/CODEOWNERS", + number: 691, + }, + }, + { variables: { after: "page-2" } }, + ]); + }); + it.each([ { name: "no checks", rollup: null }, { diff --git a/packages/junior-testing/src/http/github-drafts.ts b/packages/junior-testing/src/http/github-drafts.ts index 4cb7bba06a..f649296fad 100644 --- a/packages/junior-testing/src/http/github-drafts.ts +++ b/packages/junior-testing/src/http/github-drafts.ts @@ -1,13 +1,14 @@ // Draft pull requests whose check suite just passed. Each draft has the same // passing suite; they differ in other required checks and in the body. // #695's checks are not required, like a suite that reports before the -// required checks start. +// required checks start. #696 changes a file that has code owners. const repo = "getsentry/junior"; const web = `https://github.com/${repo}`; const api = `/repos/${repo}`; interface DraftFixture { body: string; + codeOwners?: string; failingCheck?: string; number: number; requiredNotReported?: true; @@ -40,6 +41,12 @@ const drafts: DraftFixture[] = [ requiredNotReported: true, sha: "6950000000000000000000000000000000000695", }, + { + body: draftBody, + codeOwners: "/src/cache/ @getsentry/cache-owners\n", + number: 696, + sha: "6960000000000000000000000000000000000696", + }, ]; const readied = new Set(); @@ -113,6 +120,10 @@ function graphqlPullRequest(draft: DraftFixture) { baseRefName: "main", body: draft.body, commits, + files: { + nodes: [{ path: "src/cache/warmup.ts" }], + pageInfo: { endCursor: null, hasNextPage: false }, + }, headRefName: `feat/cache-warmup-${draft.number}`, headRefOid: draft.sha, autoMergeRequest: null, @@ -162,8 +173,13 @@ export async function interceptTestGitHubDraftsHttp( } if (/\bmutation\b/.test(body.query ?? "")) return undefined; const pullRequest = graphqlPullRequest(draft); + // The CODEOWNERS file of the base branch, in `.github/`. + const github = draft.codeOwners ? { text: draft.codeOwners } : null; return Response.json({ - data: { node: pullRequest, repository: { pullRequest } }, + data: { + node: pullRequest, + repository: { docs: null, github, pullRequest, root: null }, + }, }); } if (request.method !== "GET") return undefined; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index e8c8647e32..8c55c36024 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -630,6 +630,9 @@ importers: drizzle-orm: specifier: 'catalog:' version: 0.45.4(@types/pg@8.15.6)(pg@8.21.0) + ignore: + specifier: ^7.0.5 + version: 7.0.5 marked: specifier: ^18.1.2 version: 18.1.2 From a66aef915c73831b287aac57f9c7c162a98ec88d Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Fri, 13 Nov 2026 17:53:27 +0000 Subject: [PATCH 2/4] fix(github): Fail when GitHub does not list a draft's changed files --- packages/junior-github/src/code-owners.ts | 14 +++++++-- .../tests/get-pull-request.test.ts | 31 +++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/packages/junior-github/src/code-owners.ts b/packages/junior-github/src/code-owners.ts index dc88766855..413f2df68c 100644 --- a/packages/junior-github/src/code-owners.ts +++ b/packages/junior-github/src/code-owners.ts @@ -125,8 +125,18 @@ export async function readPullRequestCodeOwners( repository.docs?.text ?? undefined; const files = repository.pullRequest.files; - paths.push(...(files?.nodes.map((file) => file.path) ?? [])); - after = files?.pageInfo.hasNextPage ? files.pageInfo.endCursor : null; + if (!files) { + // Without the changed files, no owners would read as "safe to mark + // ready". Fail instead, so the draft stays a draft. + if (codeOwners) { + throw new Error( + "GitHub did not list the changed files of the pull request", + ); + } + break; + } + paths.push(...files.nodes.map((file) => file.path)); + after = files.pageInfo.hasNextPage ? files.pageInfo.endCursor : null; } while (after); return codeOwners ? codeOwnersOf(parseCodeOwners(codeOwners), paths) : []; } diff --git a/packages/junior-github/tests/get-pull-request.test.ts b/packages/junior-github/tests/get-pull-request.test.ts index 9a6539916e..a9ce0afa51 100644 --- a/packages/junior-github/tests/get-pull-request.test.ts +++ b/packages/junior-github/tests/get-pull-request.test.ts @@ -360,6 +360,37 @@ describe("getPullRequest", () => { ]); }); + it("fails when GitHub does not list the changed files of a draft with code owners", async () => { + const draft = pullRequestResponse(); + const { tool } = toolContext(true, [ + { body: { ...draft.body, draft: true } }, + checksResponse({ contexts: [] }), + { + body: { + data: { + repository: { + github: { text: "* @getsentry/owners" }, + root: null, + docs: null, + pullRequest: { files: null }, + }, + }, + }, + }, + ]); + + // No files must not read as no code owners, or the draft would be + // marked ready and ask them. + await expect( + tool.execute?.( + { repo: "getsentry/junior", number: 691 }, + { toolCallId: "get-draft-without-files" }, + ), + ).rejects.toThrow( + "GitHub did not list the changed files of the pull request", + ); + }); + it.each([ { name: "no checks", rollup: null }, { From c7ce625c6fa908d4a9878f256fdc712bfb896b10 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Fri, 13 Nov 2026 18:01:57 +0000 Subject: [PATCH 3/4] feat(github): Keep drafts when the repo's AGENTS.md asks for drafts Co-Authored-By: Burak Yigit Kaya <126780+BYK@users.noreply.github.com> --- .../evals/coding/subscriptions.eval.ts | 27 +++++++++++++- .../junior-github/skills/github-code/SKILL.md | 2 +- .../junior-github/skills/github-code/SPEC.md | 2 +- .../{code-owners.ts => draft-readiness.ts} | 35 ++++++++++++------- .../junior-github/src/events/pull-request.ts | 3 +- .../junior-github/src/graphql-documents.ts | 11 +++--- .../src/tools/get-pull-request.ts | 19 +++++++--- .../tests/get-pull-request.test.ts | 7 +++- .../junior-testing/src/http/github-drafts.ts | 16 +++++++-- 9 files changed, 92 insertions(+), 30 deletions(-) rename packages/junior-github/src/{code-owners.ts => draft-readiness.ts} (80%) diff --git a/packages/junior-evals/evals/coding/subscriptions.eval.ts b/packages/junior-evals/evals/coding/subscriptions.eval.ts index 855640608e..bde19b6b9e 100644 --- a/packages/junior-evals/evals/coding/subscriptions.eval.ts +++ b/packages/junior-evals/evals/coding/subscriptions.eval.ts @@ -84,13 +84,15 @@ function checkSuiteWebhook(conclusion: "failure" | "success") { // The shared GitHub HTTP fixture serves these drafts: #692 with every // required check passing, #693 with an open decision in its body, #694 // with another required check failing, #695 with no required check -// reported yet, and #696 with code owners for its changed file. +// reported yet, #696 with code owners for its changed file, and #697 +// whose base branch AGENTS.md asks for draft pull requests. const draftHeadShas = { 692: "6920000000000000000000000000000000000692", 693: "6930000000000000000000000000000000000693", 694: "6940000000000000000000000000000000000694", 695: "6950000000000000000000000000000000000695", 696: "6960000000000000000000000000000000000696", + 697: "6970000000000000000000000000000000000697", }; type DraftNumber = keyof typeof draftHeadShas; @@ -395,6 +397,29 @@ describe("Watches", () => { expect(delivery.replies).toHaveLength(1); }); + test("when the AGENTS.md of a Junior draft asks for drafts, keep it a draft and tell the requester", async ({ + run, + }) => { + const conversation = await startDraftThread(run, 697); + + const delivery = await conversation.continue(draftChecksPassedWebhook(697)); + await expect(delivery).toSatisfyJudge( + RubricJudge, + rubric({ + pass: [ + "The reply says the required checks of getsentry/junior#697 pass and the draft waits for the requester's review.", + "The reply says the repository's AGENTS.md asks for draft pull requests.", + ], + fail: [ + "Do not say that getsentry/junior#697 is ready for review or was marked ready.", + ], + }), + ); + + expect(markReadyCalls(delivery)).toEqual([]); + expect(delivery.replies).toHaveLength(1); + }); + test("when a Junior draft names an open decision, keep it a draft", async ({ run, }) => { diff --git a/packages/junior-github/skills/github-code/SKILL.md b/packages/junior-github/skills/github-code/SKILL.md index d77182d902..995c8f32b0 100644 --- a/packages/junior-github/skills/github-code/SKILL.md +++ b/packages/junior-github/skills/github-code/SKILL.md @@ -37,7 +37,7 @@ Use `git` and `gh` for repository work. - When an error names a tool, use it. Change permissions only when GitHub rejected the request. - Stop for an unclear target, missing access, destructive work, or a GitHub permission failure. - Code changes always end in a pushed branch and a PR. Do not ask whether to open one. Skip it only when the user opts out or asks for investigation only. -- Open the PR ready for review when the work is complete and the checks pass. Open a draft when work, checks, or decisions remain, or when the repo has a `CODEOWNERS` file in `.github/`, the root, or `docs/` and the requester did not ask for a ready PR: opening it ready asks the code owners to review. A scope or follow-up question is not a reason to stop: open the PR, and name the question in the PR body and the report. Name each open decision of a draft in its PR body: when the required checks pass, the Watch marks a draft with no open decision ready for review. A draft whose changed files have code owners stays a draft, and the Watch tells the requester that it waits for their review. +- Open the PR ready for review when the work is complete and the checks pass. Open a draft when work, checks, or decisions remain, or when the repo has a `CODEOWNERS` file in `.github/`, the root, or `docs/` and the requester did not ask for a ready PR: opening it ready asks the code owners to review. A scope or follow-up question is not a reason to stop: open the PR, and name the question in the PR body and the report. Name each open decision of a draft in its PR body: when the required checks pass, the Watch marks a draft with no open decision ready for review. A draft whose changed files have code owners, or whose repo's `AGENTS.md` asks for drafts, stays a draft, and the Watch tells the requester that it waits for their review. - Link the PR. Mention checks only when they failed or were skipped. - The runtime adds the requester `via` line. Do not write one. - Credit other people, such as a reporter, by GitHub `@login` so GitHub notifies them. Get the login from `userLookup` (`linked_identities` with provider `github`, else a GitHub link in `profile_fields`). Pass the Slack user ID when known. With no login, write the name. Never guess a login. When you delegate the write, put the `@login` in the brief. diff --git a/packages/junior-github/skills/github-code/SPEC.md b/packages/junior-github/skills/github-code/SPEC.md index 89bc73b687..c8e3705d73 100644 --- a/packages/junior-github/skills/github-code/SPEC.md +++ b/packages/junior-github/skills/github-code/SPEC.md @@ -10,7 +10,7 @@ Guide evidence-first GitHub repository work from inspection through a reviewable - Preserve unrelated work; reject destructive Git operations except `--force-with-lease` pushes to branches Junior pushed or the requester named. - Treat shallow clones as inspection checkouts; deepen before history work; never force-push around missing ancestry. - Install dependencies with the lockfile frozen mode before verification when needed. -- Finish every code change with a pushed PR, without asking, unless the user opts out or asked for investigation only. Open it ready when work and checks are done; open a draft when work, checks, or decisions remain, or when the repo has a CODEOWNERS file and the requester did not ask for a ready PR. An open scope question goes in the PR body, not in a stop. The Watch of a draft marks it ready when its required checks pass and no decision is open, so open decisions of a draft go in its PR body. A draft whose changed files have CODEOWNERS owners stays a draft unless the requester said to mark it ready, and Junior tells the requester that it waits for their review. +- Finish every code change with a pushed PR, without asking, unless the user opts out or asked for investigation only. Open it ready when work and checks are done; open a draft when work, checks, or decisions remain, or when the repo has a CODEOWNERS file and the requester did not ask for a ready PR. An open scope question goes in the PR body, not in a stop. The Watch of a draft marks it ready when its required checks pass and no decision is open, so open decisions of a draft go in its PR body. A draft whose changed files have CODEOWNERS owners, or whose repo's root AGENTS.md asks for draft PRs or for a person to mark them ready, stays a draft unless the requester said to mark it ready, and Junior tells the requester that it waits for their review. - After each push to a branch with an open PR, check mergeability. Resolve conflicts without asking, by merging the PR base branch (usually the default branch). Never rebase for this, except in a stack: a stack merges only with a linear history, so stacked PRs restack with `gh stack`. GitHub sends no event for a new conflict, and conflicted PRs get no new checks. The plugin publishes `pull_request.merge_conflict` for a watched PR only when another webhook event arrives for it, so the post-push check stays. - Write conventional PR titles and short plain-English bodies; keep check results out of the PR body. - Report exact validation and permission failures. diff --git a/packages/junior-github/src/code-owners.ts b/packages/junior-github/src/draft-readiness.ts similarity index 80% rename from packages/junior-github/src/code-owners.ts rename to packages/junior-github/src/draft-readiness.ts index 413f2df68c..79c118466b 100644 --- a/packages/junior-github/src/code-owners.ts +++ b/packages/junior-github/src/draft-readiness.ts @@ -1,5 +1,7 @@ /** - * The code owners that GitHub asks to review a pull request. + * What a draft needs before Junior marks it ready: the code owners that + * GitHub asks to review, and the repository's AGENTS.md, which can ask that + * pull requests stay drafts for a person to mark ready. * * GitHub reads the first CODEOWNERS file of the base branch, in `.github/`, * the repository root, or `docs/`. For each changed file the last matching @@ -15,7 +17,7 @@ import type { PluginEgress } from "@sentry/junior-plugin-api"; import ignore from "ignore"; import * as z from "zod"; -import { PULL_REQUEST_CODE_OWNERS } from "./graphql-documents.js"; +import { PULL_REQUEST_DRAFT_READINESS } from "./graphql-documents.js"; import { githubApi } from "./tool-support/github-api.js"; interface CodeOwnersRule { @@ -64,6 +66,7 @@ const responseSchema = z.object({ github: blobSchema, root: blobSchema, docs: blobSchema, + agents: blobSchema, pullRequest: z.object({ files: z .object({ @@ -80,30 +83,30 @@ const responseSchema = z.object({ }); /** - * Read the code owners of the files that one pull request changes, from the - * CODEOWNERS file of its base branch. + * Read the code owners of the files that one draft changes and the root + * AGENTS.md, both from its base branch. */ -export async function readPullRequestCodeOwners( +export async function readDraftReadiness( egress: PluginEgress, repo: { name: string; owner: string }, pullRequest: { base: string; number: number }, -): Promise { +): Promise<{ agentsMd?: string; codeOwners: string[] }> { const expressions = Object.fromEntries( - Object.entries(CODE_OWNERS_PATHS).map(([key, path]) => [ - key, - `${pullRequest.base}:${path}`, - ]), + Object.entries({ ...CODE_OWNERS_PATHS, agentsExpression: "AGENTS.md" }).map( + ([key, path]) => [key, `${pullRequest.base}:${path}`], + ), ); const paths: string[] = []; let codeOwners: string | undefined; + let agentsMd: string | undefined; let after: string | null = null; do { const response = await githubApi(egress, { - operation: "github.pull.code_owners.get", + operation: "github.pull.draft_readiness.get", method: "POST", path: "/graphql", body: { - ...PULL_REQUEST_CODE_OWNERS, + ...PULL_REQUEST_DRAFT_READINESS, variables: { ...expressions, after, @@ -124,6 +127,7 @@ export async function readPullRequestCodeOwners( repository.root?.text ?? repository.docs?.text ?? undefined; + agentsMd ??= repository.agents?.text ?? undefined; const files = repository.pullRequest.files; if (!files) { // Without the changed files, no owners would read as "safe to mark @@ -138,5 +142,10 @@ export async function readPullRequestCodeOwners( paths.push(...files.nodes.map((file) => file.path)); after = files.pageInfo.hasNextPage ? files.pageInfo.endCursor : null; } while (after); - return codeOwners ? codeOwnersOf(parseCodeOwners(codeOwners), paths) : []; + return { + ...(agentsMd ? { agentsMd } : undefined), + codeOwners: codeOwners + ? codeOwnersOf(parseCodeOwners(codeOwners), paths) + : [], + }; } diff --git a/packages/junior-github/src/events/pull-request.ts b/packages/junior-github/src/events/pull-request.ts index 67f44a4e29..04998190ce 100644 --- a/packages/junior-github/src/events/pull-request.ts +++ b/packages/junior-github/src/events/pull-request.ts @@ -141,11 +141,12 @@ const GITHUB_PULL_REQUEST_OUTCOME_GUIDANCE = { * Watch of a draft also gets them. Marking the pull request ready sends the * review request. GitHub also asks the code owners then, so a draft with code * owners waits for the person who asked for it, unless they said otherwise. + * A repository's AGENTS.md can also ask that drafts wait for a person. * Each passing suite wakes the Watch, so a suite that leaves the required * checks unfinished must stay silent, or every suite posts. */ const DRAFT_CHECKS_RECOVERED_GUIDANCE = - "If the pull request is still a draft, call github_getPullRequest and read its `checks` and `code_owners`. When `checks.required` is `passing` and no decision is open, in the pull request body or in an open review item that waits on the person who asked for it: if `code_owners` is empty, or the person who asked for it said to mark it ready, mark it ready with github_updatePullRequest and draft false, and report that in the originating thread. Otherwise keep it a draft, because marking it ready asks those code owners to review. Tell the person who asked for it in the originating thread that the checks pass and the draft waits for their review, and name the code owners that marking it ready asks. If you already told them for this head commit, stay silent. When it is `failing`, fix the failing checks like a failed check. When it is `pending` or `not_reported`, stay silent: a later passing suite wakes you again. `not_reported` means the required checks may not have started on the latest commit yet, not that none are required."; + "If the pull request is still a draft, call github_getPullRequest and read its `checks`, `code_owners`, and `agents_md`. When `checks.required` is `passing` and no decision is open, in the pull request body or in an open review item that waits on the person who asked for it: mark it ready with github_updatePullRequest and draft false, and report that in the originating thread, when the person who asked for it said to mark it ready, or when `code_owners` is empty and `agents_md` does not ask for draft pull requests or say that a person marks them ready. Otherwise keep it a draft: marking it ready would ask the code owners to review, or break the AGENTS.md rule. Tell the person who asked for it in the originating thread that the checks pass and the draft waits for their review, and name why: the code owners that marking it ready asks, or the AGENTS.md rule. If you already told them for this head commit, stay silent. When it is `failing`, fix the failing checks like a failed check. When it is `pending` or `not_reported`, stay silent: a later passing suite wakes you again. `not_reported` means the required checks may not have started on the latest commit yet, not that none are required."; /** * Build the Watch created after Junior creates a pull request. Only a draft diff --git a/packages/junior-github/src/graphql-documents.ts b/packages/junior-github/src/graphql-documents.ts index 9cd0582cd2..1291d3b31e 100644 --- a/packages/junior-github/src/graphql-documents.ts +++ b/packages/junior-github/src/graphql-documents.ts @@ -157,23 +157,26 @@ export const PULL_REQUEST_CHECKS = githubGraphql(` `); /** - * Read the CODEOWNERS files of a base branch and one page of the files that - * a pull request changes. Each `*Expression` is `:`. + * Read the CODEOWNERS files and the root AGENTS.md of a base branch, and one + * page of the files that a pull request changes. Each `*Expression` is + * `:`. */ -export const PULL_REQUEST_CODE_OWNERS = githubGraphql(` - query PullRequestCodeOwners( +export const PULL_REQUEST_DRAFT_READINESS = githubGraphql(` + query PullRequestDraftReadiness( $owner: String! $name: String! $number: Int! $githubExpression: String! $rootExpression: String! $docsExpression: String! + $agentsExpression: String! $after: String ) { repository(owner: $owner, name: $name) { github: object(expression: $githubExpression) { ... on Blob { text } } root: object(expression: $rootExpression) { ... on Blob { text } } docs: object(expression: $docsExpression) { ... on Blob { text } } + agents: object(expression: $agentsExpression) { ... on Blob { text } } pullRequest(number: $number) { files(first: 100, after: $after) { pageInfo { hasNextPage endCursor } diff --git a/packages/junior-github/src/tools/get-pull-request.ts b/packages/junior-github/src/tools/get-pull-request.ts index c0303fe5ee..ad861cf8c2 100644 --- a/packages/junior-github/src/tools/get-pull-request.ts +++ b/packages/junior-github/src/tools/get-pull-request.ts @@ -1,5 +1,5 @@ import { githubObjectAnnotation } from "../annotations.js"; -import { readPullRequestCodeOwners } from "../code-owners.js"; +import { readDraftReadiness } from "../draft-readiness.js"; import { githubObjectFacts } from "../object-facts.js"; import { type PluginEgress, @@ -57,6 +57,12 @@ const checksSchema = z.object({ type Checks = z.output; const pullRequestSchema = z.object({ base: z.string(), + agents_md: z + .string() + .optional() + .describe( + "Draft only: the root AGENTS.md of the base branch, when it has one.", + ), checks: checksSchema, code_owners: z .array(z.string()) @@ -212,7 +218,7 @@ export function createGitHubGetPullRequestTool(ctx: { readOnlyHint: true, }, description: - "Get a GitHub pull request with its required checks and merge state, and for a draft, the code owners that GitHub asks to review when it is marked ready. Use this when an existing PR may need event monitoring; the result includes a subscribable hint when GitHub webhooks are configured.", + "Get a GitHub pull request with its required checks and merge state, and for a draft, what to check before marking it ready. Use this when an existing PR may need event monitoring; the result includes a subscribable hint when GitHub webhooks are configured.", inputSchema, outputSchema, async execute(input): Promise { @@ -247,9 +253,9 @@ export function createGitHubGetPullRequestTool(ctx: { providerResult.number, ); // Only a draft can still ask its code owners by being marked ready. - const codeOwners = + const readiness = providerResult.draft && providerResult.state === "open" - ? await readPullRequestCodeOwners(ctx.egress, repo, { + ? await readDraftReadiness(ctx.egress, repo, { base: providerResult.base.ref, number: providerResult.number, }) @@ -261,8 +267,11 @@ export function createGitHubGetPullRequestTool(ctx: { }); const data: PullRequest = { base: providerResult.base.ref, + ...(readiness?.agentsMd + ? { agents_md: readiness.agentsMd } + : undefined), checks, - ...(codeOwners ? { code_owners: codeOwners } : undefined), + ...(readiness ? { code_owners: readiness.codeOwners } : undefined), draft: providerResult.draft, head: providerResult.head.ref, head_sha: providerResult.head.sha.toLowerCase(), diff --git a/packages/junior-github/tests/get-pull-request.test.ts b/packages/junior-github/tests/get-pull-request.test.ts index a9ce0afa51..2b24e92fe4 100644 --- a/packages/junior-github/tests/get-pull-request.test.ts +++ b/packages/junior-github/tests/get-pull-request.test.ts @@ -301,6 +301,8 @@ describe("getPullRequest", () => { github: null, root: root === null ? null : { text: root }, docs: null, + agents: + root === null ? null : { text: "Always open PRs as draft." }, pullRequest: { files: { nodes: files.map((path) => ({ path })), @@ -338,6 +340,7 @@ describe("getPullRequest", () => { // The last matching rule wins, and a rule with no owners clears them. expect(result).toMatchObject({ + agents_md: "Always open PRs as draft.", code_owners: ["@alice", "@getsentry/docs"], draft: true, }); @@ -349,9 +352,10 @@ describe("getPullRequest", () => { ); expect(pages).toMatchObject([ { - operationName: "PullRequestCodeOwners", + operationName: "PullRequestDraftReadiness", variables: { after: null, + agentsExpression: "main:AGENTS.md", githubExpression: "main:.github/CODEOWNERS", number: 691, }, @@ -372,6 +376,7 @@ describe("getPullRequest", () => { github: { text: "* @getsentry/owners" }, root: null, docs: null, + agents: null, pullRequest: { files: null }, }, }, diff --git a/packages/junior-testing/src/http/github-drafts.ts b/packages/junior-testing/src/http/github-drafts.ts index f649296fad..0f6495c73e 100644 --- a/packages/junior-testing/src/http/github-drafts.ts +++ b/packages/junior-testing/src/http/github-drafts.ts @@ -1,12 +1,14 @@ // Draft pull requests whose check suite just passed. Each draft has the same // passing suite; they differ in other required checks and in the body. // #695's checks are not required, like a suite that reports before the -// required checks start. #696 changes a file that has code owners. +// required checks start. #696 changes a file that has code owners. #697's +// base branch has an AGENTS.md that asks for draft pull requests. const repo = "getsentry/junior"; const web = `https://github.com/${repo}`; const api = `/repos/${repo}`; interface DraftFixture { + agentsMd?: string; body: string; codeOwners?: string; failingCheck?: string; @@ -47,6 +49,12 @@ const drafts: DraftFixture[] = [ number: 696, sha: "6960000000000000000000000000000000000696", }, + { + agentsMd: "# Agent Instructions\n\n- Always open PRs as draft.\n", + body: draftBody, + number: 697, + sha: "6970000000000000000000000000000000000697", + }, ]; const readied = new Set(); @@ -173,12 +181,14 @@ export async function interceptTestGitHubDraftsHttp( } if (/\bmutation\b/.test(body.query ?? "")) return undefined; const pullRequest = graphqlPullRequest(draft); - // The CODEOWNERS file of the base branch, in `.github/`. + // The CODEOWNERS file of the base branch, in `.github/`, and its + // AGENTS.md. const github = draft.codeOwners ? { text: draft.codeOwners } : null; + const agents = draft.agentsMd ? { text: draft.agentsMd } : null; return Response.json({ data: { node: pullRequest, - repository: { docs: null, github, pullRequest, root: null }, + repository: { agents, docs: null, github, pullRequest, root: null }, }, }); } From 2c31d836f2880bc18be41211db554f864717408f Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Fri, 13 Nov 2026 21:19:32 +0000 Subject: [PATCH 4/4] docs(github-code): Open PRs as drafts and let the Watch check code owners --- packages/junior-github/skills/github-code/SKILL.md | 2 +- packages/junior-github/skills/github-code/SPEC.md | 2 +- .../junior-github/skills/github-code/references/workflow.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/junior-github/skills/github-code/SKILL.md b/packages/junior-github/skills/github-code/SKILL.md index 995c8f32b0..23909a6c6e 100644 --- a/packages/junior-github/skills/github-code/SKILL.md +++ b/packages/junior-github/skills/github-code/SKILL.md @@ -37,7 +37,7 @@ Use `git` and `gh` for repository work. - When an error names a tool, use it. Change permissions only when GitHub rejected the request. - Stop for an unclear target, missing access, destructive work, or a GitHub permission failure. - Code changes always end in a pushed branch and a PR. Do not ask whether to open one. Skip it only when the user opts out or asks for investigation only. -- Open the PR ready for review when the work is complete and the checks pass. Open a draft when work, checks, or decisions remain, or when the repo has a `CODEOWNERS` file in `.github/`, the root, or `docs/` and the requester did not ask for a ready PR: opening it ready asks the code owners to review. A scope or follow-up question is not a reason to stop: open the PR, and name the question in the PR body and the report. Name each open decision of a draft in its PR body: when the required checks pass, the Watch marks a draft with no open decision ready for review. A draft whose changed files have code owners, or whose repo's `AGENTS.md` asks for drafts, stays a draft, and the Watch tells the requester that it waits for their review. +- Open the PR as a draft, unless the requester asks for a ready PR. A scope or follow-up question is not a reason to stop: open the PR, and name the question in the PR body and the report. Name each open decision of a draft in its PR body: when the required checks pass, the Watch marks a draft with no open decision ready for review. It checks the changed files against CODEOWNERS first: a draft that would ask code owners to review, or whose repo's `AGENTS.md` asks for drafts, stays a draft, and the Watch tells the requester that it waits for their review. - Link the PR. Mention checks only when they failed or were skipped. - The runtime adds the requester `via` line. Do not write one. - Credit other people, such as a reporter, by GitHub `@login` so GitHub notifies them. Get the login from `userLookup` (`linked_identities` with provider `github`, else a GitHub link in `profile_fields`). Pass the Slack user ID when known. With no login, write the name. Never guess a login. When you delegate the write, put the `@login` in the brief. diff --git a/packages/junior-github/skills/github-code/SPEC.md b/packages/junior-github/skills/github-code/SPEC.md index c8e3705d73..521fdfb1df 100644 --- a/packages/junior-github/skills/github-code/SPEC.md +++ b/packages/junior-github/skills/github-code/SPEC.md @@ -10,7 +10,7 @@ Guide evidence-first GitHub repository work from inspection through a reviewable - Preserve unrelated work; reject destructive Git operations except `--force-with-lease` pushes to branches Junior pushed or the requester named. - Treat shallow clones as inspection checkouts; deepen before history work; never force-push around missing ancestry. - Install dependencies with the lockfile frozen mode before verification when needed. -- Finish every code change with a pushed PR, without asking, unless the user opts out or asked for investigation only. Open it ready when work and checks are done; open a draft when work, checks, or decisions remain, or when the repo has a CODEOWNERS file and the requester did not ask for a ready PR. An open scope question goes in the PR body, not in a stop. The Watch of a draft marks it ready when its required checks pass and no decision is open, so open decisions of a draft go in its PR body. A draft whose changed files have CODEOWNERS owners, or whose repo's root AGENTS.md asks for draft PRs or for a person to mark them ready, stays a draft unless the requester said to mark it ready, and Junior tells the requester that it waits for their review. +- Finish every code change with a pushed PR, without asking, unless the user opts out or asked for investigation only. Open it as a draft unless the requester asks for a ready PR: opening a PR ready asks its code owners at once, and only the draft Watch checks the changed files against CODEOWNERS. An open scope question goes in the PR body, not in a stop. The Watch of a draft marks it ready when its required checks pass and no decision is open, so open decisions of a draft go in its PR body. A draft whose changed files have CODEOWNERS owners, or whose repo's root AGENTS.md asks for draft PRs or for a person to mark them ready, stays a draft unless the requester said to mark it ready, and Junior tells the requester that it waits for their review. - After each push to a branch with an open PR, check mergeability. Resolve conflicts without asking, by merging the PR base branch (usually the default branch). Never rebase for this, except in a stack: a stack merges only with a linear history, so stacked PRs restack with `gh stack`. GitHub sends no event for a new conflict, and conflicted PRs get no new checks. The plugin publishes `pull_request.merge_conflict` for a watched PR only when another webhook event arrives for it, so the post-push check stays. - Write conventional PR titles and short plain-English bodies; keep check results out of the PR body. - Report exact validation and permission failures. diff --git a/packages/junior-github/skills/github-code/references/workflow.md b/packages/junior-github/skills/github-code/references/workflow.md index 564df9cdc3..7908ebd964 100644 --- a/packages/junior-github/skills/github-code/references/workflow.md +++ b/packages/junior-github/skills/github-code/references/workflow.md @@ -42,7 +42,7 @@ Open this when making repository edits, not for read-only inspection. - Not in a stack: GitHub checks mergeability against the PR base (`baseRefName`), which is usually the default branch. Fetch the base and merge `origin/BASE` into the PR branch. Do not rebase. Resolve each conflict so that both the PR change and the base branch change stay intact. Run the checks again and push. - Then check mergeability again. 4. Resolve the actual default branch. -5. Update an existing PR for the branch with `github_updatePullRequest`, or create with `github_createPullRequest` (`draft: true` when work, checks, or decisions remain, or when the repo has a `CODEOWNERS` file and the requester did not ask for a ready PR; otherwise ready for review). +5. Update an existing PR for the branch with `github_updatePullRequest`, or create with `github_createPullRequest` (`draft: true` unless the requester asked for a ready PR). 6. PR title matches the current dominant change, same conventional form as commits. 7. PR body: short, plain English, what changed and why. Only context the diff cannot show. No empty sections, file lists, commit logs, or Checks/Verification/Test plan blocks. 8. After material follow-up commits, refresh title and body against the current diff.