Skip to content
Merged
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
39 changes: 32 additions & 7 deletions .github/agents/dev-loop.agent.md
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,8 @@ pwsh -NoProfile -File .github/skills/evidence-capture/helpers/Publish-Evidence.p
gh run list --branch <branch-name> --limit 5
```

If CI fails, fix and push. Non-trivial fixes -> Phase 3.
If CI fails, fix and push. Non-trivial fixes -> Phase 3. If no runs appear because
hosted CI cannot run at all, see the hosted-CI-unavailable exception in Phase 8.

#### Step 5: Obtain an independent review

Expand Down Expand Up @@ -485,30 +486,53 @@ Append dry run results to PR body using `--body-file`. Construct the complete bo
from scratch -- never read-modify-write. See `copilot-instructions.md` > PR & Issue
Body Formatting.

**Exit criteria:** PR created, CI green, **an independent review (a reviewer that is not
**Exit criteria:** PR created, CI green (or the hosted-CI-unavailable exception
below applies), **an independent review (a reviewer that is not
the authoring model) read the latest diff and surfaced no new accepted Critical /
Important findings** -- via resolved Copilot threads or via the different-model path --
dry run passes (if applicable), no mojibake.

### Phase 8 -- Merge

Runs after Phase 7 exits cleanly. **Preconditions:** CI green, **an
Runs after Phase 7 exits cleanly. **Preconditions:** CI green (or the
hosted-CI-unavailable exception below applies), **an
independent review satisfied the invariant** (a reviewer that is not the
authoring model read the latest diff and surfaced no new accepted Critical /
Important findings -- a substituted different-model review satisfies this
exactly as a Copilot review does), all review threads resolved **when the review
used the Copilot transport** (there are no GitHub review threads to resolve on
the different-model path), dry run passes (if applicable).

**Merging is pre-authorized.** When the preconditions hold, merge, close the
issue, and run Phase 9 cleanup **without asking**, then report using the Task
Complete Summary Format -- the report replaces the permission prompt. The
independent review is mandatory; the permission is not: being unsure whether
the review gate held means finish the review, not ask about the merge. See
**Merging a Finished PR Is Pre-Authorized** in `CLAUDE.md`.

**Stop and ask only when:** the PR or its issue carries the `hold` label; the
reviewer's model is not recorded; the PR belongs to another session or another
author; or the change is outside the approved design.

This repo only allows **rebase merges**. Squash and merge-commit modes are
disabled. Use:

```powershell
gh pr merge <pr-number> --rebase --delete-branch
```

Never merge while CI is red. If a post-merge check fails, route back to
Phase 3 (TDD) on a new branch.
**Hosted CI that cannot run is not "CI red".** When hosted CI cannot run at all,
a recorded local run of the same checks satisfies the gate, provided the switch
to local CI is developer-confirmed and recorded and the PR carries the runner's
output with real counts. The confirmation must be verifiable by any session or
reviewer -- a linked, timestamped developer comment cited in the PR, or the
repository's own committed instructions or config; a personal or session-local
memory note does not count. While on local CI, hosted CI is rechecked daily and
the repository switches back as soon as it works -- the shared local-CI runner's
job, not implemented by hand. If hosted CI ran and failed, that is a hard stop unless
the identical failure is shown, with evidence in the PR, to be pre-existing on
`main`. If a post-merge check fails, route back to Phase 3 (TDD) on a new
branch.

**Exit criteria:** PR merged, remote feature branch deleted, ``main`` contains
the change.
Expand Down Expand Up @@ -586,6 +610,7 @@ Once Phase 7 passes with zero unresolved threads and a successful dry run:
linked issue number, independent-review status (Copilot threads or the
different-model reviewer that stood in for them).
5. **Execute Phase 8 (Merge)** -- rebase-merge the PR with
``gh pr merge <pr-number> --rebase --delete-branch``. Never merge while CI
is red or with unresolved review threads.
``gh pr merge <pr-number> --rebase --delete-branch``, without asking. Never
merge while hosted CI is red (see Phase 8 for when hosted CI cannot run) or
with unresolved review threads.
6. Execute Phase 9 (Cleanup) commands with actual values (no placeholders).
158 changes: 158 additions & 0 deletions .github/agents/tests/merge-preauthorized.Tests.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
#Requires -Version 7.0
#Requires -Modules @{ ModuleName = 'Pester'; ModuleVersion = '5.0.0' }

# Structural tests for "merging a finished PR is pre-authorized" (issue #499).
#
# Sessions stopped at the end of a finished dev loop to ask permission to merge,
# even though Phase 8 was already autonomous. Three causes: no explicit durable
# authorization (so a harness's confirm-outward-actions default won), a CI gate
# that cannot be met when hosted CI cannot run at all, and the mandatory review
# being conflated with an optional permission. These tests pin the statement of
# each fix in all three instruction files, scoped to the section that carries
# it, so a revert of any one file fails here.

BeforeAll {
$script:RepoRoot = Resolve-Path (Join-Path $PSScriptRoot '..\..\..\') | Select-Object -ExpandProperty Path
$script:ClaudeMd = Join-Path $script:RepoRoot 'CLAUDE.md'
$script:DevLoop = Join-Path $script:RepoRoot '.github/agents/dev-loop.agent.md'
$script:CopilotIns = Join-Path $script:RepoRoot '.github/copilot-instructions.md'

# A section is its heading line plus every line up to the next heading of
# the same or a higher level.
function Get-Section {
param([string]$Path, [string]$HeadingPattern)
$lines = Get-Content -LiteralPath $Path
$start = -1; $level = 0
for ($i = 0; $i -lt $lines.Count; $i++) {
if ($lines[$i] -match "^(#{1,6})\s+$HeadingPattern") {
$start = $i; $level = $Matches[1].Length; break
}
}
if ($start -lt 0) { return $null }
$end = $lines.Count
for ($j = $start + 1; $j -lt $lines.Count; $j++) {
if ($lines[$j] -match '^(#{1,6})\s' -and $Matches[1].Length -le $level) { $end = $j; break }
}
return ($lines[$start..($end - 1)]) -join "`n"
}

# The three places a session reads the merge rule, each scoped to the
# section that states it.
$script:Sections = @{
'CLAUDE.md merge section' = Get-Section -Path $script:ClaudeMd -HeadingPattern 'Merging\s+a\s+Finished\s+PR\s+Is\s+Pre-Authorized'
'copilot-instructions.md Merge Step' = Get-Section -Path $script:CopilotIns -HeadingPattern 'Merge\s+Step'
'dev-loop.agent.md Phase 8' = Get-Section -Path $script:DevLoop -HeadingPattern 'Phase\s+8\b'
}
}

Describe 'each merge rule states the authorization, the report, and the exceptions' -ForEach @(
@{ Name = 'CLAUDE.md merge section' }
@{ Name = 'copilot-instructions.md Merge Step' }
@{ Name = 'dev-loop.agent.md Phase 8' }
) {
BeforeAll { $script:Text = $script:Sections[$Name] }

It '<Name> exists' {
$script:Text | Should -Not -BeNullOrEmpty
}

It '<Name> states merging is pre-authorized, without asking' {
$script:Text | Should -Match '(?i)pre-authori[sz]ed'
$script:Text | Should -Match '(?i)without\s+asking'
}

It '<Name> replaces the permission prompt with a report' {
$script:Text | Should -Match '(?i)then\s+report'
$script:Text | Should -Match '(?i)Task\s+Complete\s+Summary'
}

It '<Name> keeps the independent review mandatory while dropping the permission' {
$script:Text | Should -Match '(?i)independent\s+review\s+is\s+(still\s+)?mandatory'
$script:Text | Should -Match '(?i)permission\s+is\s+not'
}

It '<Name> names the hold label as a reason to stop' {
$script:Text | Should -Match '`hold`'
}

It '<Name> names missing reviewer-model evidence as a reason to stop' {
$script:Text | Should -Match '(?i)reviewer(''s)?\s+model\s+(is\s+)?not\s+recorded|reviewing\s+model\s+(is\s+)?not\s+recorded'
}

It '<Name> names another session''s or author''s PR as a reason to stop' {
$script:Text | Should -Match '(?i)another\s+session'
}

It '<Name> names an out-of-design change as a reason to stop' {
$script:Text | Should -Match '(?i)outside\s+the\s+approved\s+design'
}

It '<Name> lets a developer-confirmed local CI run stand in when hosted CI cannot run' {
$script:Text | Should -Match '(?i)hosted\s+CI\s+(cannot|can''t)\s+run'
$script:Text | Should -Match '(?i)developer-confirmed'
$script:Text | Should -Match '(?i)real\s+counts'
}

It '<Name> keeps a hosted CI run that failed as a hard stop' {
$script:Text | Should -Match '(?i)hosted\s+CI\s+ran\s+and\s+failed'
$script:Text | Should -Match '(?i)pre-existing\s+on\s+`?main`?'
}

It '<Name> requires the local-CI confirmation to be verifiable by any session' {
# "Recorded" alone let a personal memory note count as the record --
# something no other session or reviewer can see.
$script:Text | Should -Match '(?i)verifiable\s+by\s+any\s+session'
$script:Text | Should -Match '(?i)timestamped\s+developer\s+comment'
$script:Text | Should -Match '(?i)committed\s+instructions\s+or\s+config'
$script:Text | Should -Match '(?i)(personal|session-local)\s+memory\s+note\s+does\s+not\s+count'
}

It '<Name> names the daily hosted-CI recheck and leaves its mechanics to the runner' {
$script:Text | Should -Match '(?i)re-?checked\s+daily'
$script:Text | Should -Match '(?i)switch(es)?\s+back'
$script:Text | Should -Match '(?i)not\s+implemented\s+by\s+hand'
}
}

Describe 'the hosted-CI exception reaches the other CI checkpoints' {
It 'the dev-loop-phase-gate Phase 7 checklist qualifies its CI item and points at CLAUDE.md' {
$gate = Get-Section -Path (Join-Path $script:RepoRoot '.github/skills/dev-loop-phase-gate/SKILL.md') -HeadingPattern 'After\s+Phase\s+7'
$gate | Should -Not -BeNullOrEmpty
$ciItem = [regex]::Match($gate, '(?ims)^- \[ \] CI workflows are green.*?(?=^- \[ \]|\z)').Value
$ciItem | Should -Not -BeNullOrEmpty
$ciItem | Should -Match '(?i)hosted\s+CI\s+(cannot|can''t)\s+run'
$ciItem | Should -Match 'CLAUDE\.md'
}

It 'dev-loop Phase 7 step 4 cross-references the Phase 8 exception' {
$step4 = Get-Section -Path $script:DevLoop -HeadingPattern 'Step\s+4:\s+Verify\s+CI'
$step4 | Should -Not -BeNullOrEmpty
$step4 | Should -Match '(?i)hosted\s+CI\s+(cannot|can''t)\s+run'
$step4 | Should -Match '(?i)Phase\s+8'
}

It '<File> qualifies every bare "CI green" precondition' -ForEach @(
@{ File = '.github/agents/dev-loop.agent.md' }
@{ File = '.github/copilot-instructions.md' }
) {
$raw = Get-Content -LiteralPath (Join-Path $script:RepoRoot $File) -Raw
$bare = [regex]::Matches($raw, '(?i)CI\s+green(?!\s+\(or\s+the\s+hosted-CI-unavailable\s+exception)')
$bare.Count | Should -Be 0 -Because "every 'CI green' in $File must carry the hosted-CI-unavailable qualifier"
}
}

Describe 'no bare "CI is red" rule survives without the hosted-CI qualification' {
# A bare "never merge while CI is red" is the rule a session could not
# satisfy when hosted CI cannot run at all. Every paragraph that still says
# it must also say what hosted CI being unavailable means.
It '<File> qualifies every CI-is-red paragraph' -ForEach @(
@{ File = 'CLAUDE.md' }
@{ File = '.github/copilot-instructions.md' }
@{ File = '.github/agents/dev-loop.agent.md' }
) {
$raw = Get-Content -LiteralPath (Join-Path $script:RepoRoot $File) -Raw
$paragraphs = $raw -split '(?:\r?\n){2,}'
$offending = @($paragraphs | Where-Object { $_ -match '(?i)CI\s+is\s+red' -and $_ -notmatch '(?i)hosted\s+CI' })
$offending | Should -BeNullOrEmpty -Because "each 'CI is red' paragraph in $File must say how an unavailable hosted CI is handled"
}
}
31 changes: 27 additions & 4 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -386,11 +386,13 @@ Brainstorm+Issue -> Worktree -> Plan -> [TDD -> Refactor -> Functional Test -> E
#### CI Failure Restart Loop

After pushing to a PR branch, if CI fails: investigate, fix locally, push again.
A PR must **never** be merged while CI is red.
A PR must **never** be merged while hosted CI is red. When hosted CI cannot run
at all, see the Merge Step below.

#### Merge Step

Once the expanding loop exits cleanly (CI green, all review threads resolved,
Once the expanding loop exits cleanly (CI green (or the hosted-CI-unavailable
exception below applies), all review threads resolved,
**an independent review satisfied the invariant** -- a reviewer that is not the
authoring model read the latest diff and surfaced no new accepted Critical /
Important findings -- dry run passes if applicable), merge the PR before running
Expand All @@ -399,14 +401,35 @@ different model than the authoring one satisfies it equally. See the
**Independent Code Review -- Reviewer != Author Model** section in `CLAUDE.md`
for the canonical rule, the substitution trigger, and the detection call.

**Merging is pre-authorized.** When those criteria hold, merge, close the issue,
and clean up the worktree **without asking**, then report using the **Task
Complete Summary Format** below -- the report replaces the permission prompt and
is not optional. The independent review is mandatory; the permission is not. See
**Merging a Finished PR Is Pre-Authorized** in `CLAUDE.md`.

**This repo only allows rebase merges:**

```powershell
gh pr merge <pr-number> --rebase --delete-branch
```

Never merge while CI is red. Never merge with unresolved review threads. If any
post-merge check fails, route back to Phase 3 (TDD) on a new branch.
Never merge with unresolved review threads. **Hosted CI that cannot run is not
"CI red"**: when hosted CI cannot run at all, a recorded local run of the same
checks satisfies the gate, provided the switch to local CI is
developer-confirmed and recorded and the PR carries the runner's output with
real counts. The confirmation must be verifiable by any session or reviewer -- a
linked, timestamped developer comment cited in the PR, or the repository's own
committed instructions or config; a personal or session-local memory note does
not count. While on local CI, hosted CI is rechecked daily and the repository
switches back as soon as it works -- the shared local-CI runner's job, not
implemented by hand. If hosted CI ran and failed, that is a hard stop unless the
identical failure is shown, with evidence in the PR, to be pre-existing on
`main`. If any post-merge check fails, route back to Phase 3 (TDD) on a new
branch.

**Stop and ask only when** the PR or its issue carries the `hold` label; the
reviewer's model is not recorded; the PR belongs to another session or another
author; or the change is outside the approved design.

Use `@plan` when exploring a new idea before committing to implementation.
Use `@systematic-debugging` (or the `systematic-debugging` skill) for bugs.
Expand Down
4 changes: 3 additions & 1 deletion .github/skills/dev-loop-phase-gate/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,9 @@ npx tsc && npx vitest run
### After Phase 7 (PR + Independent Review)

- [ ] PR created with `Closes #<issue-number>`
- [ ] CI workflows are green
- [ ] CI workflows are green -- or, when hosted CI cannot run at all, the
developer-confirmed local-CI exception in `CLAUDE.md` (**Merging a
Finished PR Is Pre-Authorized**) applies
- [ ] An independent review (a reviewer that is **not** the authoring model)
read the latest diff and surfaced no new accepted Critical / Important
findings
Expand Down
44 changes: 43 additions & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,47 @@ An **empty** result means Copilot review is not enabled -- fall through to the
different-model path immediately rather than polling for a review that will
never arrive.

## Merging a Finished PR Is Pre-Authorized

**When the dev loop's exit criteria hold, merge the PR, close its issue, and
clean up the worktree without asking -- then report.** This is the durable
authorization: do not seek per-PR confirmation. A merge is outward-facing, and
agent harnesses default to confirming such actions unless durably authorized;
this section is that authorization.

**The independent review is mandatory; the permission is not.** Merging without
asking weakens no gate: the different-model review above, behavior-first tests,
and evidence capture must all still hold. Being unsure whether the review gate
held is a reason to finish the review, not to ask about the merge.

**Then report**, using the Task Complete Summary Format (below): what was
implemented, the PR and issue links, the result display, **Assumptions**, and
anything that needs the user. The report replaces the permission prompt; it is
not optional.

**Hosted CI that cannot run is not "CI red".** When hosted CI cannot run at all
(billing block, quota, outage), a recorded local run of the same checks
satisfies the CI gate -- provided the repository's switch to local CI is
developer-confirmed and recorded, and the PR carries the local runner's output
with real counts pasted in. Whether hosted CI is unavailable is not a judgement a
session makes on its own: the developer's confirmation must be verifiable by any
session or reviewer -- a linked, timestamped developer comment cited in the PR,
or the repository's own committed instructions or config. A personal or
session-local memory note does not count. While a repository is on local CI,
hosted CI is rechecked daily and the repository switches back as soon as it
works; that belongs to the shared local-CI runner and is not implemented by hand
in a session. If hosted CI ran and failed, that is still a hard stop
-- unless the identical failure is shown, with evidence in the PR, to be
pre-existing on `main`.

**Stop and ask only when:**

- the PR or its issue carries the `hold` label, or is otherwise explicitly held;
- the independent-review evidence cannot be found -- the reviewer's model is not
recorded;
- the PR belongs to another session or another author;
- the change is outside the approved design and needs an owner decision.

## Key References

- **Development conventions**: [`.github/copilot-instructions.md`](./.github/copilot-instructions.md) -- code style, testing conventions, branching strategy, commit format, and workflow.
Expand Down Expand Up @@ -300,7 +341,8 @@ See the **Task Complete Summary Format** subsection of
- Commit format: `type(scope): description` (Conventional Commits)
- Merge to `main` only via pull request after the dev loop passes. **This repo
only allows rebase merges** -- use `gh pr merge <pr-number> --rebase --delete-branch`.
Never merge while CI is red.
Merging is pre-authorized -- do not ask. Never merge while hosted CI is red; for
when hosted CI cannot run, see **Merging a Finished PR Is Pre-Authorized**.
- **All commits must come from a worktree** — the pre-commit hook blocks commits from the repo root.
See the "Concurrent Session Safety" section in `.github/copilot-instructions.md` for details.
- **After a PR closes**, clean up the worktree and local branch. The recommended
Expand Down
Loading