Skip to content

fix(auth): resolve the conflict markers left in the verification OTP path - #595

Closed
AbdullahM07 wants to merge 1 commit into
oblien:mainfrom
AbdullahM07:fix/auth-otp-conflict-markers
Closed

fix(auth): resolve the conflict markers left in the verification OTP path#595
AbdullahM07 wants to merge 1 commit into
oblien:mainfrom
AbdullahM07:fix/auth-otp-conflict-markers

Conversation

@AbdullahM07

Copy link
Copy Markdown
Member

Fixes #594.

74ec3f72 ("patch v0.6.6") committed unresolved merge conflict markers into apps/api/src/lib/auth.ts, in the emailOTP plugin's sendVerificationOTP. main has not typechecked since.

Before

$ bun run lint
src/lib/auth.ts(418,1): error TS1185: Merge conflict marker encountered.
src/lib/auth.ts(420,1): error TS1185: Merge conflict marker encountered.
src/lib/auth.ts(428,1): error TS1185: Merge conflict marker encountered.

esbuild also cannot parse the file, so 48 API test files fail at transform time — everything that reaches auth.ts through middleware/auth.ts.

After

$ bun run lint          # apps/api
# clean, exit 0

$ bun run test          # apps/api
Test Files  354 passed | 1 failed (355)
     Tests  4158 passed | 1 failed | 3 skipped (4162)

The one remaining failure is service-exec-isolation.test.ts hitting the 20s test timeout under full-suite load; it passes in isolation in ~8s and is unrelated to this change. packages/core is 608/608.

Which side this keeps

The fail-loudly one:

if (!(await sendMail({ to: email, ...tmpl }))) {
  throw new APIError("SERVICE_UNAVAILABLE", { ... });
}

That is already what the forget-password branch does a few lines below in the same function, and its comment states the reasoning: sendMail returns silently when no transport is configured, so without the result check the user is told to check their inbox for a code that was never sent. The discarded HEAD side was the older fire-and-forget call that v0.6.6 was in the process of replacing.

Four deleted lines, no behavior change beyond removing the markers. No test asserted the silent behavior.

Not addressed here

#594 also notes that #586 merged with Typecheck and Test red. This PR only unbreaks main; the branch protection gap that allowed it is left for a separate discussion.

…path

`74ec3f72` ("patch v0.6.6") committed unresolved merge conflict markers into
the `emailOTP` plugin's `sendVerificationOTP`, so `main` stops at
`TS1185: Merge conflict marker encountered` and 48 API test files fail to load
— esbuild cannot parse the file, and everything importing it through
`middleware/auth.ts` dies at transform time.

Keeps the fail-loudly side. It is what the `forget-password` branch directly
below already does: check `sendMail`'s result and throw SERVICE_UNAVAILABLE
rather than tell someone to check their inbox for a code that was never sent.
The discarded side is the fire-and-forget behavior v0.6.6 was replacing.

No behavior change beyond removing the markers.
@AbdullahM07

Copy link
Copy Markdown
Member Author

Superseded by b2223b9, which landed the same fix — closing.

For the record the two resolutions are byte-identical: both go e2d84e81..b4394ed4 on this file, keep the guarded side, and cite the same reason (a verification code that could not be sent has to surface as SERVICE_UNAVAILABLE rather than a silent success). Nothing here to merge.

The compile break on main is fixed as of 77f67dc.

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.

main does not typecheck — conflict markers committed in apps/api/src/lib/auth.ts

1 participant