feat(ui): action keys, confirm dialog and bulk merge with the merge contract #31

deadstyle · 2026-10-02T14:31:24-05:00

deadstyle commented

2026-10-02 14:31:24 -05:00

Summary

The action keys for #9: m merge, a approve, x close, c comment (in $EDITOR), o open in browser, R open the run's page, r refresh, space to mark PRs and esc to clear marks, plus bulk merge. Merges follow the safety rules from the plan review and the PM's decisions in #13.

Changes

Core merge contract (ADR 0011):
- Every m re-checks each target first. The confirm dialog records each PR's head commit, and a PR that changed after you confirmed is refused instead of merged.
- Every target gets exactly one result:
  - merged
  - refused, with the forge's reason (branch protection, conflicts, the CI rule)
  - failed
  - unknown, meaning it timed out and the server may have merged it anyway
  - not started
- Bulk merges carry on past a failure and run at most four at a time. esc stops new merges from starting, and merges already running finish and report.
- Failed, unknown and not-started PRs stay marked, so m retries just those. The re-check finds any that were actually merged and unmarks them.
CI: the dialog warns about failing or running CI and still allows the merge. With "only merge when CI is green" on for the host or repo, it refuses instead. Cancelled CI counts as not green.
Merge strategy: always the repo's default. The dialog names it (for example "squash") when the forge reports it.
forge.ErrRefused: the Forgejo adapter maps 405 and 409 refusals (other than "head out of date") to it, keeping the server's message.
UI:
- The confirm and result dialogs list each PR once.
- c opens $EDITOR, and empty content cancels.
- o and R use the platform opener.
- Keys are hidden when the forge or your access doesn't allow them.
- Status-bar hints follow the open dialog.
cmd: the "only merge when green" setting is passed in from the selected host's config. Demo mode leaves it off, so actions can be tried safely on fake data.

Screens (`lazyforge --demo`, 80×24)

lazyforge forge.home.arpa › home/homelab › [1] Pull requests › #41







                ╭─ Merge 2 PRs ───────────────────────────────╮
                │ Strategy: repo default                      │
                │ #42 chore(deps): update postgres to v17.0 ✓ │
                │ #41 chore(deps): update traefik to v3.2.0 ✗ │
                │   CI failing, will merge anyway             │
                │                                             │
                │ y/enter confirm · esc cancel                │
                ╰─────────────────────────────────────────────╯







 BOXES  y/enter confirm · esc cancel

lazyforge forge.home.arpa › home/homelab › [1] Pull requests › #39







                 ╭─ Merge 2 PRs ─────────────────────────────╮
                 │ #42 chore(deps): update postgres to v17.0 │
                 │   merged                                  │
                 │ #41 chore(deps): update traefik to v3.2.0 │
                 │   merged                                  │
                 │                                           │
                 │ esc/enter close                           │
                 ╰───────────────────────────────────────────╯







 BOXES  esc/enter close                                            Merged 2 of 2

Test plan

Tests cover every result type, cross-repo dedup, the per-repo CI gate (warn vs refuse), freshness through the head commit, re-check before retry, cancellation, the concurrency bound (deterministic, no sleeps), cache updates after close and comment, key gating, dialog contents, marks, comment cancel, and the opener seam.
Mutation checks: core 8 of 8 caught, UI about 35 caught (2 equivalent).
Reviews:
- Spec (opus) and standards passed after two fix rounds. One fix was a late re-check that could replace an open dialog and hide the per-PR results.
- Comments and test quality passed after two fix rounds, which also made a timing-based concurrency test deterministic.
make check exit 0: format, lint (0 issues), go test -race, govulncheck, build, install test.

Rulings

forge.ErrRefused exists, and every m re-checks its targets first.
"Not started" PRs stay marked. The dialog names the repo's merge style.
c follows what the forge allows, so it's visible with read access, because Forgejo lets readers comment. A scope note is on the ticket.
The refusal mapping applies to every Forgejo endpoint, because a 405 or a non-head 409 means a refusal wherever it occurs.
The concurrency test proves merges reach the four-at-a-time bound, but not that they never exceed it. Proving that would need a timed wait, and the bound is the core's shared limiter, already tested in #7.

Closes #9

🤖 Generated with Claude Code

## Summary The action keys for #9: `m` merge, `a` approve, `x` close, `c` comment (in `$EDITOR`), `o` open in browser, `R` open the run's page, `r` refresh, `space` to mark PRs and `esc` to clear marks, plus bulk merge. Merges follow the safety rules from the plan review and the PM's decisions in #13. ## Changes - **Core merge contract** (ADR 0011): - Every `m` re-checks each target first. The confirm dialog records each PR's head commit, and a PR that changed after you confirmed is refused instead of merged. - Every target gets exactly one result: - merged - refused, with the forge's reason (branch protection, conflicts, the CI rule) - failed - unknown, meaning it timed out and the server may have merged it anyway - not started - Bulk merges carry on past a failure and run at most four at a time. `esc` stops new merges from starting, and merges already running finish and report. - Failed, unknown and not-started PRs stay marked, so `m` retries just those. The re-check finds any that were actually merged and unmarks them. - **CI:** the dialog warns about failing or running CI and still allows the merge. With "only merge when CI is green" on for the host or repo, it refuses instead. Cancelled CI counts as not green. - **Merge strategy:** always the repo's default. The dialog names it (for example "squash") when the forge reports it. - `forge.ErrRefused`: the Forgejo adapter maps 405 and 409 refusals (other than "head out of date") to it, keeping the server's message. - **UI:** - The confirm and result dialogs list each PR once. - `c` opens `$EDITOR`, and empty content cancels. - `o` and `R` use the platform opener. - Keys are hidden when the forge or your access doesn't allow them. - Status-bar hints follow the open dialog. - `cmd`: the "only merge when green" setting is passed in from the selected host's config. Demo mode leaves it off, so actions can be tried safely on fake data. ## Screens (`lazyforge --demo`, 80×24) ``` lazyforge forge.home.arpa › home/homelab › [1] Pull requests › #41 ╭─ Merge 2 PRs ───────────────────────────────╮ │ Strategy: repo default │ │ #42 chore(deps): update postgres to v17.0 ✓ │ │ #41 chore(deps): update traefik to v3.2.0 ✗ │ │ CI failing, will merge anyway │ │ │ │ y/enter confirm · esc cancel │ ╰─────────────────────────────────────────────╯ BOXES y/enter confirm · esc cancel lazyforge forge.home.arpa › home/homelab › [1] Pull requests › #39 ╭─ Merge 2 PRs ─────────────────────────────╮ │ #42 chore(deps): update postgres to v17.0 │ │ merged │ │ #41 chore(deps): update traefik to v3.2.0 │ │ merged │ │ │ │ esc/enter close │ ╰───────────────────────────────────────────╯ BOXES esc/enter close Merged 2 of 2 ``` ## Test plan - Tests cover every result type, cross-repo dedup, the per-repo CI gate (warn vs refuse), freshness through the head commit, re-check before retry, cancellation, the concurrency bound (deterministic, no sleeps), cache updates after close and comment, key gating, dialog contents, marks, comment cancel, and the opener seam. - Mutation checks: core 8 of 8 caught, UI about 35 caught (2 equivalent). - Reviews: - Spec (opus) and standards passed after two fix rounds. One fix was a late re-check that could replace an open dialog and hide the per-PR results. - Comments and test quality passed after two fix rounds, which also made a timing-based concurrency test deterministic. - `make check` exit 0: format, lint (0 issues), `go test -race`, govulncheck, build, install test. ## Rulings - `forge.ErrRefused` exists, and every `m` re-checks its targets first. - "Not started" PRs stay marked. The dialog names the repo's merge style. - `c` follows what the forge allows, so it's visible with read access, because Forgejo lets readers comment. A scope note is on the ticket. - The refusal mapping applies to every Forgejo endpoint, because a 405 or a non-head 409 means a refusal wherever it occurs. - The concurrency test proves merges reach the four-at-a-time bound, but not that they never exceed it. Proving that would need a timed wait, and the bound is the core's shared limiter, already tested in #7. Closes #9 🤖 Generated with [Claude Code](https://claude.com/claude-code)

deadstyle added 12 commits

2026-10-02 14:31:24 -05:00

feat(core): merge contract, mutations and forge.ErrRefused d13ab0701f

Service.Merge returns a result per target (merged, refused, failed,
unknown, not started), dedups targets, enforces the green-CI gate injected
from config by cmd, and lets started merges finish when ctx is cancelled.
Recheck reconciles targets before a merge. Approve, Close and Comment share
the semaphore and update the cache. The gitea adapter maps 405 and non-head
409s to forge.ErrRefused, and domain.Repo gains MergeStyle. ADR 0011.

#9

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

feat(ui): merge, approve, close, comment and open actions c40f6312f7

Adds the action keys (m, space, a, x, c, o, R), gated per repo and
selection, the modal confirm/running/done dialog for merge and close,
per-repo marks for bulk merge, $EDITOR comments and the browser opener.
Every m rechecks its targets first, so retries reconcile.

#9

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

refactor(domain): share the green-CI check between core and ui a8ac2cf65c

#9: core's merge gate and the UI's refuse/warn line each carried their
own CI state list; both now call CIState.Green.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

fix(ui): guard late rechecks and tidy action dialogs 7d953c94a5

Drop a recheckedMsg that lands while a dialog is open, show the open
dialog's keys in the status bar, and give dialog an explicit isClose
discriminator. actionDoneMsg gets a closed flag instead of matching the
verb string. Comma-ok assertions replace unchecked ones in ui and core,
the gitea refusal case falls through to one Errorf, esc's help text is
now "dismiss", and ADR 0011 records why action messages carry the repo.

#9

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

fix(gitea): return ErrRefused inline to fix errorlint regression 68eb8e4ab8

Return the ErrRefused error directly in the switch case instead of assigning
to a sentinel and checking it later. This addresses the errorlint violation
at line 138 (comparing sentinel with == rather than using errors.Is).
Behavior is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

#9

test(core): edge cases for merge dedup, gate scope and failed mutations d1925ed29c

Covers #9: per-repo dedup and CI gate, Fake refusal reason, recheck and
mutation failures leaving the cache alone, and cmd wiring of require_green_ci.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

test(core): pass ctx to cache seeding in mutation test 399343f7ff

#9

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

test(ui): cover merge, close, approve, comment and open actions 9d8b25cf7e

Backfills the S2 acceptance criteria: key gating, marks, the confirm and
result dialog, recheck freshness, retry reconcile, esc cancel, editor
result, opener and stale results, and dialog layout.

#9

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

chore: merge core tests (#9 ) 9b40d289a9

chore: merge ui tests (#9 ) eda2b862cf

test(core,ui): deterministic merge bound, dedup via Update; trim comments 617ccef6a8

Refs #9. Phase 9 fixes: TestMergeBound blocks on channels, dedup test
drives recheckedMsg and the rendered dialog, comment cleanup.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

test(core): fix TestMergeBound goroutine Fatal and drop duplicate field comments

ci / check (pull_request) Successful in 1m4s

Details

b1beb5c83e

Build targets before starting the merge goroutine and release blocked
merges via t.Cleanup so a failed assertion can't leak goroutines. Remove
field comments that repeated the type docs.

#9

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

deadstyle merged commit 0b2c232529 into develop

2026-10-02 14:32:46 -05:00

deadstyle deleted branch feature/9-actions

2026-10-02 14:32:46 -05:00

deadstyle referenced this pull request

2026-10-02 14:32:47 -05:00

Actions: merge, approve, close, comment, open, refresh, bulk merge #9

deadstyle referenced this pull request from a commit

2026-10-02 14:32:47 -05:00

feat(ui): action keys, confirm dialog and bulk merge with the merge contract (#31)

Sign in to join this conversation.

No reviewers