Skip to content

feat: adding basic end-to-end testing and ci workflow - #225

Merged
KillianLarcher merged 23 commits into
mainfrom
feat/e2e
Mar 18, 2026
Merged

KillianLarcher merged 23 commits into
mainfrom
feat/e2e

Conversation

@KillianLarcher

@KillianLarcher KillianLarcher commented Mar 18, 2026 •

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • New Features

    • Added end-to-end cross‑browser Playwright tests and configuration; Playwright added as a dev dependency.
  • Improvements

    • Restored and expanded authentication: email verification, password reset, social SSO, passkeys, account linking, session and organization handling, role mapping and related notifications.
  • Bug Fixes

    • Adjusted agent restoration API response format for consistency.
  • Chores

    • CI/workflow updates for Docker, e2e and release flows; updated ignore rules and removed placeholder/example env entries.

@coderabbitai

coderabbitai Bot commented Mar 18, 2026 •

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Removes a sample env var and placeholder files, adds Playwright config/tests and CI workflow, requires and wires a new ref input through docker/release workflows, tweaks one API response shape, and substantially rewrites the authentication initialization and helpers.

Changes

Cohort / File(s) Summary
E2E Tests & Playwright
e2e/auth.spec.ts, playwright.config.ts, package.json
Adds Playwright config, a comprehensive auth-focused end-to-end test suite, and @playwright/test devDependency.
E2E CI Workflow
.github/workflows/e2e.yml
Adds a new GitHub Actions workflow that builds the image, starts Postgres, runs the app container, waits for readiness, runs Playwright tests, uploads reports on failure, and always cleans up.
Reusable Docker Workflow
.github/workflows/docker.yml
Adds a required ref input to workflow_call and forwards it to the checkout step (with.ref, with.fetch-depth: 0).
Release Workflow
.github/workflows/release.yml
Wires the release ref into the docker reusable workflow call.
Auth subsystem
src/lib/auth/auth.ts
Large, comprehensive rework restoring consolidated auth initialization: DB adapter (drizzle/pg), email/password flows (reset, verify), social/OIDC mapping, passkeys, 2FA, org/admin plugins, account linking, session hooks, role mapping, and many helper APIs.
API Response Shape
app/api/agent/[agentId]/restore/route.ts
Alters POST response JSON shape from { message: true, details: "..." } to { status: true, message: "..." }.
Env & Gitignore
.env.example, .gitignore
Removes PROJECT_DESCRIPTION from .env.example; adds Playwright-related ignore patterns to .gitignore.
Repo placeholders removed
seeds/keycloak/.gitkeep, seeds/pocket-id/.gitkeep
Removes empty .gitkeep files used as directory placeholders.

Sequence Diagram(s)

sequenceDiagram
  participant GH as "GitHub Actions Runner"
  participant Docker as "Docker (build & run)"
  participant DB as "Postgres 17 service"
  participant Playwright as "Playwright test runner"

  GH->>DB: start Postgres service (user/password/db + healthcheck)
  GH->>Docker: build image (docker/dockerfile/Dockerfile)
  GH->>Docker: run app container (host network, expose :8887, set envs)
  Playwright->>Docker: poll BASE_URL http://localhost:8887 until ready
  Playwright->>Docker: execute browser tests against app
  Docker->>DB: app connects to database
  Playwright-->>GH: upload report artifact on failure
  GH->>Docker: stop & remove app container (always)
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

  • fix: release.yml #214: Adds/wires ref input between docker.yml and release.yml and renames the API response field — directly overlaps these workflow and route changes.
  • dev #193: Similar edits to the reusable .github/workflows/docker.yml inputs and propagation.
  • dev #194: Related adjustments in .github/workflows/release.yml to pass workflow inputs into the docker workflow.

Poem

🐰 I hopped through CI with lantern bright,
spun up Postgres, watched containers light.
Auth dons new keys and roles take flight,
browsers raced tests into the night.
A crunchy carrot for the green-light sight!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding end-to-end testing infrastructure and CI workflow, which aligns with the substantial additions of Playwright tests, e2e workflows, and related configurations.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/e2e
📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 14

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/docker.yml:
- Around line 9-11: The new required input "ref" added to the docker.yml
workflow is not being provided by release-candidate.yml, causing runs to fail;
either make the "ref" input optional with a sensible default in docker.yml or
update release-candidate.yml to pass the same ref value as release.yml does.
Locate the "inputs: ref" declaration in docker.yml and either remove required:
true and add a default (e.g., default: ${{ github.ref }} or similar), or edit
release-candidate.yml to add the docker.yml workflow_call input "ref" with the
appropriate expression so it matches release.yml's call.

In @.github/workflows/e2e.yml:
- Around line 69-72: Update the GitHub Actions step named "Run Playwright tests"
so it uses pnpm instead of npx: replace the current run command "npx playwright
test" with the pnpm equivalent (for example "pnpm exec playwright test") while
keeping the existing env variable BASE_URL set to http://localhost:8887; ensure
the step name "Run Playwright tests" and its environment block remain unchanged.
- Around line 38-45: The workflow's Run app container step launches the Docker
container named myapp which uses DATABASE_URL pointing at host "postgres" that
it cannot resolve; update that step to run myapp on the same network as the
Postgres service (either by using host network mode with --network host and
change DATABASE_URL host to localhost, or create a user-defined Docker network
and run both the Postgres service and docker run for myapp with --network
<network_name>) so the container can reach the database; adjust the DATABASE_URL
env var accordingly and ensure the step "Run app container" uses the chosen
network option.
- Around line 66-67: The workflow uses npm commands but the repo uses pnpm;
update the e2e GitHub Actions steps to use pnpm equivalents: replace the "npm
ci" step with "pnpm install --frozen-lockfile" (to honor the lockfile) and
replace "npx playwright install --with-deps" with the pnpm-compatible invocation
such as "pnpm dlx playwright install --with-deps" (or "pnpm exec playwright
install --with-deps") so dependency installation and Playwright setup run under
pnpm; update the steps referencing "npm ci", "npx playwright install
--with-deps" in the .github/workflows/e2e.yml file accordingly.

In @.gitignore:
- Around line 51-57: Remove the redundant node_modules/ entry from the
.gitignore: keep the existing /node_modules root pattern and delete the
duplicate "node_modules/" line added in the Playwright block so you don't repeat
exclusion rules; ensure any intentional nested node_modules exclusions are
handled only if needed, otherwise remove the extra "node_modules/" entry.

In `@e2e/auth.spec.ts`:
- Around line 39-42: The test "Redirect to login if not connected" in
e2e/auth.spec.ts uses a relative URL in page.goto('dashboard/projects'); change
it to an absolute path page.goto('/dashboard/projects') (and update any other
page.goto('...') calls in this file to start with '/') so navigation no longer
relies on baseURL-relative resolution and clearly indicates root-relative
routes.
- Around line 11-14: The tests define a users map (users: Record<string,
UserCredentials>) where the "normal" entry has a typo in its email
("john5@example.comm"); update the "normal" user's email to a valid domain
(e.g., "john5@example.com") so email validation in tests won't fail—locate the
users object and correct the normal entry's email string.
- Around line 133-136: The test fails because logout() is a no-op and the test
never ensures a logged-in state; implement logout() to perform the actual
sign-out flow (e.g., open the user menu and click the "Logout" button and wait
for navigation) and update the "Successful logout" test to ensure an
authenticated state before calling logout() (either call the existing login()
helper or set the auth cookie/session), then call logout() and assert the page
navigates to 'login'. Locate and modify the logout() function and the test named
'Successful logout' to add these steps.

In `@playwright.config.ts`:
- Around line 27-33: The Playwright config currently hardcodes use.baseURL to
'http://localhost:8887'; change it to read from the environment with a fallback
(e.g., process.env.BASE_URL || 'http://localhost:8887') so CI/workflows can
override the URL; update the use block inside the exported config (the use
object in playwright.config.ts) to set baseURL from process.env.BASE_URL and
ensure the value is a string-compatible fallback.

In `@src/lib/auth/auth.ts`:
- Around line 151-152: Remove the use of //@ts-ignore and resolve the duplicated
issuer field by either deleting the redundant top-level issuer assignment or by
updating the type so the root object legitimately includes issuer; specifically
inspect the object where oidcConfig is defined and the root-level issuer
property (symbol: issuer and oidcConfig in auth.ts), then keep issuer only in
one place or extend the interface/type to include issuer so TypeScript no longer
raises an error and //@ts-ignore is unnecessary.
- Around line 574-585: The empty catch blocks in revokeSession, unlinkAccount,
checkSlugOrganization, getActiveMember, and setActiveOrganization silently
swallow errors; update each catch to either log the caught error (e.g.,
console.error or the module logger) with contextual text including the function
name and the error, and then re-throw or return a failure indicator as
appropriate for the caller; specifically, modify revokeSession (which calls
auth.api.revokeSession and headers()) and the other listed functions to capture
the caught exception (e) and call logger.error/console.error("revokeSession
failed:", e) (or similar for the other function names) and then throw e or
return a determinable error value so failures are not lost.
- Around line 472-474: The call to internalAdapter.updateUser on (await
auth.$context) is not awaited, so lastConnectedAt may not be persisted before
the surrounding function returns; update the code in the block that uses (await
auth.$context).internalAdapter.updateUser(user.id, { lastConnectedAt: new Date()
}) to await the Promise (i.e., await (await
auth.$context).internalAdapter.updateUser(...)) and optionally handle errors
(try/catch or propagate) so failures are not silently ignored.
- Around line 664-678: The getLastOrganizationOrFirst function is querying
db.query.organization but uses the member.userId condition
(drizzleDb.schemas.member.userId), which is wrong; change the logic to query the
member table for rows where drizzleDb.schemas.member.userId equals the provided
userId, then extract the member.organizationId values (or fetch related
organization ids), and return the first organization id found (or null) — update
references to db.query.organization to db.query.member (or perform a join from
member to organization) and use drizzleDb.schemas.member.organizationId to
obtain the organization id.
- Around line 371-393: The role-selection based on userCount in the after(user,
context) hook is racy because the new user already exists when you count; fix by
moving the user-count check into the user creation transaction (or into the
before hook) so the decision is atomic: run the count via db.select({count:
count()}).from(drizzleDb.schemas.user) before inserting the new user (or wrap
count + user insert + member insert in a single DB transaction), compute role =
count === 0 ? "owner" : "admin", then insert into drizzleDb.schemas.member with
that role (replace the current logic in after and/or implement in before or the
transaction surrounding user creation).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: c2e5275d-3153-4196-b935-f6406348b524

📥 Commits

Reviewing files that changed from the base of the PR and between 260b3ba and e575ea1.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (12)
  • .env.example
  • .github/workflows/docker.yml
  • .github/workflows/e2e.yml
  • .github/workflows/release.yml
  • .gitignore
  • app/api/agent/[agentId]/restore/route.ts
  • e2e/auth.spec.ts
  • package.json
  • playwright.config.ts
  • seeds/keycloak/.gitkeep
  • seeds/pocket-id/.gitkeep
  • src/lib/auth/auth.ts
💤 Files with no reviewable changes (3)
  • seeds/pocket-id/.gitkeep
  • .env.example
  • seeds/keycloak/.gitkeep
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build-and-test
🧰 Additional context used
🧬 Code graph analysis (1)
src/lib/auth/auth.ts (7)
src/lib/auth/oidc.ts (1)
  • getOidcProviders (23-93)
src/db/index.ts (1)
  • db (52-57)
src/env.mjs (2)
  • env (8-141)
  • env (8-141)
src/db/utils/index.ts (1)
  • withUpdatedAt (1-3)
src/lib/auth/oauth.ts (1)
  • getOAuthProviders (56-155)
src/lib/auth/config.ts (1)
  • SUPPORTED_PROVIDERS (24-79)
src/utils/detection.ts (1)
  • getDeviceDetails (11-37)
🪛 Checkov (3.2.508)
.github/workflows/e2e.yml

[medium] 42-43: Basic Auth Credentials

(CKV_SECRET_4)

🔇 Additional comments (5)
package.json (2)

106-106: LGTM - Playwright version verified.

The @playwright/test version ^1.58.2 is the latest version, last published about a month ago. This is an appropriate version for the new E2E testing infrastructure.


106-106: No action needed. The version ^1.58.2 is the latest stable release of @playwright/test and is correctly specified.

.github/workflows/release.yml (1)

88-92: LGTM!

The addition of the ref input ensures the Docker build uses the correct version tag, properly coordinating with the docker workflow's ref handling.

app/api/agent/[agentId]/restore/route.ts (1)

68-71: This appears to be an internal agent-to-backend endpoint. No evidence was found in the TypeScript codebase of:

  1. Consumers of this response structure expecting the old format
  2. Recent changes to this response structure (git history shows only unrelated CSP fix)
  3. Any migration or compatibility layer for clients

The endpoint is called by an external agent service (not in this repository), so the impact of response format changes cannot be verified from the codebase alone.

			> Likely an incorrect or invalid review comment.
.github/workflows/docker.yml (1)

43-47: LGTM!

The checkout configuration correctly uses the ref input to ensure the specific git reference is checked out, and fetch-depth: 0 ensures full history is available for any git operations (e.g., tag resolution, versioning tools). This is appropriate for a release/build workflow.

Comment thread .github/workflows/docker.yml
Comment thread .github/workflows/e2e.yml
Comment thread .github/workflows/e2e.yml Outdated
Comment thread .github/workflows/e2e.yml Outdated
Comment thread .gitignore
Comment thread src/lib/auth/auth.ts
Comment thread src/lib/auth/auth.ts
Comment thread src/lib/auth/auth.ts
Comment thread src/lib/auth/auth.ts
Comment thread src/lib/auth/auth.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
.github/workflows/e2e.yml (1)

52-59: ⚠️ Potential issue | 🔴 Critical

App container still cannot reliably reach Postgres with current host/network setup.

DATABASE_URL uses @postgres, but this container is started via docker run and is not guaranteed to resolve the GitHub Actions service alias unless explicitly networked together. This can break app boot and make the e2e job fail/flaky.

🐛 Suggested fix (host networking)
       - name: Run app container
         run: |
           docker run -d --name myapp \
-            -p 8887:8887 \
-            -e DATABASE_URL=postgres://devuser:changeme@postgres:5432/devdb \
+            --network host \
+            -e DATABASE_URL=postgres://devuser:changeme@localhost:5432/devdb \
             -e NODE_ENV=production \
             -e PROJECT_URL=http://localhost:8887 \
             portabase/portabase:test
#!/bin/bash
# Read-only verification: confirms current workflow still uses service hostname without explicit docker network mode.
rg -n -C2 'Run app container|docker run|-e DATABASE_URL|--network' .github/workflows/e2e.yml
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/e2e.yml around lines 52 - 59, The container run currently
uses docker run without joining the Actions service network so the DATABASE_URL
host "postgres" won't resolve; update the docker run invocation to join the
runner network (e.g., add --network host or an explicit network that can see the
service) and adjust ports/env if needed so the app can reach Postgres; modify
the docker run command that sets DATABASE_URL and add the --network host (or
create/join a user-defined network) flag so the service hostname "postgres"
resolves for the app container.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/e2e.yml:
- Line 13: The CI Postgres service image is currently using a mutable tag
("postgres:17"); replace that with the provided immutable digest to ensure
reproducible builds by changing the service image value from "postgres:17" to
"postgres:17@sha256:dedfd97c186f1154564d4dde3bbf26232be65611cf23823c7c2cf2a03b9d9f3d"
in the e2e workflow configuration (update the image entry under the Postgres
service declaration).

---

Duplicate comments:
In @.github/workflows/e2e.yml:
- Around line 52-59: The container run currently uses docker run without joining
the Actions service network so the DATABASE_URL host "postgres" won't resolve;
update the docker run invocation to join the runner network (e.g., add --network
host or an explicit network that can see the service) and adjust ports/env if
needed so the app can reach Postgres; modify the docker run command that sets
DATABASE_URL and add the --network host (or create/join a user-defined network)
flag so the service hostname "postgres" resolves for the app container.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0e8cbaac-3d97-4c29-8d76-ee5f505e4493

📥 Commits

Reviewing files that changed from the base of the PR and between e575ea1 and 014ad35.

📒 Files selected for processing (1)
  • .github/workflows/e2e.yml
📜 Review details
🧰 Additional context used
🪛 Checkov (3.2.508)
.github/workflows/e2e.yml

[medium] 56-57: Basic Auth Credentials

(CKV_SECRET_4)

🔇 Additional comments (1)
.github/workflows/e2e.yml (1)

29-41: Nice consistency on pnpm across dependency install and Playwright execution.

This aligns the workflow with the repo package manager and avoids lockfile/tooling drift.

Also applies to: 75-81

Comment thread .github/workflows/e2e.yml Outdated
Comment thread e2e/auth.spec.ts Dismissed

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
.github/workflows/e2e.yml (1)

50-57: ⚠️ Potential issue | 🔴 Critical

DATABASE_URL host is unreachable from the app container network.

Line 54 points to @postgres, but docker run starts myapp on its own default network. Without explicit network alignment, the app container won’t reach the Postgres service, and e2e will fail at startup.

🐛 Proposed fix
       - name: Run app container
         run: |
           docker run -d --name myapp \
-            -p 8887:8887 \
-            -e DATABASE_URL=postgres://devuser:changeme@postgres:5432/devdb \
+            --network host \
+            -e DATABASE_URL=postgres://devuser:changeme@localhost:5432/devdb \
             -e NODE_ENV=production \
             -e PROJECT_URL=http://localhost:8887 \
             portabase/portabase:test
#!/bin/bash
set -euo pipefail
f=.github/workflows/e2e.yml
sed -n '48,60p' "$f"
rg -n 'Run app container|DATABASE_URL=.*@(postgres|localhost):5432|--network host' "$f" -C2
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/e2e.yml around lines 50 - 57, The DATABASE_URL uses host
"postgres" but the docker run for container "myapp" is started on Docker's
default network so it cannot reach the Postgres service; update the docker run
invocation (the block starting with "docker run -d --name myapp") to join the
same network as Postgres (e.g. add --network <e2e_network_name> or use --network
host if appropriate) so the "postgres" hostname resolves, or alternatively
change DATABASE_URL to a reachable host (e.g. host.docker.internal) if you
intentionally run on the host network; ensure the env var DATABASE_URL and the
docker run network flags are consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@e2e/auth.spec.ts`:
- Around line 11-14: The users object uses static emails causing cross-project
registration races; update the users declaration (the users: Record<string,
UserCredentials> object used by the "Successful register for admin" test) to
generate per-run unique emails (e.g., append a timestamp/UUID/process.pid or
Playwright testInfo.project.name to the local-part) so each test run/project
gets a distinct email; ensure the change is applied wherever users is
imported/used so the register flow uses the generated email for admin and normal
accounts.

---

Duplicate comments:
In @.github/workflows/e2e.yml:
- Around line 50-57: The DATABASE_URL uses host "postgres" but the docker run
for container "myapp" is started on Docker's default network so it cannot reach
the Postgres service; update the docker run invocation (the block starting with
"docker run -d --name myapp") to join the same network as Postgres (e.g. add
--network <e2e_network_name> or use --network host if appropriate) so the
"postgres" hostname resolves, or alternatively change DATABASE_URL to a
reachable host (e.g. host.docker.internal) if you intentionally run on the host
network; ensure the env var DATABASE_URL and the docker run network flags are
consistent.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 830eac64-91f1-4ad4-9db2-37f9aaea104d

📥 Commits

Reviewing files that changed from the base of the PR and between 014ad35 and 67d9de6.

📒 Files selected for processing (3)
  • .github/workflows/e2e.yml
  • .gitignore
  • e2e/auth.spec.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build-and-test
🧰 Additional context used
🪛 Checkov (3.2.508)
.github/workflows/e2e.yml

[medium] 54-55: Basic Auth Credentials

(CKV_SECRET_4)

🔇 Additional comments (1)
.gitignore (1)

51-56: Playwright ignore additions are appropriate.

These patterns correctly exclude transient test artifacts and auth state from source control.

Comment thread e2e/auth.spec.ts
coderabbitai[bot]
coderabbitai Bot previously requested changes Mar 18, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (2)
.github/workflows/e2e.yml (2)

50-57: ⚠️ Potential issue | 🔴 Critical

DATABASE_URL points to localhost inside myapp, so Postgres is unreachable.

At Line 54, localhost resolves inside the app container namespace. Since Line 52 does not attach myapp to host networking (or the same Docker network as the DB), DB connections will fail.

🐛 Proposed fix
       - name: Run app container
         run: |
           docker run -d --name myapp \
-            -p 8887:8887 \
+            --network host \
             -e DATABASE_URL=postgres://devuser:changeme@localhost:5432/devdb \
             -e NODE_ENV=production \
             -e PROJECT_URL=http://localhost:8887 \
             portabase/portabase:test
#!/bin/bash
set -euo pipefail

wf="$(fd -t f '^e2e\.yml$' .github/workflows | head -n1)"
echo "Workflow: $wf"
echo "---- Run app container block ----"
sed -n '48,60p' "$wf"

echo "---- Network/DB host checks ----"
rg -n -C2 'Run app container|docker run|--network host|DATABASE_URL=.*localhost:5432|DATABASE_URL=.*@postgres:' "$wf"

echo
echo "Expected after fix: '--network host' present in Run app container block."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/e2e.yml around lines 50 - 57, The Docker run in the "Run
app container" block sets DATABASE_URL to postgres://...@localhost:5432/devdb
which points to localhost inside the myapp container (so Postgres is
unreachable); fix by making the app container reach the DB either by adding the
docker run --network host flag to the docker run line or by configuring
DATABASE_URL to point at the DB container hostname on a shared Docker network
(e.g., replace localhost with the Postgres service/container name or attach
myapp to the same Docker network), updating the docker run invocation that
creates the myapp container and the DATABASE_URL environment variable
accordingly.

13-13: 🧹 Nitpick | 🔵 Trivial

Pin Postgres image by digest for reproducible CI.

Line 13 uses mutable tag postgres:17; this can change underneath the workflow and introduce nondeterministic failures.

What is the current immutable Docker Hub digest for `postgres:17`, and what is the recommended GitHub Actions `services.postgres.image` syntax to pin it as `postgres:17@sha256:...`?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/e2e.yml at line 13, Replace the mutable tag "image:
postgres:17" with an immutable digest-pinned image by setting
services.postgres.image to "postgres:17@sha256:<digest>"; to get the correct
digest, retrieve the current sha256 from Docker Hub or the registry (e.g., via
docker manifest/inspect or the image's Repository > Tags page) and substitute
that value into the image string so the workflow always uses the exact same
Postgres image.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/e2e.yml:
- Around line 59-71: The current readiness probe only checks TCP bind via "nc -z
localhost 8887" which is flaky; change the loop to poll a real application
readiness endpoint (e.g., HTTP GET to localhost:8887/health or /ready) instead
of just checking the port, retrying up to the same limit and succeeding only on
an expected 2xx body/response; keep the same retry/backoff behavior and preserve
the diagnostic steps (printing "Waiting..." messages, running "docker logs
myapp" and exiting 1 on timeout) so failures still produce logs.

---

Duplicate comments:
In @.github/workflows/e2e.yml:
- Around line 50-57: The Docker run in the "Run app container" block sets
DATABASE_URL to postgres://...@localhost:5432/devdb which points to localhost
inside the myapp container (so Postgres is unreachable); fix by making the app
container reach the DB either by adding the docker run --network host flag to
the docker run line or by configuring DATABASE_URL to point at the DB container
hostname on a shared Docker network (e.g., replace localhost with the Postgres
service/container name or attach myapp to the same Docker network), updating the
docker run invocation that creates the myapp container and the DATABASE_URL
environment variable accordingly.
- Line 13: Replace the mutable tag "image: postgres:17" with an immutable
digest-pinned image by setting services.postgres.image to
"postgres:17@sha256:<digest>"; to get the correct digest, retrieve the current
sha256 from Docker Hub or the registry (e.g., via docker manifest/inspect or the
image's Repository > Tags page) and substitute that value into the image string
so the workflow always uses the exact same Postgres image.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4eae4d01-d0c5-4e34-a678-77af7c43beec

📥 Commits

Reviewing files that changed from the base of the PR and between 67d9de6 and a2193eb.

📒 Files selected for processing (1)
  • .github/workflows/e2e.yml
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build-and-test
🧰 Additional context used
🪛 Checkov (3.2.508)
.github/workflows/e2e.yml

[medium] 54-55: Basic Auth Credentials

(CKV_SECRET_4)

Comment thread .github/workflows/e2e.yml

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
.github/workflows/e2e.yml (1)

53-54: ⚠️ Potential issue | 🟡 Minor

Remove -p 8887:8887 when using --network host.

With --network host, Docker ignores published ports, making the -p 8887:8887 flag misleading and confusing for future maintainers.

🔧 Proposed fix
       - name: Run app container
         run: |
           docker run -d --name myapp \
             --network host \
-            -p 8887:8887 \
             -e DATABASE_URL=postgres://devuser:changeme@localhost:5432/devdb \
             -e NODE_ENV=production \
             -e PROJECT_URL=http://localhost:8887 \
             portabase/portabase:test
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/e2e.yml around lines 53 - 54, Remove the misleading port
publish flag when running the container with host networking: delete the "-p
8887:8887" token from the Docker run arguments that also include "--network
host" so only "--network host" is used (remove the standalone "-p 8887:8887"
entry), ensuring the workflow's Docker invocation doesn't advertise a published
port that Docker ignores; keep the "--network host" flag intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In @.github/workflows/e2e.yml:
- Around line 53-54: Remove the misleading port publish flag when running the
container with host networking: delete the "-p 8887:8887" token from the Docker
run arguments that also include "--network host" so only "--network host" is
used (remove the standalone "-p 8887:8887" entry), ensuring the workflow's
Docker invocation doesn't advertise a published port that Docker ignores; keep
the "--network host" flag intact.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: cd49c443-db36-4176-98d8-204e2de5626a

📥 Commits

Reviewing files that changed from the base of the PR and between a2193eb and b901e8b.

📒 Files selected for processing (2)
  • .github/workflows/e2e.yml
  • app/layout.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build-and-test
🧰 Additional context used
🪛 Checkov (3.2.508)
.github/workflows/e2e.yml

[medium] 55-56: Basic Auth Credentials

(CKV_SECRET_4)

🔇 Additional comments (2)
app/layout.tsx (1)

9-9: Good branding fallback and single source of truth.

Line 9 keeps metadata title values consistent while providing a safe default ("Portabase") when PROJECT_NAME is unset.

.github/workflows/e2e.yml (1)

60-72: Readiness check is still TCP-only and can produce false positives.

This was already flagged earlier and still applies: nc -z only confirms socket bind, not application readiness (e.g., migrations/startup completion).

Is checking only `nc -z <host> <port>` considered sufficient for application readiness in CI, or is an HTTP health/readiness probe recommended?

@KillianLarcher KillianLarcher changed the title Feat/e2e feat: adding basic end-to-end testing and ci workflow Mar 18, 2026
@KillianLarcher
KillianLarcher merged commit 67246ef into main Mar 18, 2026
5 checks passed
@KillianLarcher
KillianLarcher deleted the feat/e2e branch March 18, 2026 18:47
@coderabbitai coderabbitai Bot mentioned this pull request Apr 8, 2026
@coderabbitai coderabbitai Bot mentioned this pull request May 15, 2026
@coderabbitai coderabbitai Bot mentioned this pull request Aug 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant