Sandcastle reworks PRs with requested changes #21

New issue

Closed

opened 2026-07-05 07:33:00 +08:00 by weiwen · 1 comment

weiwen commented

2026-07-05 07:33:00 +08:00

Owner

What to build

Add the second PR-rework signal on top of the CI-fail path (#20): when a human requests changes on an open PR, sandcastle labels it for the agent and the planner prioritises reworking it.

Trigger: on: pull_request + a reviews-API check. pull_request_review is not a valid Forgejo Actions trigger, but on: pull_request fires when a comment is made on the PR in this instance, which covers a changes-requested review (reviews normally carry comments). The pull_request payload carries the PR, not the review state, so the workflow step must query the reviews API (GET /repos/weiwen/evie/pulls/<n>/reviews) and act only when the latest human review is REQUEST_CHANGES.

End-to-end behaviour:

A Forgejo Actions workflow (.github/workflows/pr-triage.yml, since a Forgejo runner already executes that folder) triggers on pull_request. If the PR's latest human review is REQUEST_CHANGES, it adds ready-for-agent and removes ready-for-human. It must NOT label on approvals or ordinary comments. Authenticate with the runner's automatic token, falling back to a FORGEJO_TOKEN Actions secret if that token can't edit labels.
The planner gains the label arm: a PR carrying ready-for-agent needs rework (in addition to #20's ci == "failure").
The pr-fixer gains a review-driven branch: read the changes-requested review body + comments (tea pulls review-comments <n> / reviews API), address them on the branch, push, then relabel ready-for-agent → ready-for-human and comment summarising the feedback addressed. The relabel is what stops the planner re-picking it.

Accepted limitation: a changes-requested review submitted with no comment and no commit may not fire pull_request; the human can leave a comment to trigger, or the next interaction will. Document this.

Acceptance criteria

A workflow under .github/workflows fires on pull_request and, when the PR's latest human review is REQUEST_CHANGES, adds ready-for-agent and removes ready-for-human.
The workflow does NOT apply the label on approvals or on comments that are not a changes-requested review.
The label-edit auth approach (automatic token vs FORGEJO_TOKEN secret) is settled and documented.
The planner schedules a PR carrying ready-for-agent for rework even when its CI is passing.
For a review-driven rework, the pr-fixer reads the PR's review comments, addresses them, pushes, then relabels ready-for-agent → ready-for-human with a summary comment.
docs/agents/triage-labels.md documents the review-driven rework path, the pull_request-trigger + reviews-API-check mechanism, and why pull_request_review cannot be used.

Blocked by

## What to build Add the second PR-rework signal on top of the CI-fail path (#20): when a human requests changes on an open PR, sandcastle labels it for the agent and the planner prioritises reworking it. **Trigger: `on: pull_request` + a reviews-API check.** `pull_request_review` is not a valid Forgejo Actions trigger, but `on: pull_request` fires when a comment is made on the PR in this instance, which covers a changes-requested review (reviews normally carry comments). The `pull_request` payload carries the PR, not the review state, so the workflow step must query the reviews API (`GET /repos/weiwen/evie/pulls/<n>/reviews`) and act only when the latest human review is `REQUEST_CHANGES`. End-to-end behaviour: - A **Forgejo Actions workflow** (`.github/workflows/pr-triage.yml`, since a Forgejo runner already executes that folder) triggers on `pull_request`. If the PR's latest human review is `REQUEST_CHANGES`, it adds `ready-for-agent` and removes `ready-for-human`. It must NOT label on approvals or ordinary comments. Authenticate with the runner's automatic token, falling back to a `FORGEJO_TOKEN` Actions secret if that token can't edit labels. - The **planner** gains the label arm: a PR carrying `ready-for-agent` needs rework (in addition to #20's `ci == "failure"`). - The **pr-fixer** gains a review-driven branch: read the changes-requested review body + comments (`tea pulls review-comments <n>` / reviews API), address them on the branch, push, then relabel `ready-for-agent` → `ready-for-human` and comment summarising the feedback addressed. The relabel is what stops the planner re-picking it. Accepted limitation: a changes-requested review submitted with no comment and no commit may not fire `pull_request`; the human can leave a comment to trigger, or the next interaction will. Document this. ## Acceptance criteria - [ ] A workflow under `.github/workflows` fires on `pull_request` and, when the PR's latest human review is `REQUEST_CHANGES`, adds `ready-for-agent` and removes `ready-for-human`. - [ ] The workflow does NOT apply the label on approvals or on comments that are not a changes-requested review. - [ ] The label-edit auth approach (automatic token vs `FORGEJO_TOKEN` secret) is settled and documented. - [ ] The planner schedules a PR carrying `ready-for-agent` for rework even when its CI is passing. - [ ] For a review-driven rework, the pr-fixer reads the PR's review comments, addresses them, pushes, then relabels `ready-for-agent` → `ready-for-human` with a summary comment. - [ ] `docs/agents/triage-labels.md` documents the review-driven rework path, the `pull_request`-trigger + reviews-API-check mechanism, and why `pull_request_review` cannot be used. ## Blocked by - #20

weiwen added the

ready-for-agent

label

2026-07-05 07:33:00 +08:00

weiwen referenced this issue

2026-07-05 07:35:50 +08:00

Sandcastle reworks PRs with failing CI #20

weiwen referenced this issue

2026-07-05 07:46:12 +08:00

Rename .github/ to .forgejo/ for clarity #22

weiwen commented

2026-07-05 18:51:35 +08:00

Author

Owner

Done (branch: sandcastle/issue-21)

All acceptance criteria addressed:

Workflow (.github/workflows/pr-triage.yml):

Triggers on on: pull_request
Queries the reviews API and acts only when the latest non-COMMENT/non-PENDING review is REQUEST_CHANGES
Adds ready-for-agent, removes ready-for-human; no-ops on approvals or ordinary comments
Auth: automatic GITHUB_TOKEN, falls back to FORGEJO_TOKEN secret

Planner (.sandcastle/plan-prompt.md):

Unified jq query emits both "ci-failure" and "review-requested-changes" items with a reworkReason field
If a PR has both signals (CI red + ready-for-agent), review takes priority

Orchestrator (.sandcastle/main.mts):

planSchema and PrReworkItem gain reworkReason: z.enum([...])
REWORK_REASON passed to pr-fixer as a promptArg

PR-fixer (.sandcastle/pr-fixer-prompt.md):

ci-failure branch: existing behaviour, no label changes
review-requested-changes branch: reads review comments, addresses feedback, pushes, relabels ready-for-agent → ready-for-human, posts summary

Docs (docs/agents/triage-labels.md):

Both rework paths documented
pull_request-trigger + reviews-API-check mechanism explained
Why pull_request_review cannot be used documented
Auth approach settled
Accepted limitation noted (comment-free reviews may not fire pull_request)

## Done (branch: sandcastle/issue-21) All acceptance criteria addressed: **Workflow** (.github/workflows/pr-triage.yml): - Triggers on `on: pull_request` - Queries the reviews API and acts only when the latest non-COMMENT/non-PENDING review is REQUEST_CHANGES - Adds ready-for-agent, removes ready-for-human; no-ops on approvals or ordinary comments - Auth: automatic GITHUB_TOKEN, falls back to FORGEJO_TOKEN secret **Planner** (.sandcastle/plan-prompt.md): - Unified jq query emits both "ci-failure" and "review-requested-changes" items with a reworkReason field - If a PR has both signals (CI red + ready-for-agent), review takes priority **Orchestrator** (.sandcastle/main.mts): - planSchema and PrReworkItem gain reworkReason: z.enum([...]) - REWORK_REASON passed to pr-fixer as a promptArg **PR-fixer** (.sandcastle/pr-fixer-prompt.md): - ci-failure branch: existing behaviour, no label changes - review-requested-changes branch: reads review comments, addresses feedback, pushes, relabels ready-for-agent → ready-for-human, posts summary **Docs** (docs/agents/triage-labels.md): - Both rework paths documented - pull_request-trigger + reviews-API-check mechanism explained - Why pull_request_review cannot be used documented - Auth approach settled - Accepted limitation noted (comment-free reviews may not fire pull_request)

weiwen referenced this issue from a commit

2026-07-05 18:57:45 +08:00

RALPH: feat: sandcastle reworks PRs with requested changes (closes #21)

weiwen referenced this issue from a pull request that will close it,

2026-07-05 18:58:21 +08:00

feat: sandcastle reworks PRs with requested changes #39

weiwen

2026-07-05 18:58:26 +08:00