Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions .github/workflows/e2e-smoke.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
name: e2e-smoke
on:
pull_request:
workflow_dispatch:
jobs:
smoke:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: pnpm/action-setup@v4
with: { version: 10 }
- uses: actions/setup-node@v4
with: { node-version: 20, cache: pnpm }
Comment on lines +9 to +13

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Harden workflow actions: pin by commit SHA and disable checkout credential persistence.

Line 9/10/12/20 use tag refs (@v4) instead of immutable SHAs, and checkout doesn’t set persist-credentials: false. This weakens CI supply-chain posture.

Suggested hardening pattern
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@<full-commit-sha>
+        with:
+          persist-credentials: false
-      - uses: pnpm/action-setup@v4
+      - uses: pnpm/action-setup@<full-commit-sha>
-      - uses: actions/setup-node@v4
+      - uses: actions/setup-node@<full-commit-sha>
-      - uses: actions/upload-artifact@v4
+      - uses: actions/upload-artifact@<full-commit-sha>

Also applies to: 20-20

🧰 Tools
🪛 zizmor (1.25.2)

[warning] 9-9: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)


[error] 9-9: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)


[error] 10-10: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)


[error] 12-12: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/e2e-smoke.yml around lines 9 - 13, The workflow uses
mutable action tags and leaves checkout credentials persistent; update the three
actions (actions/checkout, pnpm/action-setup, actions/setup-node) to pinned
commit SHAs instead of tag refs (replace `@v4` with the corresponding immutable
SHA for each action) and change the checkout step (actions/checkout) to include
persist-credentials: false to disable token persistence; ensure any inputs like
node-version or pnpm version remain as before but reference the pinned action
SHAs and keep the same configuration keys (e.g., node-version, cache) when
updating the action refs.

Source: Linters/SAST tools

- run: pnpm install --frozen-lockfile
- run: pnpm exec playwright install --with-deps chromium
- run: pnpm test:e2e
env:
E2E_BASE_URL: ${{secrets.E2E_BASE_URL}}
VERCEL_AUTOMATION_BYPASS_SECRET: ${{secrets.VERCEL_AUTOMATION_BYPASS_SECRET}}
- uses: actions/upload-artifact@v4
if: success() || failure()
with:
name: playwright-report
path: playwright-report/
retention-days: 7
Comment thread
greptile-apps[bot] marked this conversation as resolved.
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,6 @@ scratch/
!tsconfig*.json
!jsconfig.json


test-results/
playwright-report/
20 changes: 20 additions & 0 deletions e2e/fixtures.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { test as base, expect } from "@playwright/test"

// Extends the base test so every test collects console + page errors.
// Assert `expect(consoleErrors).toEqual([])` at the end of each test.
export const test = base.extend<{ consoleErrors: string[] }>({
consoleErrors: async ({ page }, use) => {
const errors: string[] = []
page.on("console", (msg) => {
if (msg.type() === "error") {
console.error("[Browser Error]", msg.text())
errors.push(msg.text())
}
})
page.on("pageerror", (err) => errors.push(err.message))
// eslint-disable-next-line react-hooks/rules-of-hooks -- Playwright fixture 'use', not a React Hook
await use(errors)
},
})

export { expect }
116 changes: 116 additions & 0 deletions e2e/helpers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
import { Page, expect } from "@playwright/test"

export const DEMO = {
client: "+919876500001",
worker: "+919876500011",
admin: "+919876500099",
} as const

export const DEMO_OTP = "123456"

// Phone-OTP login (DEMO_MODE bypass code). TODO: align test ids/urls.
export async function login(page: Page, phone: string) {
await page.goto("/login")

await page.getByTestId("phone-input").fill(phone.replace(/^\+91/, ""))
await page.getByTestId("send-otp").click()

await expect(page).toHaveURL(/\/login\/verify/)

await page.getByTestId("otp-input").pressSequentially(DEMO_OTP, { delay: 50 })
Comment on lines +15 to +20

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: These selectors don't exist in the current login UI (phone-form.tsx and verify-form.tsx have no matching data-testid attributes), so login() will time out before authentication starts. Update the helper to target existing stable selectors or add matching test IDs in the login forms. [api mismatch]

Severity Level: Critical 🚨
- ❌ All auth-dependent Playwright tests fail before OTP entry.
- ❌ Smoke gate cannot validate boot/auth paths at all.
- ⚠️ CI signal for login regressions becomes unusable.
Steps of Reproduction ✅
1. Run the Playwright suite (e.g. `pnpm playwright test e2e/smoke.spec.ts`), which uses
the shared `login()` helper from `e2e/helpers.ts:12-22`.

2. In test `"phone-OTP login works for client and worker"` at `e2e/smoke.spec.ts:12-17`,
`login(page, DEMO.client)` is called, which navigates to `/login` and executes
`page.getByTestId("phone-input")` and `page.getByTestId("send-otp")` at
`e2e/helpers.ts:15-16`.

3. The login form implementation in `src/app/login/phone-form.tsx:39-58` renders the phone
input as `<Input id="phone" ... {...form.register("phone")}>` with no
`data-testid="phone-input"` or `"send-otp"` attributes, and
`src/app/login/verify/verify-form.tsx:71-120` similarly renders the OTP slots without
`data-testid="otp-input"`.

4. Because these `data-testid` attributes do not exist anywhere in the codebase (confirmed
by a project-wide grep for `data-testid="phone-input"`, `data-testid="send-otp"`, and
`data-testid="otp-input"`), Playwright cannot resolve these locators and the first
`login()` call times out, causing all tests that rely on `login()` to fail before
authentication.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** e2e/helpers.ts
**Line:** 15:20
**Comment:**
	*Api Mismatch: These selectors don't exist in the current login UI (`phone-form.tsx` and `verify-form.tsx` have no matching `data-testid` attributes), so `login()` will time out before authentication starts. Update the helper to target existing stable selectors or add matching test IDs in the login forms.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎


await expect(page).toHaveURL(/\/(client|worker|admin|onboarding)/)
}

export async function logout(page: Page) {
await page.locator("a[href$='/account']").click()
await page.getByTestId("sign-out").click()
await page.getByTestId("sign-out-confirm").click()
Comment on lines +27 to +28

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: The account UI currently has no sign-out / sign-out-confirm test IDs, so this logout helper cannot find its targets and will fail every flow that switches users. Align selectors with the actual SignOutButton dialog controls or add those test IDs. [api mismatch]

Severity Level: Critical 🚨
- ❌ Any test that switches users fails on logout.
- ❌ Escrow path tests B–F cannot complete end-to-end.
- ⚠️ Prevents validating multi-role session handling.
Steps of Reproduction ✅
1. Run `pnpm playwright test e2e/smoke.spec.ts`; tests call the shared `logout()` helper
from `e2e/helpers.ts:25-30` whenever switching between client and worker users (e.g.
`"phone-OTP login works for client and worker"` at `e2e/smoke.spec.ts:12-17`, Section
B/C/E/F tests).

2. `logout(page)` first clicks the account navigation link via
`page.locator("a[href$='/account']").click()` at `e2e/helpers.ts:26`, which matches
`/client/account` and `/worker/account` links defined in
`src/components/nav/client-nav-shell.tsx:10-12` and
`src/components/nav/worker-nav-shell.tsx:10-12`.

3. Once on the account page (`src/app/client/account/page.tsx:4-22` or
`src/app/worker/account/page.tsx:6-23`), the sign-out UI is rendered by `SignOutButton`
(`src/components/account/sign-out-button.tsx:21-69`), which exposes a `<Button>` and an
`AlertDialogAction` with visible text "Sign out" but no `data-testid="sign-out"` or
`data-testid="sign-out-confirm"`.

4. A repo-wide grep finds no `data-testid="sign-out"` or `"sign-out-confirm"` anywhere, so
`page.getByTestId("sign-out")` and `"sign-out-confirm"` at `e2e/helpers.ts:27-28` always
fail, causing every test that logs out to error at the first logout call.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** e2e/helpers.ts
**Line:** 27:28
**Comment:**
	*Api Mismatch: The account UI currently has no `sign-out` / `sign-out-confirm` test IDs, so this logout helper cannot find its targets and will fail every flow that switches users. Align selectors with the actual `SignOutButton` dialog controls or add those test IDs.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

await expect(page).toHaveURL(/\/login/)
Comment on lines +15 to +29

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Architect Review — CRITICAL

The Playwright login/logout and wallet top-up helpers depend on data-testid selectors (phone-input, send-otp, otp-input, sign-out, sign-out-confirm, add-money, topup-amount, topup-confirm) that are not present anywhere in the current UI components, so the core smoke tests cannot drive these flows and will fail.

Suggestion: Add matching data-testid attributes to the login/verify forms, sign-out dialog, and TopUpDialog components, or update the helpers/tests to use stable role/name-based locators that match the existing markup.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is an **Architect / Logical Review** comment left during a code review. These reviews are first-class, important findings — not optional suggestions. Do NOT dismiss this as a 'big architectural change' just because the title says architect review; most of these can be resolved with a small, localized fix once the intent is understood.

**Path:** e2e/helpers.ts
**Line:** 15:29
**Comment:**
	*CRITICAL: The Playwright login/logout and wallet top-up helpers depend on data-testid selectors (phone-input, send-otp, otp-input, sign-out, sign-out-confirm, add-money, topup-amount, topup-confirm) that are not present anywhere in the current UI components, so the core smoke tests cannot drive these flows and will fail.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
If a suggested approach is provided above, use it as the authoritative instruction. If no explicit code suggestion is given, you MUST still draft and apply your own minimal, localized fix — do not punt back with 'no suggestion provided, review manually'. Keep the change as small as possible: add a guard clause, gate on a loading state, reorder an await, wrap in a conditional, etc. Do not refactor surrounding code or expand scope beyond the finding.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

}

// Reads a money value rendered with data-testid="wallet-available" / "wallet-locked".
export async function balance(page: Page, which: "available" | "locked") {
const raw = await page.getByTestId(`wallet-${which}`).innerText()
return Number(raw.replace(/[^0-9.]/g, ""))
}

// Waits for a specific RPC to return 2xx (proves the money write happened).
export async function expectRpcOk(page: Page, urlPart: string, act: () => Promise<void>) {
const [res] = await Promise.all([
page.waitForResponse((r) => r.url().includes(urlPart) && r.request().method() === "POST"),
act(),
])
expect(res.status(), `${urlPart} should return 2xx`).toBeLessThan(400)
return res
}
Comment on lines +39 to +46

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 expectRpcOk is exported but never called in the spec

The helper is fully implemented and documented but smoke.spec.ts never imports or uses it. If it's intended for future tests, a comment explaining its planned use would help; if it's dead code from an earlier design, removing it keeps the test surface clean.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


/**
* Drive the 6-step PostJobForm wizard as the currently-logged-in client.
* Navigates to /client/jobs/new, fills all required fields, submits.
* Returns the newly created job's ID extracted from the redirect URL.
*/
export async function postJob(
page: Page,
opts: { title?: string; budget?: string; milestoneTitle?: string } = {},
): Promise<string> {
const title = opts.title ?? "E2E test job"
const budget = opts.budget ?? "1000"
const milestoneTitle = opts.milestoneTitle ?? "Complete work"

await page.getByTestId("post-job").click()
await expect(page).toHaveURL(/\/client\/jobs\/new/)

// Step 1: Basics
await page.getByTestId("job-title").fill(title)
await page.getByRole("combobox").click()
await page.getByRole("option").first().click()
await page.locator("#description").fill("E2E smoke test job for escrow flow testing")
await page.getByRole("button", { name: "Next", exact: true }).click()

// Step 2: Location
await page.locator("#location_text").fill("Banjara Hills, Hyderabad")
await page.getByRole("button", { name: "Next", exact: true }).click()

// Step 3: Budget
await page.locator("#total_budget").fill(budget)
await page.getByRole("button", { name: "Next", exact: true }).click()

// Step 4: Milestones
await page.locator("#ms-title-0").fill(milestoneTitle)
await page.getByTestId("milestone-amount-0").fill(budget)
await page.getByRole("button", { name: "Next", exact: true }).click()

// Step 5: Materials — skip
await page.getByRole("button", { name: "Next", exact: true }).click()

// Step 6: Review → Submit
await page.getByTestId("job-submit").click()
await expect(page).toHaveURL(/\/client\/jobs\/[0-9a-f-]+$/, { timeout: 15_000 })

return page.url().split("/").pop()!
}

/**
* Worker applies to a job (navigates to /worker/jobs/:id, opens modal, fills bid).
*/
export async function applyToJob(page: Page, jobId: string, bid: string = "1000") {
await page.goto(`/worker/jobs/${jobId}`)
await page.getByTestId("apply-job").click()
await page.locator("#bid_amount").fill(bid)
await page.locator("#eta_days").fill("7")
await page.getByRole("button", { name: /submit application/i }).click()
await expect(page.getByRole("button", { name: /submit application/i })).toBeHidden({
timeout: 15_000,
})
}

/**
* Client accepts first applicant on a job (navigates to /client/jobs/:id).
* After accept, the page redirects to /milestones.
*/
export async function acceptApplicant(page: Page, jobId: string) {
await page.goto(`/client/jobs/${jobId}`)
await page.getByTestId("accept-applicant").click()
await expect(page).toHaveURL(/\/client\/jobs\/[0-9a-f-]+\/milestones/, { timeout: 15_000 })
}
Loading
Loading