Repository navigation
Conversation
The login callback built its redirects from request.url. The image's standalone server sees its listening address as the origin of requests (http://0.0.0.0:3000), so after a login it sent the browser to http://0.0.0.0:3000/dashboard, where the session cookies (set for the real origin) don't apply. The redirects now go to APP_ORIGIN, and to the origin of the request only when it isn't set. Found by running the end-to-end tests against the image. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An APP_ORIGIN without a scheme made the callback throw (a 500, or a misleading log line in the token error path). It now logs the problem and redirects to the origin of the request, as without the setting. The tests cover every redirect of the callback with a request at the listening address. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With E2E_IMAGE set, Playwright starts that Docker image as the app instead of building the checkout: as the image's non-root user, with a read-only root file system, a writable /tmp and all settings at start, on the host network, with the mock services' certificate mounted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CI's new e2e-image job runs the Chromium journey against the image of the Build image job (pull requests) or the pushed digest (pushes), and both publish and CI passed need it. The release workflow's new e2e job does the same with the release digest before release-image attests, signs and tags it. The development docs say how to run the tests against a local image; the release and deployment docs list the new checks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The certificate directory is resolved from the config's directory, where the mock services write it, and like the image it may hold only plain characters, as both are part of a shell command. Comments say why Playwright runs on the runner for the image, and the docs how to remove a container that a killed run left behind and that Release image waits for the end-to-end tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Reviewer's GuideAPIMake every authentication callback redirect use
ConfigurationConfigure Playwright to run the app from a validated Docker image under deployment-like constraints, and add CI and release gates that test the relevant image before publishing or releasing it.
DocumentationDocument local image-based end-to-end runs and update the deployment and release descriptions to include the new image checks.
TestsCover callback redirects to the configured origin and the request-origin fallback across success and error cases.
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #82 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 12 12
Lines 273 279 +6
Branches 77 78 +1
=========================================
+ Hits 273 279 +6 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
ubuntu-latest is Ubuntu 26.04 now, where Playwright 1.56 can't install
Chromium ("Playwright does not support chromium on ubuntu26.04-x64").
The image tests run Playwright on the runner, so they pin Ubuntu 24.04
until @playwright/test supports 26.04.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Linked issuesChecked at commit f5b39bc #79 test: run the end-to-end tests on the image · 3 of 4 met
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/app/auth/api/callback/route.ts" line_range="26" />
<code_context>
+ );
+ }
+ }
+ return new URL(path, request.url);
+}
+
</code_context>
<issue_to_address>
**Logout fails after fallback login**
When `APP_ORIGIN` is set to a non-URL value and the callback otherwise succeeds, `appUrl` lets the callback complete with a malformed `APP_ORIGIN`, but the logout route still returns 500 for that setting without clearing session cookies. The user can log in but cannot log out through the sidebar.
Make logout handle the invalid setting consistently with the callback, or reject the invalid configuration rather than allowing login to proceed.
</issue_to_address>
### Comment 2
<location path="docs/development/index.md" line_range="106-119" />
<code_context>
tests in the Playwright image, as CI does (see the `e2e` job in
`.github/workflows/ci.yml`).
+ To run them against a Docker image instead of the build of the
+ checkout, set `E2E_IMAGE` to the image. Playwright then starts it as it
+ is deployed: as the image's non-root user, with a read-only root file
+ system, a writable `/tmp` and all settings at start. A difference
+ between the two, such as a file missing from the standalone output or a
+ setting read at build time, fails the tests then. This needs Docker with
+ the host network (Linux) and a free container name `krini-e2e`. CI does
+ this in Chromium for every image it builds (`e2e-image` job), and the
+ release workflow for every release image (`e2e` job):
+
+ ```bash
+ docker build -t krini:local .
+ E2E_IMAGE=krini:local bun run test:e2e --project chromium
+ ```
+
+ The container is removed when the tests end. If a run was killed and
</code_context>
<issue_to_address>
**README test instructions**
The issue asks for local-image instructions in the README's Tests section. The change adds those instructions to `docs/development/index.md` instead.
</issue_to_address>Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: src/app/auth/api/callback/route.ts:26
This branch had an error being deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #79
Summary
playwright.config.ts: withE2E_IMAGEset, Playwright starts that Docker image as the app instead of building the checkout. The image runs as it is deployed: as its non-root user, with--read-only --tmpfs /tmp, and with all settings at start (--env NAME, so values never appear on the command line). It uses the host network and gets the mock services' certificate mounted. The image reference and the certificate path are validated, because both go into a shell command.e2e-image("End-to-end tests (image)") runs the Chromium journey against the image ofBuild imageon pull requests, and against the pushed digest on pushes.publishandCI passedneed it, so the ruleset stays unchanged.e2e("End-to-end tests (release image)") runs the same tests against the exact release digest.Release image(attest, sign, tag) now needs it.docs/development/index.mdexplains how to run the tests against a local image, and how to remove a container a killed run left behind.releasing.mdanddeployment.mdlist the new checks.Bug found by the new tests (separate issue and PR to follow)
Against the image, 5 of 11 Chromium tests failed at the redirect after a login. The login callback built its redirects from
request.url, which in the image's standalone server is its listening address. So after a login, the browser went tohttp://0.0.0.0:3000/dashboard, without the session.APP_ORIGIN, and fall back to the request's origin, with a log line, ifAPP_ORIGINis unset or isn't a URL. Unit tests cover all 5 redirects.fix/callback-origin, for their own issue and PR, which the maintainer opens. This PR will be updated fromdevonce that PR is merged, so that only the test: run the end-to-end tests on the image #79 changes remain.Verification
E2E_IMAGE=krini:e2e-local npx playwright test --project chromium→ 5 failed, 6 passed.CI=1 E2E_IMAGE=krini:e2e-local npx playwright test→ 22 passed, in Chromium and Firefox. Afterwards nokrini-e2econtainer was left.CI=1 npx playwright test --project chromium→ 11 passed.krini-e2emakes the run fail within 1 s ("Conflict", exit code 125), not after a timeout.npm run test:coverage→ 124 passed, 100% statements, branches, functions and lines.bun run lint→ clean.bunx tsc --noEmit→ clean.actionlint(with shellcheck) → clean.mkdocs build --strict→ clean.adversarial-reviewerandquality-reviewer. No high findings. All medium and low findings are fixed, except the one declined below.End-to-end tests (image)(57 s) andCI passed.ubuntu-latestis now Ubuntu 26.04, which Playwright 1.56 doesn't support.ubuntu-24.04, with a comment saying when to drop the pin.e2ejob isn't affected, because it runs in Playwright's image.e2ejob only runs on a version tag. It uses the same steps ase2e-image, apart from pulling the image by digest, which follows the releasesmokejob.Declined review findings
docker pull, and the Playwright setup, are repeated across the CI and release jobs. This follows the existing pattern of theimage,scanandsmokejobs. Moving them into a shared script would change those existing jobs as well, which is beyond test: run the end-to-end tests on the image #79.Open questions
None.
🤖 Generated with Claude Code
Summary by Sourcery
Validate deployed Docker images with end-to-end tests throughout CI and release workflows, while making authentication redirects reliable in standalone deployments.
New Features:
Bug Fixes:
Enhancements:
CI:
Documentation:
Tests: