Skip to content

fix: use pre-written app.ini mount for Gogs webhook support#82

Merged
Meldiron merged 4 commits intoutopia-php:copilot/add-gogs-adapter-and-tests-againfrom
jaysomani:fix/gogs-fixes-2
Mar 24, 2026
Merged

fix: use pre-written app.ini mount for Gogs webhook support#82
Meldiron merged 4 commits intoutopia-php:copilot/add-gogs-adapter-and-tests-againfrom
jaysomani:fix/gogs-fixes-2

Conversation

@jaysomani
Copy link
Contributor

No description provided.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e39b5a9860

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@greptile-apps
Copy link

greptile-apps bot commented Mar 24, 2026

Greptile Summary

This PR fixes Gogs webhook delivery in the test environment by pre-mounting a hand-crafted app.ini into the Gogs container instead of relying on the bootstrap script to patch it post-install. The key additions are LOCAL_NETWORK_ALLOWLIST = * (which allows Gogs to send webhooks to local Docker network addresses) and a pre-configured [webhook] section. The corresponding markTestSkipped override in GogsTest is removed so that testWebhookPushEvent is now exercised against Gogs.

Key changes:

  • docker-compose.yml: binds ./resources/gogs/app.ini into /data/gogs/conf/app.ini in the Gogs container.
  • resources/gogs/app.ini: new file with INSTALL_LOCK = true, LOCAL_NETWORK_ALLOWLIST = *, and the [webhook] section pre-configured.
  • tests/VCS/Adapter/GogsTest.php: removes the skipped-test override so webhook push delivery is now tested end-to-end.

Concerns:

  • The pre-mounted app.ini sets INSTALL_LOCK = true, but the bootstrap script still POSTs to /install to create the admin user. If Gogs 0.14 honours INSTALL_LOCK and rejects that POST on a fresh database, no admin user is created and the bootstrap fails at token generation — blocking the entire test run. This interaction should be verified or the admin-creation step should be replaced with a CLI approach (as used by the Gitea and Forgejo bootstraps).
  • The webhook-config append block in gogs-bootstrap (lines 176–178) is now dead code since the pre-mounted app.ini already contains [webhook].
  • resources/gogs/app.ini is missing a trailing newline.

Confidence Score: 3/5

  • The approach is sound but there is a plausible regression: INSTALL_LOCK=true in the pre-mounted app.ini may silently prevent the bootstrap from creating the Gogs admin user, causing the test service to never start.
  • The core idea of pre-mounting app.ini to fix webhook delivery is correct and the test removal is intentional. However, the interaction between INSTALL_LOCK=true and the existing /install POST in the bootstrap is unverified and could cause the bootstrap to exit with code 1, stalling all Gogs tests. Additionally, dead code was left in the bootstrap and the app.ini has minor quality issues (missing EOF newline).
  • docker-compose.yml — specifically the gogs-bootstrap command and whether the /install POST still works with INSTALL_LOCK=true pre-set.

Important Files Changed

Filename Overview
docker-compose.yml Adds a bind-mount of the pre-written app.ini into the Gogs container. The mount itself is correct, but the existing bootstrap logic that POSTs to /install to create the admin user may be silently broken by the INSTALL_LOCK=true setting now present in the pre-mounted config, and the webhook-append block is now dead code.
resources/gogs/app.ini New pre-written Gogs configuration file. Correctly sets INSTALL_LOCK, LOCAL_NETWORK_ALLOWLIST, and the [webhook] section needed for webhook delivery. Minor issues: missing trailing newline at EOF and a hardcoded placeholder SECRET_KEY.
tests/VCS/Adapter/GogsTest.php Removes the markTestSkipped override for testWebhookPushEvent, allowing the inherited GiteaTest implementation to run against Gogs now that webhook delivery is configured. Note that the inherited test reads the TESTS_GITEA_REQUEST_CATCHER_URL env var (not a Gogs-specific one), but since both point to the same request-catcher service this works correctly in practice.

Comments Outside Diff (2)

  1. docker-compose.yml, line 176-178 (link)

    P2 Dead code — webhook append block is now unreachable

    The pre-mounted resources/gogs/app.ini already contains the [webhook] section, so the grep check at line 176 will always evaluate to false and the printf append at line 177 will never execute. This block is now dead code and can be removed to keep the bootstrap script tidy.

  2. docker-compose.yml, line 157-172 (link)

    P1 INSTALL_LOCK may silently break admin user creation

    The bootstrap script still POSTs to /install to create the admin user, but the pre-mounted app.ini sets INSTALL_LOCK = true. In Gogs, this flag prevents the install endpoint from processing form submissions — the request is rejected, but || true at line 172 silently swallows that failure.

    The script then proceeds to create a token for the admin user (line 180). If the admin user was never created because the install POST was blocked, the token API returns a non-sha1 response, $$TOKEN will be null, and the bootstrap exits with code 1 (line 187). Because the tests service declares gogs-bootstrap: condition: service_completed_successfully, this causes the entire test run to stall.

    Please confirm that Gogs 0.14 still processes the /install POST when INSTALL_LOCK = true is pre-set on a fresh, empty database. If it does not, the admin user creation must be done via an alternative mechanism consistent with the other bootstraps — e.g. using the Gogs CLI admin create-user command (similar to how gitea-bootstrap and forgejo-bootstrap run su git -c "gitea admin user create ...").

Reviews (1): Last reviewed commit: "fix: use pre-written app.ini mount for G..." | Re-trigger Greptile

@Meldiron Meldiron added test Enables E2E tests in CI/CD and removed test Enables E2E tests in CI/CD labels Mar 24, 2026
@Meldiron Meldiron merged commit f9b144a into utopia-php:copilot/add-gogs-adapter-and-tests-again Mar 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test Enables E2E tests in CI/CD

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants