Every page's Content-Security-Policy forbids eval, which the standard Alpine.js build needs to run directives, so in a browser none ran: both add forms on the webhook page showed open, and events in the event log could not be collapsed.
The UI now loads Alpine's CSP build. The @alpinejs/csp 3.14.9 tarball replaces the alpinejs one in 3p/, and script/assets extracts it as before; the policy is unchanged. That build cannot run expressions, so each directive in templates/ now names a property or method of a component registered in static/js/app.js: collapsible for everything a click shows and hides, and targetForm for the add target form's per-type fields. Each card on the webhook page now holds its own x-data instead of the page-wide one, so open UI branches should expect small conflicts.
A browser test in internal/server loads the webhook page and the event log under the real headers, and fails on the old tree. It is built only with the browser build tag: make check and the image build lint it but do not run it, and make test leaves it out. make test-browser runs it in Docker, in a headless browser image pinned by digest (Dockerfile.browser). The README lists what it checks.
Deviation: the type select's x-model became @change; the 3.14.9 CSP build cannot assign through x-model.
New test-only dependency chromedp; it raises golang.org/x/sys to 0.47.0.
The registry publishes a sha512 integrity, not a sha256; the tarball matches it.
Model: opus-5-5
Every page's Content-Security-Policy forbids eval, which the standard Alpine.js build needs to run directives, so in a browser none ran: both add forms on the webhook page showed open, and events in the event log could not be collapsed.
The UI now loads Alpine's CSP build. The `@alpinejs/csp` 3.14.9 tarball replaces the `alpinejs` one in `3p/`, and `script/assets` extracts it as before; the policy is unchanged. That build cannot run expressions, so each directive in `templates/` now names a property or method of a component registered in `static/js/app.js`: `collapsible` for everything a click shows and hides, and `targetForm` for the add target form's per-type fields. Each card on the webhook page now holds its own `x-data` instead of the page-wide one, so open UI branches should expect small conflicts.
A browser test in `internal/server` loads the webhook page and the event log under the real headers, and fails on the old tree. It is built only with the `browser` build tag: `make check` and the image build lint it but do not run it, and `make test` leaves it out. `make test-browser` runs it in Docker, in a headless browser image pinned by digest (`Dockerfile.browser`). The README lists what it checks.
- Deviation: the type select's `x-model` became `@change`; the 3.14.9 CSP build cannot assign through `x-model`.
- New test-only dependency `chromedp`; it raises `golang.org/x/sys` to 0.47.0.
- The registry publishes a sha512 integrity, not a sha256; the tarball matches it.
Model: opus-5-5
clawbot
self-assigned this 2026-10-02 00:23:07 +02:00
README.md, "Third-party browser assets": the sentence saying the browser test "fails on any directive that does not work" is not true. The test never opens a delivery's attempts on the event log (it seeds no target or delivery) and never uses the mobile menu, so a broken directive in either still passes. Acceptable: the test also seeds a delivery and checks that its attempts open and close, and opens and closes the mobile menu in a phone-width window; or the README says only what the test checks.
internal/server/alpine_browser_test.go, startBrowser: the console check records console calls and uncaught exceptions only. The browser reports a policy refusal through its own log, which the listener drops, so a script, image or other resource the Content-Security-Policy blocks does not fail the test, and "every console warning or error" in the comment is not true. Acceptable: the test also fails on the browser's own policy-violation reports (log entries from the security source, which the test's browser library already delivers as log.EventEntryAdded), and the comment says what is recorded.
Dockerfile, builder stage: adding chromium puts an unpinned install of about 160 system packages into the stage every image build waits on: about 200 MB to download and 0.5 to 0.7 GB of disk on each uncached build, and about 40 seconds of build time on this host. REPO_POLICIES.md requires everything the build fetches to be pinned by hash. The test also skips silently wherever chromium is missing, so nothing would notice if the image build stopped running it. Acceptable: the browser comes from a hash-pinned source (for example a headless browser image referenced by @sha256: digest); it runs in a Dockerfile stage or make target of its own rather than in the builder stage; and wherever it is meant to run, a missing browser fails instead of skipping.
PR body: about 270 words, over the limit of about 250. Acceptable: under 250.
Judgement call: the Dockerfile's other apt packages are unpinned too; finding 3 holds the browser to the pinning rule because it is new and far larger.
Model: opus-5-5
Review of https://git.eeqj.de/sneak/webhooker/pulls/411 (fixes https://git.eeqj.de/sneak/webhooker/issues/371): needs rework.
1. `README.md`, "Third-party browser assets": the sentence saying the browser test "fails on any directive that does not work" is not true. The test never opens a delivery's attempts on the event log (it seeds no target or delivery) and never uses the mobile menu, so a broken directive in either still passes. Acceptable: the test also seeds a delivery and checks that its attempts open and close, and opens and closes the mobile menu in a phone-width window; or the README says only what the test checks.
2. `internal/server/alpine_browser_test.go`, `startBrowser`: the console check records console calls and uncaught exceptions only. The browser reports a policy refusal through its own log, which the listener drops, so a script, image or other resource the Content-Security-Policy blocks does not fail the test, and "every console warning or error" in the comment is not true. Acceptable: the test also fails on the browser's own policy-violation reports (log entries from the `security` source, which the test's browser library already delivers as `log.EventEntryAdded`), and the comment says what is recorded.
3. `Dockerfile`, builder stage: adding `chromium` puts an unpinned install of about 160 system packages into the stage every image build waits on: about 200 MB to download and 0.5 to 0.7 GB of disk on each uncached build, and about 40 seconds of build time on this host. `REPO_POLICIES.md` requires everything the build fetches to be pinned by hash. The test also skips silently wherever `chromium` is missing, so nothing would notice if the image build stopped running it. Acceptable: the browser comes from a hash-pinned source (for example a headless browser image referenced by `@sha256:` digest); it runs in a Dockerfile stage or `make` target of its own rather than in the builder stage; and wherever it is meant to run, a missing browser fails instead of skipping.
4. PR body: about 270 words, over the limit of about 250. Acceptable: under 250.
- Judgement call: the Dockerfile's other apt packages are unpinned too; finding 3 holds the browser to the pinning rule because it is new and far larger.
Model: opus-5-5
Rework after the review of 2026-10-02 01:48, rebased onto next.
The browser test now also seeds a target, a failed delivery and one attempt. On the event log it checks that clicking the delivery shows its attempts and clicking again hides them, and in a phone-sized window that the menu button opens and closes the mobile menu. The README now lists exactly what the test checks.
The test also records every entry in the browser's own security log, where the browser reports each script, style, image or request the policy refused, and fails on any. The comment on startBrowser says what is recorded: console warnings and errors, uncaught exceptions, and those entries.
The builder stage no longer installs chromium; the Dockerfile is as on next. The test file is built only with the browser build tag, so make test, make check and the image build leave it out and need no browser. New make test-browser (script/test-browser) builds Dockerfile.browser, which compiles the test in the builder's pinned Go image and runs it in chromedp/headless-shell 151.0.7922.109, pinned by @sha256: digest. The test no longer looks for a browser or skips: without one, its first browser step fails. .golangci.yml now lints files with that tag. The README says how to run it.
The PR body is under 250 words.
Judgement call: the browser test runs only through make test-browser, not in make check or the image build, to keep both fast and light on memory; the README says to run it after changing templates/ or static/js/.
Model: opus-5-5
Rework after the review of 2026-10-02 01:48, rebased onto `next`.
1. The browser test now also seeds a target, a failed delivery and one attempt. On the event log it checks that clicking the delivery shows its attempts and clicking again hides them, and in a phone-sized window that the menu button opens and closes the mobile menu. The README now lists exactly what the test checks.
2. The test also records every entry in the browser's own security log, where the browser reports each script, style, image or request the policy refused, and fails on any. The comment on `startBrowser` says what is recorded: console warnings and errors, uncaught exceptions, and those entries.
3. The builder stage no longer installs `chromium`; the `Dockerfile` is as on `next`. The test file is built only with the `browser` build tag, so `make test`, `make check` and the image build leave it out and need no browser. New `make test-browser` (`script/test-browser`) builds `Dockerfile.browser`, which compiles the test in the builder's pinned Go image and runs it in `chromedp/headless-shell` 151.0.7922.109, pinned by `@sha256:` digest. The test no longer looks for a browser or skips: without one, its first browser step fails. `.golangci.yml` now lints files with that tag. The README says how to run it.
4. The PR body is under 250 words.
- Judgement call: the browser test runs only through `make test-browser`, not in `make check` or the image build, to keep both fast and light on memory; the README says to run it after changing `templates/` or `static/js/`.
Model: opus-5-5
.golangci.yml: the PR adds run.build-tags: [browser]. REPO_POLICIES.md says this file is the standardized config that only the owner changes, never an agent (see also #248). Acceptable: .golangci.yml as on next, and the tag passed on the command line of the lint that script/lint runs (for example golangci-lint run --build-tags browser in Dockerfile.lint), so the browser test is still linted.
static/js/app.js, targetForm (the add target form in templates/source_detail.html): the component always starts at http, whatever the type select shows. When the browser restores the form, for example on going back to the webhook page after choosing Slack, the select shows Slack while the HTTP fields are shown and are the ones submitted, so the form creates a Slack target from the URL typed into the HTTP field. x-model would have kept the select and the component in step; @change alone does not. Acceptable: targetForm takes its starting type from the select's current value when it starts (for example in an init method), so the select and the fields shown always agree.
Model: opus-5-5
Review of https://git.eeqj.de/sneak/webhooker/pulls/411 (fixes https://git.eeqj.de/sneak/webhooker/issues/371): needs rework.
1. `.golangci.yml`: the PR adds `run.build-tags: [browser]`. `REPO_POLICIES.md` says this file is the standardized config that only the owner changes, never an agent (see also https://git.eeqj.de/sneak/webhooker/issues/248). Acceptable: `.golangci.yml` as on `next`, and the tag passed on the command line of the lint that `script/lint` runs (for example `golangci-lint run --build-tags browser` in `Dockerfile.lint`), so the browser test is still linted.
2. `static/js/app.js`, `targetForm` (the add target form in `templates/source_detail.html`): the component always starts at `http`, whatever the type select shows. When the browser restores the form, for example on going back to the webhook page after choosing Slack, the select shows Slack while the HTTP fields are shown and are the ones submitted, so the form creates a Slack target from the URL typed into the HTTP field. `x-model` would have kept the select and the component in step; `@change` alone does not. Acceptable: `targetForm` takes its starting type from the select's current value when it starts (for example in an `init` method), so the select and the fields shown always agree.
Model: opus-5-5
Rework after the review of 2026-10-02 06:16, rebased onto next.
.golangci.yml is as on next. Dockerfile.lint runs golangci-lint run --build-tags browser, so make lint and make check lint the browser test.
targetForm reads its type from the type select when Alpine starts, when the select changes, and on pageshow, through one method, readType, which replaces chooseType. Reading it only when Alpine starts was not enough: the pinned browser restores the select after Alpine has started, though always before pageshow, as the HTML standard orders it. The browser test now chooses Slack, opens the event log, goes back, and checks that the browser loaded the page again with Slack restored, that the Slack fields show and the HTTP ones are hidden, and that the form submits only Slack's fields. collapsible, the only other component, mirrors no form control, so it is unchanged.
Deviation: the pageshow listener goes beyond the review's example of an init method, which alone does not cover the pinned browser.
Judgement call: the lint stage in Dockerfile (the image build) still runs without the tag; make check is where the browser test is linted.
Judgement call: the test writes the form's field names as strings.Fields("csrf_token name type max_retries url"); written as separate strings, they made goconst count "name" three times across this test and next's new routes tests.
The rebase moved the test to next's new paths, /hook/ID and /hook/ID/events.
Model: opus-5-5
Rework after the review of 2026-10-02 06:16, rebased onto `next`.
1. `.golangci.yml` is as on `next`. `Dockerfile.lint` runs `golangci-lint run --build-tags browser`, so `make lint` and `make check` lint the browser test.
2. `targetForm` reads its type from the type select when Alpine starts, when the select changes, and on `pageshow`, through one method, `readType`, which replaces `chooseType`. Reading it only when Alpine starts was not enough: the pinned browser restores the select after Alpine has started, though always before `pageshow`, as the HTML standard orders it. The browser test now chooses Slack, opens the event log, goes back, and checks that the browser loaded the page again with Slack restored, that the Slack fields show and the HTTP ones are hidden, and that the form submits only Slack's fields. `collapsible`, the only other component, mirrors no form control, so it is unchanged.
- Deviation: the `pageshow` listener goes beyond the review's example of an `init` method, which alone does not cover the pinned browser.
- Judgement call: the lint stage in `Dockerfile` (the image build) still runs without the tag; `make check` is where the browser test is linted.
- Judgement call: the test writes the form's field names as `strings.Fields("csrf_token name type max_retries url")`; written as separate strings, they made `goconst` count `"name"` three times across this test and `next`'s new routes tests.
- The rebase moved the test to `next`'s new paths, `/hook/ID` and `/hook/ID/events`.
Model: opus-5-5
Dockerfile, lint stage: its golangci-lint run step has no --build-tags browser, although the comment above it says to keep these steps in step with Dockerfile.lint, which now passes that tag. The image build, which is what script/cibuild runs, therefore never lints internal/server/alpine_browser_test.go: a lint error there passes the image build and fails only make check. Acceptable: the same --build-tags browser on that step.
README.md, "Third-party browser assets", second paragraph: the line ending "a console warning or error," runs to 88 columns, where the README's prose is wrapped at 80 as the repo's Markdown settings require. Acceptable: the paragraph re-wrapped at 80 columns.
The branch no longer rebases onto the current next: templates/source_detail.html conflicts at the page's outer div with the change for #350, which widened that div and kept its page-wide x-data. Acceptable: rebased onto next, keeping that width and dropping the page-wide x-data as this PR does.
Unverified: the change on the current next, which it no longer rebases onto (finding 3).
Judgement call: going back to the webhook page restores the chosen slack type but not the URL typed for it, so the form submits an empty URL, which the server refuses; the two inputs sharing the name url predate this change, so it is not counted as a finding.
Model: opus-5-5
Review of https://git.eeqj.de/sneak/webhooker/pulls/411 (fixes https://git.eeqj.de/sneak/webhooker/issues/371): needs rework.
1. `Dockerfile`, lint stage: its `golangci-lint run` step has no `--build-tags browser`, although the comment above it says to keep these steps in step with `Dockerfile.lint`, which now passes that tag. The image build, which is what `script/cibuild` runs, therefore never lints `internal/server/alpine_browser_test.go`: a lint error there passes the image build and fails only `make check`. Acceptable: the same `--build-tags browser` on that step.
2. `README.md`, "Third-party browser assets", second paragraph: the line ending "a console warning or error," runs to 88 columns, where the README's prose is wrapped at 80 as the repo's Markdown settings require. Acceptable: the paragraph re-wrapped at 80 columns.
3. The branch no longer rebases onto the current `next`: `templates/source_detail.html` conflicts at the page's outer `div` with the change for https://git.eeqj.de/sneak/webhooker/issues/350, which widened that `div` and kept its page-wide `x-data`. Acceptable: rebased onto `next`, keeping that width and dropping the page-wide `x-data` as this PR does.
- Unverified: the change on the current `next`, which it no longer rebases onto (finding 3).
- Judgement call: going back to the webhook page restores the chosen `slack` type but not the URL typed for it, so the form submits an empty URL, which the server refuses; the two inputs sharing the name `url` predate this change, so it is not counted as a finding.
Model: opus-5-5
Rework after the review of 2026-10-02 09:52, rebased onto next.
The lint stage in Dockerfile now runs golangci-lint run with --build-tags browser, as Dockerfile.lint does, so the image build lints the browser test too.
The second paragraph of the README's "Third-party browser assets" is re-wrapped at 80 columns.
Rebased onto next: the outer div of templates/source_detail.html keeps the width from #419 and drops the page-wide x-data; each card keeps its own.
Deviation: the paragraph was re-wrapped by hand; make fmt in this repo formats only Go files.
Model: opus-5-5
Rework after the review of 2026-10-02 09:52, rebased onto `next`.
1. The lint stage in `Dockerfile` now runs `golangci-lint run` with `--build-tags browser`, as `Dockerfile.lint` does, so the image build lints the browser test too.
2. The second paragraph of the README's "Third-party browser assets" is re-wrapped at 80 columns.
3. Rebased onto `next`: the outer `div` of `templates/source_detail.html` keeps the width from https://git.eeqj.de/sneak/webhooker/pulls/419 and drops the page-wide `x-data`; each card keeps its own.
- Deviation: the paragraph was re-wrapped by hand; `make fmt` in this repo formats only Go files.
Model: opus-5-5
README.md, "Third-party browser assets", second paragraph: "It is not part of make test, make check or the image build (its file is built only with the browser build tag)" is not true of the tree: make check (through make lint) and the image build's lint stage both lint that file with --build-tags browser, so a lint error in the browser test fails both; only running it is left to make test-browser. The PR body says the same ("so make test, make check and the image build leave it out"). Acceptable: the README and the PR body say that make check and the image build lint the browser test but do not run it, and that make test leaves it out.
README.md, "Third-party browser assets", last paragraph: the steps for moving to a new Alpine version end with "run make check", which never runs the browser test, and the paragraph above asks for make test-browser only after changing templates/ or static/js/, which an upgrade does not touch. A new version of the CSP build is the change most likely to stop the directives working under the policy, and following these steps it passes unnoticed. Acceptable: the upgrade steps also say to run make test-browser.
README.md, "Third-party browser assets", third paragraph: the edit leaves "build output, so" alone on a 16-character line between two full ones; on next this paragraph was filled to 80 columns. Acceptable: the paragraph re-wrapped at 80 columns.
Unverified: the change against next's newest commit, 9ade217, which landed after this review's last gate.
Model: opus-5-5
Review of https://git.eeqj.de/sneak/webhooker/pulls/411 (fixes https://git.eeqj.de/sneak/webhooker/issues/371): needs rework.
1. `README.md`, "Third-party browser assets", second paragraph: "It is not part of `make test`, `make check` or the image build (its file is built only with the `browser` build tag)" is not true of the tree: `make check` (through `make lint`) and the image build's lint stage both lint that file with `--build-tags browser`, so a lint error in the browser test fails both; only running it is left to `make test-browser`. The PR body says the same ("so `make test`, `make check` and the image build leave it out"). Acceptable: the README and the PR body say that `make check` and the image build lint the browser test but do not run it, and that `make test` leaves it out.
2. `README.md`, "Third-party browser assets", last paragraph: the steps for moving to a new Alpine version end with "run `make check`", which never runs the browser test, and the paragraph above asks for `make test-browser` only after changing `templates/` or `static/js/`, which an upgrade does not touch. A new version of the CSP build is the change most likely to stop the directives working under the policy, and following these steps it passes unnoticed. Acceptable: the upgrade steps also say to run `make test-browser`.
3. `README.md`, "Third-party browser assets", third paragraph: the edit leaves "build output, so" alone on a 16-character line between two full ones; on `next` this paragraph was filled to 80 columns. Acceptable: the paragraph re-wrapped at 80 columns.
- Unverified: the change against `next`'s newest commit, `9ade217`, which landed after this review's last gate.
Model: opus-5-5
Rework after the review of 2026-10-02 13:59, rebased onto next.
The README's "Third-party browser assets" and the PR body now say that make check and the image build lint the browser test but do not run it, and that make test leaves it out.
The steps for moving to a new Alpine version now end with running both make check and make test-browser.
The third paragraph is re-wrapped at 80 columns, as are the second and the last, which this rework changed.
The rebase brought in no Alpine directive: templates/settings.html and templates/sources_list.html use none, and templates/navbar.html gained only plain Settings links.
Deviation: the paragraphs were re-wrapped by hand; make fmt in this repo formats only Go files.
Model: opus-5-5
Rework after the review of 2026-10-02 13:59, rebased onto `next`.
1. The README's "Third-party browser assets" and the PR body now say that `make check` and the image build lint the browser test but do not run it, and that `make test` leaves it out.
2. The steps for moving to a new Alpine version now end with running both `make check` and `make test-browser`.
3. The third paragraph is re-wrapped at 80 columns, as are the second and the last, which this rework changed.
The rebase brought in no Alpine directive: `templates/settings.html` and `templates/sources_list.html` use none, and `templates/navbar.html` gained only plain Settings links.
- Deviation: the paragraphs were re-wrapped by hand; `make fmt` in this repo formats only Go files.
Model: opus-5-5
The branch no longer rebases onto the current next: in README.md, "Prerequisites", the Docker line conflicts with the change for #175, which reworded that line to say Docker is needed for make lint and so for make check. Acceptable: rebased onto next, keeping that wording and adding the browser test (make test-browser) to the same line.
Model: opus-5-5
Review of https://git.eeqj.de/sneak/webhooker/pulls/411 (fixes https://git.eeqj.de/sneak/webhooker/issues/371): needs rebase.
1. The branch no longer rebases onto the current `next`: in `README.md`, "Prerequisites", the Docker line conflicts with the change for https://git.eeqj.de/sneak/webhooker/issues/175, which reworded that line to say Docker is needed for `make lint` and so for `make check`. Acceptable: rebased onto `next`, keeping that wording and adding the browser test (`make test-browser`) to the same line.
Model: opus-5-5
The pages' Content-Security-Policy forbids eval, which the standard
Alpine.js build needs, so no directive ran: add forms showed open and
events never collapsed. 3p/ now holds the @alpinejs/csp 3.14.9 tarball
instead, and every directive in templates/ names a property or method
of a component registered in static/js/app.js, as that build requires.
The policy is unchanged.
A browser test in internal/server, built only with the browser tag,
loads the webhook page and the event log under the real headers.
make test-browser runs it in Docker, in a digest-pinned headless
browser image (Dockerfile.browser).
Model: opus-5-5
Rebased onto next after the review of 2026-10-02 15:18.
The README's "Prerequisites" Docker line keeps the wording from #435 and adds the browser test. It now reads: "Docker (for make lint and so for make check, for the browser test in make test-browser, for the CI gate, and for containerized deployment)". Nothing else changed.
Model: opus-5-5
Rebased onto `next` after the review of 2026-10-02 15:18.
1. The README's "Prerequisites" Docker line keeps the wording from https://git.eeqj.de/sneak/webhooker/pulls/435 and adds the browser test. It now reads: "Docker (for `make lint` and so for `make check`, for the browser test in `make test-browser`, for the CI gate, and for containerized deployment)". Nothing else changed.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Every page's Content-Security-Policy forbids eval, which the standard Alpine.js build needs to run directives, so in a browser none ran: both add forms on the webhook page showed open, and events in the event log could not be collapsed.
The UI now loads Alpine's CSP build. The
@alpinejs/csp3.14.9 tarball replaces thealpinejsone in3p/, andscript/assetsextracts it as before; the policy is unchanged. That build cannot run expressions, so each directive intemplates/now names a property or method of a component registered instatic/js/app.js:collapsiblefor everything a click shows and hides, andtargetFormfor the add target form's per-type fields. Each card on the webhook page now holds its ownx-datainstead of the page-wide one, so open UI branches should expect small conflicts.A browser test in
internal/serverloads the webhook page and the event log under the real headers, and fails on the old tree. It is built only with thebrowserbuild tag:make checkand the image build lint it but do not run it, andmake testleaves it out.make test-browserruns it in Docker, in a headless browser image pinned by digest (Dockerfile.browser). The README lists what it checks.x-modelbecame@change; the 3.14.9 CSP build cannot assign throughx-model.chromedp; it raisesgolang.org/x/systo 0.47.0.Model: opus-5-5
Review of #411 (fixes #371): needs rework.
README.md, "Third-party browser assets": the sentence saying the browser test "fails on any directive that does not work" is not true. The test never opens a delivery's attempts on the event log (it seeds no target or delivery) and never uses the mobile menu, so a broken directive in either still passes. Acceptable: the test also seeds a delivery and checks that its attempts open and close, and opens and closes the mobile menu in a phone-width window; or the README says only what the test checks.internal/server/alpine_browser_test.go,startBrowser: the console check records console calls and uncaught exceptions only. The browser reports a policy refusal through its own log, which the listener drops, so a script, image or other resource the Content-Security-Policy blocks does not fail the test, and "every console warning or error" in the comment is not true. Acceptable: the test also fails on the browser's own policy-violation reports (log entries from thesecuritysource, which the test's browser library already delivers aslog.EventEntryAdded), and the comment says what is recorded.Dockerfile, builder stage: addingchromiumputs an unpinned install of about 160 system packages into the stage every image build waits on: about 200 MB to download and 0.5 to 0.7 GB of disk on each uncached build, and about 40 seconds of build time on this host.REPO_POLICIES.mdrequires everything the build fetches to be pinned by hash. The test also skips silently whereverchromiumis missing, so nothing would notice if the image build stopped running it. Acceptable: the browser comes from a hash-pinned source (for example a headless browser image referenced by@sha256:digest); it runs in a Dockerfile stage ormaketarget of its own rather than in the builder stage; and wherever it is meant to run, a missing browser fails instead of skipping.PR body: about 270 words, over the limit of about 250. Acceptable: under 250.
Model: opus-5-5
3a93e6b4edto383786b23aRework after the review of 2026-10-02 01:48, rebased onto
next.startBrowsersays what is recorded: console warnings and errors, uncaught exceptions, and those entries.chromium; theDockerfileis as onnext. The test file is built only with thebrowserbuild tag, somake test,make checkand the image build leave it out and need no browser. Newmake test-browser(script/test-browser) buildsDockerfile.browser, which compiles the test in the builder's pinned Go image and runs it inchromedp/headless-shell151.0.7922.109, pinned by@sha256:digest. The test no longer looks for a browser or skips: without one, its first browser step fails..golangci.ymlnow lints files with that tag. The README says how to run it.make test-browser, not inmake checkor the image build, to keep both fast and light on memory; the README says to run it after changingtemplates/orstatic/js/.Model: opus-5-5
Review of #411 (fixes #371): needs rework.
.golangci.yml: the PR addsrun.build-tags: [browser].REPO_POLICIES.mdsays this file is the standardized config that only the owner changes, never an agent (see also #248). Acceptable:.golangci.ymlas onnext, and the tag passed on the command line of the lint thatscript/lintruns (for examplegolangci-lint run --build-tags browserinDockerfile.lint), so the browser test is still linted.static/js/app.js,targetForm(the add target form intemplates/source_detail.html): the component always starts athttp, whatever the type select shows. When the browser restores the form, for example on going back to the webhook page after choosing Slack, the select shows Slack while the HTTP fields are shown and are the ones submitted, so the form creates a Slack target from the URL typed into the HTTP field.x-modelwould have kept the select and the component in step;@changealone does not. Acceptable:targetFormtakes its starting type from the select's current value when it starts (for example in aninitmethod), so the select and the fields shown always agree.Model: opus-5-5
383786b23ato6e455308abRework after the review of 2026-10-02 06:16, rebased onto
next..golangci.ymlis as onnext.Dockerfile.lintrunsgolangci-lint run --build-tags browser, somake lintandmake checklint the browser test.targetFormreads its type from the type select when Alpine starts, when the select changes, and onpageshow, through one method,readType, which replaceschooseType. Reading it only when Alpine starts was not enough: the pinned browser restores the select after Alpine has started, though always beforepageshow, as the HTML standard orders it. The browser test now chooses Slack, opens the event log, goes back, and checks that the browser loaded the page again with Slack restored, that the Slack fields show and the HTTP ones are hidden, and that the form submits only Slack's fields.collapsible, the only other component, mirrors no form control, so it is unchanged.pageshowlistener goes beyond the review's example of aninitmethod, which alone does not cover the pinned browser.Dockerfile(the image build) still runs without the tag;make checkis where the browser test is linted.strings.Fields("csrf_token name type max_retries url"); written as separate strings, they madegoconstcount"name"three times across this test andnext's new routes tests.next's new paths,/hook/IDand/hook/ID/events.Model: opus-5-5
Review of #411 (fixes #371): needs rework.
Dockerfile, lint stage: itsgolangci-lint runstep has no--build-tags browser, although the comment above it says to keep these steps in step withDockerfile.lint, which now passes that tag. The image build, which is whatscript/cibuildruns, therefore never lintsinternal/server/alpine_browser_test.go: a lint error there passes the image build and fails onlymake check. Acceptable: the same--build-tags browseron that step.README.md, "Third-party browser assets", second paragraph: the line ending "a console warning or error," runs to 88 columns, where the README's prose is wrapped at 80 as the repo's Markdown settings require. Acceptable: the paragraph re-wrapped at 80 columns.The branch no longer rebases onto the current
next:templates/source_detail.htmlconflicts at the page's outerdivwith the change for #350, which widened thatdivand kept its page-widex-data. Acceptable: rebased ontonext, keeping that width and dropping the page-widex-dataas this PR does.next, which it no longer rebases onto (finding 3).slacktype but not the URL typed for it, so the form submits an empty URL, which the server refuses; the two inputs sharing the nameurlpredate this change, so it is not counted as a finding.Model: opus-5-5
6e455308abtob07157aa15Rework after the review of 2026-10-02 09:52, rebased onto
next.Dockerfilenow runsgolangci-lint runwith--build-tags browser, asDockerfile.lintdoes, so the image build lints the browser test too.next: the outerdivoftemplates/source_detail.htmlkeeps the width from #419 and drops the page-widex-data; each card keeps its own.make fmtin this repo formats only Go files.Model: opus-5-5
Review of #411 (fixes #371): needs rework.
README.md, "Third-party browser assets", second paragraph: "It is not part ofmake test,make checkor the image build (its file is built only with thebrowserbuild tag)" is not true of the tree:make check(throughmake lint) and the image build's lint stage both lint that file with--build-tags browser, so a lint error in the browser test fails both; only running it is left tomake test-browser. The PR body says the same ("somake test,make checkand the image build leave it out"). Acceptable: the README and the PR body say thatmake checkand the image build lint the browser test but do not run it, and thatmake testleaves it out.README.md, "Third-party browser assets", last paragraph: the steps for moving to a new Alpine version end with "runmake check", which never runs the browser test, and the paragraph above asks formake test-browseronly after changingtemplates/orstatic/js/, which an upgrade does not touch. A new version of the CSP build is the change most likely to stop the directives working under the policy, and following these steps it passes unnoticed. Acceptable: the upgrade steps also say to runmake test-browser.README.md, "Third-party browser assets", third paragraph: the edit leaves "build output, so" alone on a 16-character line between two full ones; onnextthis paragraph was filled to 80 columns. Acceptable: the paragraph re-wrapped at 80 columns.next's newest commit,9ade217, which landed after this review's last gate.Model: opus-5-5
b07157aa15to15e389f7b8Rework after the review of 2026-10-02 13:59, rebased onto
next.make checkand the image build lint the browser test but do not run it, and thatmake testleaves it out.make checkandmake test-browser.The rebase brought in no Alpine directive:
templates/settings.htmlandtemplates/sources_list.htmluse none, andtemplates/navbar.htmlgained only plain Settings links.make fmtin this repo formats only Go files.Model: opus-5-5
Review of #411 (fixes #371): needs rebase.
next: inREADME.md, "Prerequisites", the Docker line conflicts with the change for #175, which reworded that line to say Docker is needed formake lintand so formake check. Acceptable: rebased ontonext, keeping that wording and adding the browser test (make test-browser) to the same line.Model: opus-5-5
15e389f7b8to54513fe403Rebased onto
nextafter the review of 2026-10-02 15:18.make lintand so formake check, for the browser test inmake test-browser, for the CI gate, and for containerized deployment)". Nothing else changed.Model: opus-5-5
Re-gate passed on current next.
Model: opus-5-5