Fix reproduced regressions in StepsTool and dev email trap parsing#146
Fix reproduced regressions in StepsTool and dev email trap parsing#146
Conversation
Co-authored-by: Deepak Thomas <thinkdj@users.noreply.github.com>
Co-authored-by: Deepak Thomas <thinkdj@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes two reproduced regressions across @ottabase/ottaeditor (StepsTool validation) and @ottabase/auth (dev email trap inbox sizing) by making validation/parsing more defensive and documenting the updated behavior.
Changes:
- Harden
StepsTool.validate()to avoid throwing on malformed importeditems. - Add a dedicated parser for
DEV_EMAIL_TRAP_MAX_EMAILSand use it when creating the dev email trap store in auth. - Add/adjust tests and README documentation for both behaviors.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ottaeditor/src/tools/StepsTool/StepsTool.ts | Makes validate() defensive against malformed items entries to prevent runtime exceptions. |
| packages/ottaeditor/src/tools/StepsTool/StepsTool.test.ts | Adds coverage for malformed validation payloads not throwing. |
| packages/ottaeditor/README.md | Documents defensive validation behavior for imported content. |
| packages/auth/src/providers.ts | Introduces parseDevEmailTrapMaxEmails() helper used for consistent max inbox sizing. |
| packages/auth/src/backend-handler.ts | Switches dev-trap store maxEntries to the new parsing helper. |
| packages/auth/src/tests/backend-handler.test.ts | Adds a regression test for non-positive DEV_EMAIL_TRAP_MAX_EMAILS fallback behavior. |
| packages/auth/README.md | Documents fallback behavior for invalid/non-positive dev-trap inbox size. |
| return fallback; | ||
| } | ||
|
|
||
| return Math.floor(parsed); |
There was a problem hiding this comment.
parseDevEmailTrapMaxEmails() can return 0 for fractional values between 0 and 1 (e.g. "0.5" -> Math.floor(0.5)), which would configure the dev-trap store with maxEntries: 0 and effectively prune all messages. Consider flooring/truncating first and then enforcing >= 1 (or falling back) so the function always returns a positive integer as intended; if you do this, the worker-side helper should be kept consistent too.
| return Math.floor(parsed); | |
| const maxEmails = Math.floor(parsed); | |
| if (maxEmails < 1) { | |
| return fallback; | |
| } | |
| return maxEmails; |
| of sending through SMTP or an external API. When you set `DEV_EMAIL_TRAP_MAX_EMAILS`, non-positive or invalid values now | ||
| fall back to the default inbox size instead of shrinking the trap to a single message. |
There was a problem hiding this comment.
The README says “non-positive or invalid values” fall back. With the current parser implementation, fractional values between 0 and 1 (e.g. "0.5") are positive but would floor to 0, which is neither a useful inbox size nor covered by this wording. Either tighten the parser to guarantee a >= 1 integer (recommended) or clarify here that values are truncated/floored and anything < 1 falls back.
| of sending through SMTP or an external API. When you set `DEV_EMAIL_TRAP_MAX_EMAILS`, non-positive or invalid values now | |
| fall back to the default inbox size instead of shrinking the trap to a single message. | |
| of sending through SMTP or an external API. When you set `DEV_EMAIL_TRAP_MAX_EMAILS`, values are truncated to an integer and | |
| anything less than 1 or invalid falls back to the default inbox size instead of shrinking the trap to a single message. |
| export function parseDevEmailTrapMaxEmails(value: string | undefined, fallback = 50): number { | ||
| const parsed = Number(value); | ||
| if (!Number.isFinite(parsed) || parsed <= 0) { | ||
| return fallback; | ||
| } | ||
|
|
||
| return Math.floor(parsed); | ||
| } |
There was a problem hiding this comment.
PR description mentions “reuse a shared positive-integer parser” for DEV_EMAIL_TRAP_MAX_EMAILS, but the template worker still has its own toPositiveInteger() helper in apps/ottabase-template-app-tanstack/worker/lib/email-provider.ts with the same logic. Consider switching that code to import parseDevEmailTrapMaxEmails (or moving the parser to a truly shared utilities module) so the behavior can’t drift over time, or adjust the PR description if sharing across packages isn’t intended.
|
@copilot summarize (heading + short desc on what to do) for all the valid items from this PR. I will use the summary to discuss with peers |
Here's a summary of valid items from the PR review, each with a short description of what to do: 1. 2. Duplicate parser — 3. README wording — "non-positive" doesn't cover truncated fractions All three items have been addressed in commit |
StepsTool.validate()guard against malformed imported itemsparseDevEmailTrapMaxEmailsshared parser in@ottabase/auth/providers>= 1check so "0.5" falls back instead of settingmaxEntries: 0toPositiveInteger()helper in worker'semail-provider.tsand import the sharedparseDevEmailTrapMaxEmailsfrom@ottabase/auth/providersinsteadpackages/auth/README.mdwording to clarify truncation ("less than 1" falls back)backend-handler.test.ts⌨️ Start Copilot coding agent tasks without leaving your editor — available in VS Code, Visual Studio, JetBrains IDEs and Eclipse.