Skip to content

fix(maintenance): extract bypass cookie config into shared module - #885

Open
Rafiat30 wants to merge 5 commits into
Iris-IV:mainfrom
Rafiat30:fix/maintenance-bypass-cookie-config
Open

fix(maintenance): extract bypass cookie config into shared module#885
Rafiat30 wants to merge 5 commits into
Iris-IV:mainfrom
Rafiat30:fix/maintenance-bypass-cookie-config

Conversation

@Rafiat30

Copy link
Copy Markdown
Contributor

Closes #556

Summary

BYPASS_COOKIE_MAX_AGE was defined and exported from middleware.ts and was already being applied as max-age when MaintenanceBypass.tsx sets the maintenance bypass cookie via document.cookie. The remaining issue was architectural: a client component was importing constants from an Edge-runtime middleware file.

Changes

  • New file: src/lib/maintenanceConfig.ts — shared MAINTENANCE_COOKIE and BYPASS_COOKIE_MAX_AGE constants.
  • Modified: src/middleware.ts — imports the constants from the new shared module instead of defining/exporting them locally.
  • Modified: src/components/MaintenanceBypass.tsx — imports the constants from @/lib/maintenanceConfig instead of @/middleware.

Why

Client components importing from middleware.ts risk pulling Edge/Next.js-server-only code into the client bundle. Moving the shared constants into a plain module used by both sides removes that coupling without changing runtime behavior — the cookie already carried the correct max-age.

How to test

  1. Set NEXT_PUBLIC_MAINTENANCE_MODE=true and NEXT_PUBLIC_MAINTENANCE_ALLOWLIST=<your wallet address> in .env.local.
  2. Run the app, connect the allowlisted wallet.
  3. Inspect document.cookie in devtools — maintenance_bypass should have max-age=86400.
  4. npm run typecheck and npm test pass.

@drips-wave

drips-wave Bot commented Jul 30, 2026

Copy link
Copy Markdown

@Rafiat30 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@sshdopey

Copy link
Copy Markdown
Contributor

Auto-review failed (API error). Leaving PR for human review.

1 similar comment
@sshdopey

Copy link
Copy Markdown
Contributor

Auto-review failed (API error). Leaving PR for human review.

@davidmaronio

Copy link
Copy Markdown
Contributor

clean fix: moving MAINTENANCE_COOKIE/BYPASS_COOKIE_MAX_AGE into src/lib/maintenanceConfig.ts removes the client-component import of the edge middleware module, and the comment explaining why the module exists is appreciated. code looks good to merge.

one non-blocking note:

  1. src/tests/components/MaintenanceBypass.test.tsx:21: buildBypassCookie re-implements the component's cookie string in the test, so those two assertions verify the test helper rather than the component. the two rendered-component tests below are the ones doing real work; consider dropping or reframing the helper-based ones.

gate: branch is behind main; please update/rebase and let CI rerun, then this is good to go.

Move MAINTENANCE_COOKIE and BYPASS_COOKIE_MAX_AGE out of middleware.ts
into src/lib/maintenanceConfig.ts so the client-side MaintenanceBypass
component no longer imports from an Edge-runtime middleware file.
Behavior is unchanged: the bypass cookie already applied max-age from
BYPASS_COOKIE_MAX_AGE, this just fixes the import graph.

Refs Iris-IV#556
Adds regression coverage for the maintenanceConfig.ts extraction: locks the
24h BYPASS_COOKIE_MAX_AGE and MAINTENANCE_COOKIE values, verifies the exact
cookie string MaintenanceBypass.tsx builds embeds max-age from the shared
constant, and covers the no-wallet-connected render path.

Refs Iris-IV#556
buildBypassCookie duplicated the cookie string MaintenanceBypass.tsx
builds, so those assertions verified the test helper rather than the
component. The rendered-component tests below already cover the
real behavior.
@Rafiat30
Rafiat30 force-pushed the fix/maintenance-bypass-cookie-config branch from f87e211 to a2a20a1 Compare August 5, 2026 13:50
@sshdopey

sshdopey commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Auto-review failed (API error). Leaving PR for human review.

- Format useMultiSigProposals.test.tsx (Prettier check was failing on it)
- Add missing Causes.listView/mapView translation keys (en, es) — the
  view-toggle buttons on /causes reference these keys but they were
  never added, so next-intl throws MISSING_MESSAGE and the Playwright
  smoke test fails when it navigates through that page
- Add the missing getAllCampaigns mock to the AppPageComponents
  contractClient mock, and cover the two HomeClient stats/CTA branches
  that were never exercised as a result (queryFn was undefined, so
  the stats panel and the wallet-connecting CTA state never rendered)

None of this is related to this branch's actual change; main's CI
was already failing on all three checks before this branch rebased
onto it.
@sshdopey

sshdopey commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Auto-review failed (API error). Leaving PR for human review.

@Rafiat30

Rafiat30 commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

@davidmaronio pls help review. i have fixed every issue

@davidmaronio

Copy link
Copy Markdown
Contributor

this is verified and approved, but today's merge wave put the branch into conflict before it reached the front of the queue (lots of prs touching the same i18n and test files landed at once). please rebase onto current main and push, i'll merge as soon as it's green, no re-review needed.

@sshdopey

sshdopey commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Auto-review failed (API error). Leaving PR for human review.

@davidmaronio

Copy link
Copy Markdown
Contributor

the red checks were the pre-existing main breakage (sharebuttons lint, multisig test types), not your change. #867 just landed with the main fixes, so please rebase onto current main and this should go green, then i'll merge.

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.

[Bug] middleware.ts BYPASS_COOKIE_MAX_AGE exported but never used to set cookie — bypass tokens expire with browser session

3 participants