Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .codewith/skills/merge-pr/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ Actual merge means `immediate-merge`, `auto-merge`, or `merge-queue`.

1. For actual merge, create a native Codewith goal plan when available. Do not clear or replace an active goal unless the user explicitly authorizes it. The plan must track: `preflight`, `reviewer-artifact`, `executor-recheck-merge`, and `postverify`.
2. Require one independent reviewer artifact from the fixed reviewer run tied to the exact PR head SHA. Self-review is not an acceptable fallback for actual merge. If that independent artifact cannot be obtained, stop before merge. A coordinator may deliberately fix additional reviewers before review begins, but the skill never requires more than one; every supplied artifact must have a distinct reviewer identity or run id.
3. Each reviewer artifact must include: repository, PR number, exact head SHA, reviewer identity or run id, timestamp, verdict, checked risks summary, and blocking findings. Treat missing, invalid, future, or stale artifact timestamps as blockers; the helper default staleness window is 24 hours unless `--max-artifact-age-hours` is explicitly set.
3. Each reviewer artifact must bind one receipt containing: repository, PR number, reviewed base SHA, exact head SHA, merge-result tree SHA, reviewer identity and run id, receipt id, workflow-required provider/model, runtime provider/model/thread claims, timestamp, verdict, remediation cycle, checked risks summary, and blocking findings. The helper resolves the reviewer run id through package-owned `codewith agent read <run-id> --json`; that authoritative read must report the same run id, the same thread id claimed by the artifact, terminal `completed` state, an attestation for the same run, the workflow-required applied provider, and the workflow-required bound runtime model. Artifact-authored provenance fields are claims to compare, never authority. The provenance timestamp must match the review timestamp. Missing runtime state, failed reads, unavailable-provider substitution, mismatches, or missing/invalid/future/stale timestamps block merge; the helper default staleness window is 24 hours unless `--max-artifact-age-hours` is explicitly set.
4. The executor must re-fetch and re-check immediately before merge. A preflight JSON snapshot is advisory only and is not authority to merge.
5. Immediate squash merges must run through `scripts/merge_pr_execute.py`, which holds one exclusive `github-merge/OWNER/REPO/refs/heads/BASE` resource lock across authoritative recheck, exact base/head/tree validation, merge, and postverification. Pass `--lock-owner registered-agent/session-id`; the executable appends a unique invocation id so even parallel merges from one session cannot refresh the same ownership record.
6. The merge command must include `gh pr merge <pr> --match-head-commit <head_sha>` and must never include `--admin`.
Expand All @@ -35,7 +35,7 @@ Actual merge means `immediate-merge`, `auto-merge`, or `merge-queue`.
4. For actual merge, create or append to the native Codewith goal plan without replacing an active goal unless authorized. Record the PR head SHA in the plan.
5. Obtain one independent reviewer artifact for that exact head SHA from the reviewer fixed before review begins. Treat missing, stale, duplicate-run, self-review, or blocking-verdict artifacts as blockers. If the coordinator deliberately fixed additional reviewers before review began, validate every supplied artifact without turning the optional additions into a requirement.
6. Executor recheck immediately before merge:
- For immediate squash, invoke `scripts/merge_pr_execute.py` with the reviewed base SHA, head SHA, merge-result tree SHA, reviewer artifact, merge body file, and a unique lock owner. The executable acquires the stable repository/default-branch lock before the authoritative checks.
- For immediate squash, invoke `scripts/merge_pr_execute.py` with the reviewed base SHA, head SHA, merge-result tree SHA, reviewer artifact, workflow-declared `--required-reviewer-provider` and `--required-reviewer-model`, merge body file, and a unique lock owner. The executable refuses missing or mismatched reviewer route requirements before mutation, then acquires the stable repository/default-branch lock before the authoritative checks.
- `git fetch` or equivalent remote refresh.
- Re-read PR state, head SHA, base, mergeability, checks, reviews, draft/conflict state, and queue/protection behavior.
- Verify the current base SHA, head SHA, and computed merge-result tree still equal the reviewed candidate. If any changed, stop and get a new reviewer artifact from the fixed reviewer set.
Expand Down Expand Up @@ -88,5 +88,5 @@ Record the PR state, merged commit or queue state, target branch state, CI/check

Validate the contract with static checks and fixtures:

- Static checks: trigger text present, `scancommitpr` non-goal present, four modes present, one independent reviewer artifact required, no self-review fallback for actual merge, optional additional artifacts remain distinct, executor recheck required, stable branch lock required for immediate squash, exact base/head/tree required, `--match-head-commit` required, `--admin` forbidden, postverify fields present.
- Fixtures: green PR with one artifact, green PR with two distinct optional artifacts, no reviewer artifacts, malformed or stale artifact, wrong repository/PR/head artifact, duplicate reviewer identity, pending checks, failed checks, requested changes, draft PR, conflict PR, merge queue passed checks, merge queue pending checks, explicit merge-when-green, head SHA changes between preflight and executor, branch protection queue, unchanged-base immediate squash, two concurrent candidates on one repository/default branch, moved-base retry refusal, and no generated command containing `--admin`.
- Static checks: trigger text present, `scancommitpr` non-goal present, four modes present, one independent reviewer artifact required, no self-review fallback for actual merge, optional additional artifacts remain distinct, workflow-declared reviewer provider/model required, authoritative `codewith agent read` validation bound to the reviewer run, thread, attestation, and receipt claims, executor recheck required, stable branch lock required for immediate squash, exact base/head/tree required, `--match-head-commit` required, `--admin` forbidden, postverify fields present.
- Fixtures: required Fable plus OpenAI rejected, required Fable plus verified Fable accepted, unavailable required provider refusing substitution, forged artifact claims rejected by authoritative runtime readback, missing/failed/nonterminal reviewer runs rejected, missing or mismatched attestation/run/thread/provider/model rejected, green PR with one artifact, green PR with two distinct optional artifacts, no reviewer artifacts, malformed or stale artifact, wrong repository/PR/head/base/tree artifact, duplicate reviewer run, pending checks, failed checks, requested changes, draft PR, conflict PR, merge queue passed checks, merge queue pending checks, explicit merge-when-green, head SHA changes between preflight and executor, branch protection queue, unchanged-base immediate squash, two concurrent candidates on one repository/default branch, moved-base retry refusal, remediation cycles 0..2 preserved, and no generated command containing `--admin`.
2 changes: 1 addition & 1 deletion .codewith/skills/merge-pr/references/merge-safety.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ These rules keep merge execution tied to fresh evidence rather than stale snapsh
- Check summaries are decision evidence, not navigation evidence. Project GitHub check reads at the CLI boundary to `name,state,bucket`, retain only named decision fields, and never request or persist check links, `detailsUrl`, raw `statusCheckRollup`, or other URL-bearing check fields.
- Executor recheck is authoritative. Immediately before any actual merge action, re-fetch and re-read PR state, head SHA, mergeability, checks, reviews, draft/conflict state, and queue/protection behavior.
- Immediate squash merges use the package-owned `merge_pr_execute.py` path. It acquires one exclusive lock keyed to `github-merge/OWNER/REPO/refs/heads/BASE` before authoritative recheck and holds it through exact base/head/tree validation, merge, and postverification. The caller supplies its registered agent/session identity and the executable appends a unique invocation id, so parallel work from one session cannot refresh the same ownership record.
- Actual merge requires one independent reviewer artifact from the reviewer fixed before review begins for the exact PR head SHA. A coordinator may deliberately fix additional reviewers before review begins, but the skill never requires more than one and every supplied artifact must identify a distinct reviewer run. Missing, duplicate-run, self-review, stale-head, blocking-verdict, or blocking-finding artifacts stop the merge.
- Actual merge requires one independent reviewer artifact from the reviewer fixed before review begins. One validated receipt binds workflow-required provider/model, reviewer run and receipt ids, claimed runtime thread/provider/model, review timestamp, exact base/head/merge-result tree, verdict, and remediation cycle. The package-owned `codewith agent read <run-id> --json` result is the runtime authority: run id, thread id, completed state, attestation run id, applied provider, and bound runtime model must corroborate the artifact and workflow. Artifact-authored provenance booleans and strings are claims only. The executor fails closed on read failure, missing runtime state, unavailable-provider substitution, mismatch, duplicate runs, stale candidate identity, blocking verdict, or blocking findings. A coordinator may deliberately fix additional reviewers before review begins, but the skill never requires more than one and every supplied artifact must identify a distinct reviewer run.
- Reviewer artifact freshness is part of merge safety. Missing, invalid, future, or stale timestamps are blockers; the helper default staleness window is 24 hours unless explicitly overridden for a run.
- Every merge command must include `gh pr merge <pr> --match-head-commit <head_sha>`. This protects against merging a changed PR head after review.
- `--match-head-commit` pins only the PR head. GitHub's supported merge input exposes no expected base OID, so the branch lock serializes participating merge-pr executors but cannot exclude a nonparticipating external actor. Postverify must continue comparing the merged commit's first parent and tree with the reviewed base and tree, and must report any mismatch rather than treating the merge command's success as completion.
Expand Down
14 changes: 14 additions & 0 deletions .codewith/skills/merge-pr/scripts/merge_pr_execute.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,10 @@ def preflight_command(args: argparse.Namespace) -> list[str]:
]
for artifact in args.artifact:
command.extend(["--artifact", artifact])
command.extend(["--expected-base", args.expected_base])
command.extend(["--expected-tree", args.expected_tree])
command.extend(["--required-reviewer-provider", args.required_reviewer_provider])
command.extend(["--required-reviewer-model", args.required_reviewer_model])
command.extend(["--max-artifact-age-hours", str(args.max_artifact_age_hours)])
return command

Expand Down Expand Up @@ -715,6 +719,16 @@ def main() -> int:
default=[],
help="Independent reviewer artifact JSON; may be repeated",
)
parser.add_argument(
"--required-reviewer-provider",
required=True,
help="Workflow-declared reviewer provider required by the validated artifact",
)
parser.add_argument(
"--required-reviewer-model",
required=True,
help="Workflow-declared reviewer model required by the validated artifact",
)
parser.add_argument("--body-file", required=True, help="Merge body ending in one Agent trailer")
parser.add_argument("--subject", help="Optional squash merge subject")
parser.add_argument(
Expand Down
Loading
Loading