From ee175c900cf210d86e2c7237badf588e0e082a32 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Tue, 22 Sep 2026 16:00:03 -0400 Subject: [PATCH 1/2] Say when an agent may resolve a review thread, and give `core` an overlay `pr-guidelines` is a base skill whose description covers reviewing someone else's PR, and it had no `repos/core.md`. `tools/install` skips a skill whose `repos/` holds no file for the target repo, so `core` received none of it. --- .../skills/pr-guidelines/repos/core.md | 15 +++++++++++++++ .../pr-guidelines/repos/metamask-extension.md | 9 +++++++++ 2 files changed, 24 insertions(+) create mode 100644 domains/pr-workflow/skills/pr-guidelines/repos/core.md diff --git a/domains/pr-workflow/skills/pr-guidelines/repos/core.md b/domains/pr-workflow/skills/pr-guidelines/repos/core.md new file mode 100644 index 00000000..3520033f --- /dev/null +++ b/domains/pr-workflow/skills/pr-guidelines/repos/core.md @@ -0,0 +1,15 @@ +--- +repo: core +parent: pr-guidelines +--- + +## Reviewing Pull Requests + +### Resolving Review Threads + +Applies to agents in particular, where the cost of getting this wrong is a thread closed before its reviewer read it. + +- **Never resolve a review thread before human reviewers have had time to read new comments and respond.** This is a hard rule, not a convention. +- **Do not resolve threads unilaterally.** Prefer to let the human reviewers engaging with a thread close it once they have determined the raised issues are resolved. +- **Leave a thread open if it may be relevant, educational, or of future reference value.** Most threads should still be closed to avoid clutter. +- **Do not re-post on an unresolved thread that has not been updated.** Resolving every thread is not a merge requirement in this repo, unlike Mobile, so an unresolved thread is not by itself a signal that anything is outstanding. diff --git a/domains/pr-workflow/skills/pr-guidelines/repos/metamask-extension.md b/domains/pr-workflow/skills/pr-guidelines/repos/metamask-extension.md index cc7eec91..5ef71534 100644 --- a/domains/pr-workflow/skills/pr-guidelines/repos/metamask-extension.md +++ b/domains/pr-workflow/skills/pr-guidelines/repos/metamask-extension.md @@ -237,6 +237,15 @@ When NOT to use: - Non-blocking suggestions - Nitpicks +### Resolving Review Threads + +Applies to agents in particular, where the cost of getting this wrong is a thread closed before its reviewer read it. + +- **Never resolve a review thread before human reviewers have had time to read new comments and respond.** This is a hard rule, not a convention. +- **Do not resolve threads unilaterally.** Prefer to let the human reviewers engaging with a thread close it once they have determined the raised issues are resolved. +- **Leave a thread open if it may be relevant, educational, or of future reference value.** Most threads should still be closed to avoid clutter. +- **Do not re-post on an unresolved thread that has not been updated.** Resolving every thread is not a merge requirement in this repo, unlike Mobile, so an unresolved thread is not by itself a signal that anything is outstanding. + ## Receiving Feedback ### Be Open to Other Perspectives From 312d3f600a494cbf06bfd4159c922314dd77807a Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Wed, 23 Sep 2026 05:04:39 -0400 Subject: [PATCH 2/2] Never collapse a human's comment without being asked Resolving, minimizing and hiding all take a comment out of the default view, which asserts the matter is settled. That judgement belongs to whoever raised it, and an agent making it on a principal's behalf makes it in their name. --- domains/pr-workflow/skills/pr-guidelines/repos/core.md | 1 + .../pr-workflow/skills/pr-guidelines/repos/metamask-extension.md | 1 + 2 files changed, 2 insertions(+) diff --git a/domains/pr-workflow/skills/pr-guidelines/repos/core.md b/domains/pr-workflow/skills/pr-guidelines/repos/core.md index 3520033f..22224cb9 100644 --- a/domains/pr-workflow/skills/pr-guidelines/repos/core.md +++ b/domains/pr-workflow/skills/pr-guidelines/repos/core.md @@ -9,6 +9,7 @@ parent: pr-guidelines Applies to agents in particular, where the cost of getting this wrong is a thread closed before its reviewer read it. +- **Never resolve, minimize or hide a comment a human wrote, unless that human explicitly asks you to.** Collapsing a comment takes it out of the default view and asserts the matter is settled, which is a judgement belonging to whoever raised it. Doing it on a principal's behalf makes that assertion in their name. - **Never resolve a review thread before human reviewers have had time to read new comments and respond.** This is a hard rule, not a convention. - **Do not resolve threads unilaterally.** Prefer to let the human reviewers engaging with a thread close it once they have determined the raised issues are resolved. - **Leave a thread open if it may be relevant, educational, or of future reference value.** Most threads should still be closed to avoid clutter. diff --git a/domains/pr-workflow/skills/pr-guidelines/repos/metamask-extension.md b/domains/pr-workflow/skills/pr-guidelines/repos/metamask-extension.md index 5ef71534..c3b139f5 100644 --- a/domains/pr-workflow/skills/pr-guidelines/repos/metamask-extension.md +++ b/domains/pr-workflow/skills/pr-guidelines/repos/metamask-extension.md @@ -241,6 +241,7 @@ When NOT to use: Applies to agents in particular, where the cost of getting this wrong is a thread closed before its reviewer read it. +- **Never resolve, minimize or hide a comment a human wrote, unless that human explicitly asks you to.** Collapsing a comment takes it out of the default view and asserts the matter is settled, which is a judgement belonging to whoever raised it. Doing it on a principal's behalf makes that assertion in their name. - **Never resolve a review thread before human reviewers have had time to read new comments and respond.** This is a hard rule, not a convention. - **Do not resolve threads unilaterally.** Prefer to let the human reviewers engaging with a thread close it once they have determined the raised issues are resolved. - **Leave a thread open if it may be relevant, educational, or of future reference value.** Most threads should still be closed to avoid clutter.