Fix duplicate script declaration error in pull request merge box by silverwind · Pull Request #36759 · go-gitea/gitea

silverwind · 2026-02-25T22:22:08Z

Move merge form JSON construction from template to Go, and clean up the data flow.

Replace inline <script> with data-merge-form JSON attribute on the Vue mount point
Move ~70-line template JsonUtils.EncodeToString logic into typed Go structs with json.Marshal (pull_merge_form.go)
Pre-compute allOverridableChecksOk and canMergeNow in Go, eliminating duplicate template logic
Add e2e tests for merge box: style switching, status check transitions, auto-merge, actual merge

🤖 Generated with Claude Code

Replace the inline <script> in the merge box template with a JSON data attribute. The previous approach used executeScripts() to re-append <script> tags to document.body on each merge box reload, but never removed old ones, causing "Identifier has already been declared" errors on the second reload. The merge form data is now passed via a data-merge-form attribute using dict/JsonUtils.EncodeToString, and read by JS via JSON.parse. This eliminates the need for executeScripts() entirely. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

silverwind · 2026-02-25T22:32:33Z

The lint-templates error is a false-positive in djlint, I've opened djlint/djLint#1678 and will set an exclusion.

The closing </div> is matched with an opening <div> inside an {{else}} branch, which djlint's regex parser cannot trace through Go template conditionals, causing a spurious "orphan tag" warning. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

templates/repo/issue/view_content/pull_merge_box.tmpl

…Data Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

templates/repo/issue/view_content/pull_merge_box.tmpl

wxiaoguang · 2026-02-27T03:40:07Z

Or you can use

<script type="module">
(function () => {
    const ....
})();

for a "quick" fix.

silverwind · 2026-02-27T07:47:41Z

No, I'm of the opionion all script tags must go, to allow running without unsafe-inline CSP.

* origin/main: Move Fomantic dropdown CSS to custom module (go-gitea#36530) Use "Enable Gravatar" but not "Disable" (go-gitea#36771) feat: add branch_count to repository API (go-gitea#35351) (go-gitea#36743) Deprecate RenderWithErr (go-gitea#36769) Lazy-load some Vue components, fix heatmap chunk loading on every page (go-gitea#36719) Filter out untracked files from spellchecking (go-gitea#36756) Fix CSS stacking context issue in actions log (go-gitea#36749) Fix milestone/project text overflow in issue sidebar (go-gitea#36741) Update tool dependencies and fix new lint issues (go-gitea#36702) Instance-wide (global) info banner and maintenance mode (go-gitea#36571) Add created_by filter to SearchIssues (go-gitea#36670) Inline and lazy-load EasyMDE CSS, fix border colors (go-gitea#36714) # Conflicts: # templates/repo/issue/view_content/pull_merge_box.tmpl # web_src/js/features/repo-issue-pull.ts

Move the ~70-line `JsonUtils.EncodeToString(dict ...)` template logic for the pull request merge form into a Go struct with `json.Marshal`, improving IDE support, type safety, and compile-time error checking. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

templates/repo/issue/view_content/pull_merge_box.tmpl

Signed-off-by: silverwind <me@silverwind.io>

wxiaoguang

No. don't do ctx.Data["RequireSigned"].(bool)

wxiaoguang · 2026-02-27T17:48:17Z

AI code is full of bad smells. Please make sure the code is maintainable.

It's even worse than before, and doesn't match the expectation:

then the backend code gets IDE highlights, strict syntax support, compiler time error check, easier to read and test and refactor.

All dynamic key access, no strict syntax, no compiler time error check, harder to read/test/refactor

Pass blocked-by values and other locals via mergeFormParams struct instead of re-reading from ctx.Data or making duplicate DB calls. Remove unused SliceUtils.Pack and djlint H025 exclude. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

silverwind · 2026-02-27T17:57:08Z

Better now?

wxiaoguang · 2026-02-27T17:59:30Z

Better now?

If you can understand the logic 2 months later, then it is better.

To me, it's just better than the worst.

silverwind · 2026-02-27T18:01:46Z

IDK, this is replacing a compact-ish json with almost 3 times as much go code. I would go back to template logic.

wxiaoguang · 2026-02-27T18:04:08Z

IDK, this is replacing a compact-ish json with almost 3 times as much go code. I would go back to template logic.

Because you duplicated the logic in go again.

To correctly refactor, the logic in templates (e.g.: canMergeNow) should be done in backend.

silverwind · 2026-02-27T19:55:12Z

Should be good now, I added e2e tests for the merge box as well.

Consolidate 7 separate tests into 3 by sharing repo setup across related assertions. Reduces test time from ~32s to ~13s by avoiding redundant repo/branch/PR creation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

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

wxiaoguang · 2026-02-28T03:36:52Z

Comments

dismiss

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

Copilot

Pull request overview

This PR fixes the duplicate script declaration issue in the PR merge box by removing inline <script> data injection and shifting merge-form data construction to Go, passing it to the Vue merge form via a data-merge-form JSON attribute.

Changes:

Replace merge-form inline <script> + executeScripts() re-execution with data-merge-form JSON on the Vue mount element.
Add Go-side merge-form JSON builder (pull_merge_form.go) and precompute AllOverridableChecksOk / CanMergeNow in Go.
Add Playwright e2e coverage for merge box UI behavior (style switching, status checks, auto-merge, merge completion) and supporting API helpers.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
web_src/js/features/repo-issue-pull.ts	Mount merge form with root-prop and remove dynamic script re-execution on merge-box reload.
web_src/js/components/PullRequestMergeForm.vue	Read merge-form data from `data-merge-form` on the mount element instead of `window.config.pageData`.
templates/repo/issue/view_content/pull_merge_box.tmpl	Render Vue mount point with `data-merge-form` and use server-computed mergeability flags.
routers/web/repo/pull_merge_form.go	New Go builder for merge-form JSON and mergeability flag computation.
routers/web/repo/pull.go	Return structured merge inputs from PR view prep to feed merge-form JSON builder.
routers/web/repo/issue_view.go	Centralize PR review/merge preparation and wire merge-form JSON builder into the view pipeline.
tests/e2e/utils.ts	Add API helpers for branch creation, PR creation, commit statuses, and branch protections.
tests/e2e/repo/pull-merge-box.test.ts	New e2e tests validating merge box rendering and behavior across key states.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

routers/web/repo/pull_merge_form.go

Rename GetCommitMessages to SquashCommitMessages since it holds a string value, not a getter. Rename DefaultMergeMessage to DefaultMergeBody in the JSON struct and Vue component to avoid confusion with the mergeFormParams.DefaultMergeMessage field which holds the commit title. Co-Authored-By: Claude claude-opus-4-6 20250627 <noreply@anthropic.com>

* origin/main: (27 commits) Fix OAuth2 authorization code expiry and reuse handling (go-gitea#36797) Fix org permission API visibility checks for hidden members and private orgs (go-gitea#36798) Fix non-admins unable to automerge PRs from forks (go-gitea#36833) upgrade to github.com/cloudflare/circl 1.6.3, svgo 4.0.1, markdownlint-cli 0.48.0 (go-gitea#36837) Fix dump release asset bug (go-gitea#36799) build(deps): update material-icon-theme v5.32.0 (go-gitea#36832) Fix bug to check whether user can update pull request branch or rebase branch (go-gitea#36465) Fix forwarded proto handling for public URL detection (go-gitea#36810) Fix artifacts v4 backend upload problems (go-gitea#36805) Add a git grep search timeout (go-gitea#36809) fix(repo): unify DEFAULT_SHOW_FULL_NAME output in templates and dropdown (go-gitea#36597) Harden render iframe open-link handling (go-gitea#36811) [skip ci] Updated translations via Crowdin fix: /repos/{owner}/{repo}/actions/{runs,jobs} requiring owner permissions (go-gitea#36818) Fix CRAN package version validation to allow more than 4 version components (go-gitea#36813) Fix API not persisting pull request unit config when has_pull_requests is not set (go-gitea#36718) feat: Add Actions API rerun endpoints for runs and jobs (go-gitea#36768) Fix bug when pushing mirror with wiki (go-gitea#36795) Pull Request Pusher should be the author of the merge (go-gitea#36581) Delete non-exist branch should return 404 (go-gitea#36694) ... # Conflicts: # routers/web/repo/issue_view.go

lunny · 2026-03-13T02:25:39Z

routers/web/repo/issue_view.go

 	}
-	preparePullViewPullInfo(ctx, issue)
-	preparePullViewReviewAndMerge(ctx, issue)
+	preparePullViewReviewAndMergeAll(ctx, issue)


if ctx.Written() { return }

missed here?

Resolve conflicts in: - tests/e2e/utils.ts: keep both branch's new API helpers and main's user/password helpers - web_src/js/components/PullRequestMergeForm.vue: use pageData instead of DOM attribute - web_src/js/features/repo-issue-pull.ts: use simplified import without props Co-Authored-By: Claude (Opus 4.6) <noreply@anthropic.com>

wxiaoguang

According to the recent experiences of PR reviewing, I don't trust the AI code here, it just looks like a new pile of slop.

Two real examples of AI slop:

They both: look good, seem running well for some cases, contain superficial tests. But the AI's solutions are completely wrong and make the situation worse.

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 25, 2026

github-actions bot added modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files modifies/frontend labels Feb 25, 2026

silverwind added the type/bug label Feb 25, 2026

silverwind mentioned this pull request Feb 25, 2026

JavaScript error: Failed to execute 'append' on 'Element': Identifier 'defaultMergeTitle' has already been declared (https://gitea.com/gitea/gitea-mcp/pulls/137 @ 1:1). Open browser console to see more details. #36753

Closed

lunny reviewed Feb 25, 2026

View reviewed changes

templates/repo/issue/view_content/pull_merge_box.tmpl Outdated Show resolved Hide resolved

Read merge form data from element props instead of window.config.page…

f7dcbd8

…Data Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

wxiaoguang reviewed Feb 26, 2026

View reviewed changes

templates/repo/issue/view_content/pull_merge_box.tmpl Outdated Show resolved Hide resolved

silverwind marked this pull request as draft February 26, 2026 23:48

silverwind and others added 2 commits February 27, 2026 18:14

silverwind commented Feb 27, 2026

View reviewed changes

templates/repo/issue/view_content/pull_merge_box.tmpl Outdated Show resolved Hide resolved

Apply suggestion from @silverwind

3cdf645

Signed-off-by: silverwind <me@silverwind.io>

silverwind marked this pull request as ready for review February 27, 2026 17:46

Merge branch 'main' into fixmerge

54dc981

wxiaoguang previously requested changes Feb 27, 2026

View reviewed changes

GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Feb 27, 2026

silverwind and others added 4 commits February 27, 2026 21:00

Optimize pull merge box e2e tests

ad0c12c

Consolidate 7 separate tests into 3 by sharing repo setup across related assertions. Reduces test time from ~32s to ~13s by avoiding redundant repo/branch/PR creation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Shorten e2e test names and use lowercase

b059455

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

Remove redundant recovery check in status checks e2e test

33e154a

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

fix lint

96e8b2a

silverwind force-pushed the fixmerge branch from c5fef62 to 96e8b2a Compare February 27, 2026 20:11

GiteaBot added lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. and removed lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged labels Feb 28, 2026

silverwind marked this pull request as draft February 28, 2026 04:57

silverwind and others added 4 commits March 1, 2026 14:45

Fix type assertion panic when HeadTarget is template.HTML

93fae30

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

Remove unused apiCreateFile and extract preparePullViewReviewAndMergeAll

4e72424

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

Remove unused clickDropdownItem utility function

ff3e554

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

Merge branch 'main' into fixmerge

885897b

silverwind marked this pull request as ready for review March 1, 2026 14:02

silverwind requested a review from Copilot March 1, 2026 14:02

Copilot started reviewing on behalf of silverwind March 1, 2026 14:03 View session

silverwind and others added 2 commits March 1, 2026 15:05

Add comments explaining merge permission logic

a826a20

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

Restore stillCanManualMerge closure

3e02cd4

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

Copilot AI reviewed Mar 1, 2026

View reviewed changes

routers/web/repo/pull_merge_form.go Show resolved Hide resolved

silverwind and others added 3 commits March 6, 2026 17:10

fmt

0affa9c

lunny reviewed Mar 13, 2026

View reviewed changes

wxiaoguang mentioned this pull request Apr 2, 2026

Fix various problems #37077

Merged

wxiaoguang requested changes Apr 5, 2026

View reviewed changes

GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 5, 2026

Uh oh!

Conversation

silverwind commented Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

silverwind commented Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wxiaoguang commented Feb 27, 2026

Uh oh!

silverwind commented Feb 27, 2026

Uh oh!

Uh oh!

wxiaoguang left a comment

Choose a reason for hiding this comment

Uh oh!

wxiaoguang commented Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

silverwind commented Feb 27, 2026

Uh oh!

wxiaoguang commented Feb 27, 2026

Uh oh!

silverwind commented Feb 27, 2026

Uh oh!

wxiaoguang commented Feb 27, 2026

Uh oh!

silverwind commented Feb 27, 2026

Uh oh!

wxiaoguang commented Feb 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

lunny Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

wxiaoguang left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

silverwind commented Feb 25, 2026 •

edited

Loading

silverwind commented Feb 25, 2026 •

edited

Loading

wxiaoguang commented Feb 27, 2026 •

edited

Loading

wxiaoguang commented Feb 28, 2026 •

edited

Loading