Skip to content

Redesign the transactional emails, AI push copy and error messages (R20) - #532

Merged
thomasluizon merged 34 commits into
redesign/mainfrom
feature/ticket-75-emails
Sep 25, 2026
Merged

thomasluizon merged 34 commits into
redesign/mainfrom
feature/ticket-75-emails

Conversation

@thomasluizon

@thomasluizon thomasluizon commented Sep 18, 2026 •

Copy link
Copy Markdown
Owner

Closes thomasluizon/orbit-tickets#75 (R20).

Inventory, counted from the tree

The ticket's counts are all low. I trusted the tree.

surface ticket says tree has where
Email types 6 + layout 7 + layout src/Orbit.Infrastructure/Email/Templates/
ErrorMessages constants 138 146 src/Orbit.Application/Common/ErrorMessages.cs
DomainErrors constants 74 83 src/Orbit.Domain/Common/DomainErrors.cs
Error codes not stated 146 src/Orbit.Application/Common/ErrorCodes.cs
AI generators at 0.7 to 0.9 3 3 push/summary, 5 in the band src/Orbit.Infrastructure/Services/
  • The seventh email is ApiKeyCreation (.html + .txt), which no ticket owned. Every type but the layout ships an html and txt pair, and both are updated.
  • The two extra services in the temperature band are AiGoalReviewService and AiRetrospectiveService at 0.7. Neither is push copy, so neither is in scope.
  • ErrorCopy covers 222 distinct codes, which is every code in ErrorCodes, ErrorMessages and DomainErrors, in EN and pt-BR.

Emails

DESIGN.md put the old templates in breach on six counts, so this is a rebuild rather than a recolour:

  1. Accent #7F46F7 violet, against the granted warm orange #C4530F.
  2. A gradient header band, against No gradient wash.
  3. box-shadow: 0 8px 28px rgba(127, 70, 247, 0.45) on every CTA, against No decorative glow.
  4. Rubik and Inter, against Geist Sans / Space Grotesk / Geist Mono.
  5. A planet emoji and sparkle glyph bullets, against No sparkle icon; identity is the mark.
  6. The accent painted on the verification code, which is not one of the four rationed accent roles. The code is now --fg-1 on a well.

Dark mode, honestly

Email clients are the constraint, so this is stated plainly rather than claimed.

  • The templates now author light inline and override dark through @media (prefers-color-scheme: dark), with color-scheme: light dark. That matches "light mode MANDATORY, dark primary".
  • The dark block needs !important because an inline style beats a <style> rule everywhere.
  • Apple Mail, iOS Mail and Outlook for macOS honour the query. Gmail and Outlook.com do not; they apply their own inversion. Inverting a light document degrades gracefully, which is exactly why the old dark only document was the riskier choice.
  • Web fonts do not load in most clients. Geist is named first and the stack falls back to the system UI face, so the mark carries identity rather than the type.
  • translate="no" is on the title and the logo alt. It is not inside the body copy, because those strings are shared with the .txt part and cannot carry markup.

LogoUrl is untouched. It is a load bearing URL owned by the web repo.

Voice

BRAND.md is the authority, and the reasoning is short: Orbit sells to adults who cannot keep a routine, so the emails describe a mechanism and name one next action rather than listing features. The welcome email's three feature bullets became three things to do first, and its CTA is a plain verb, "Add your first habit". Every email now says what happened before what to do next. Zero exclamation marks, zero dash characters, both languages edited together. "Orbit" and "Astra" are never translated.

AI push copy

The slip alert and proactive prompts each carried a worked example that demonstrated the two things the voice bans:

- Title: 5-8 words max, personal and warm (e.g., "Stay strong today!" or "You've got this!")
- Be creative and varied -- don't use the same structure every time

At temperature 0.9 the model copies its examples, so the prompt was the slop source. The daily summary prompt had a doubled hyphen in eleven places, including its one GOOD example.

NotificationVoice.Rules is now the single voice contract injected into all three prompts. It states each ban directly and carries no example at all. The fallback strings carried the same two tells and are rewritten in both languages.

Two things I did not do, deliberately:

  • No post-processing scrub. The ticket says to pin the register in the prompt, not after it.
  • No temperature change. The prompt is the instructed lever. The residual risk is real and belongs on the record: a prompt constraint at 0.9 is probabilistic, not a guarantee.

Tests assert the constraint, never the wording: every prompt carries the contract, and no prompt contains an exclamation mark, an em dash, an en dash or a doubled hyphen.

Error messages, and the leak

The reported bug reproduces exactly. The API returns { error, errorCode } with an English message. The UI translates client side from a 62 code map plus 30 rules that match English message substrings. DAYS_REQUIRE_QUANTITY_ONE is in neither, and the domain guard's wording does not contain the substring the validator's wording matches, so the raw English falls through. Matching on server prose is the defect class, not the one string.

The fix follows the ticket's own instruction that the user facing text is a separate localised constant selected by the error code:

  • ErrorCopy maps every code to EN and pt-BR copy. Each entry answers what happened and what to do next.
  • LocalizedErrorResultFilter swaps the message at the response boundary, using Accept-Language. One seam, no call site churn: threading a language through the 190 call sites of ToErrorResult and ToPayGateAwareResult would have put the same three lines in 190 places and still depended on each remembering.
  • DomainErrors is untouched. Its sentences stay developer facing, as the ticket requires.
  • The catalog is total, and ErrorCopyTests proves it by reflection over all three sources. A new constant without copy fails the build, so the raw message fallback is unreachable rather than merely unused.
  • ErrorMessages now takes its English from the same catalog, which deletes 145 sentences that were being written in two files and could silently drift.

AppError and Result now carry the arguments behind a {0} placeholder, because Format bakes them into the message and the boundary otherwise cannot format the localised template with the same values.

INVALID_CODE_ATTEMPTS_REMAINING is added. INVALID_VERIFICATION_CODE was shared by constants that differ in how many values they take, and copy selected by code cannot serve both. Adding a code is append only; the old one still serves the no argument case.

Found and fixed on the way

  • Five pay gate strings ending in Upgrade to unlock!, a banned word and an exclamation on a sell. They now share one calm sentence, since the screen already names the feature.
  • Three chat tools held inlined copies of error sentences.
  • UnhandledExceptionHandler hardcoded its own 500 message.
  • ConfirmWaitlistCommand returned an uncoded raw string.

Wire compatibility

Shape, status codes and the JSON property names are unchanged. ErrorResponse pins its names and marks Args [JsonIgnore].

One value changed and is now back: the wrong-code failure on the deletion and API key step-up flows keeps INVALID_VERIFICATION_CODE. Round 2 finding C2 caught it moving to a new code, which both shipped clients have no branch for.

Round 2 review, pull request 532 at dd862844

Sixteen findings, four blocking. Every one is answered below. Nothing is left open except the two cross repo items at the end, which are named rather than silently dropped.

C1, CRITICAL. The deletion email promised a wipe the product does not perform

ConfirmAccountDeletionCommandHandler calls user.Deactivate(scheduledDate), AccountDeletionService removes the row at that date, and signing in again calls User.CancelDeactivation. The intro now names the window and the way out of it, and reads the number from AppConstants.MaxDeletionGraceDays rather than typing it.

You asked to delete your Orbit account. Orbit deactivates it first, then removes it for good within 30 days. Sign in again while it is deactivated and the account comes back with everything in it.

The window is written as an upper bound because the send happens before confirmation, so the exact date is not known yet: a free account is scheduled 7 days out and a Pro account at plan expiry plus 7, both capped at the constant.

C2, HIGH. The new error code broke the step up screen on both shipped clients

Taken: keep the existing code. ErrorMessages.InvalidDeletionCode and InvalidApiKeyCreationCode return to ErrorCodes.InvalidVerificationCode, and ErrorCodes.InvalidCodeAttemptsRemaining is deleted.

The count needed a sentence the plain catalog entry cannot carry, so ErrorCopy grew a counted variant selected by the presence of arguments rather than by a second code. Its trailing Remaining attempts: {0} is byte identical in both languages on purpose: packages/shared/src/utils/step-up.ts:117 parses it with /remaining attempts:\s*(\d+)\s*$/i, and a translated token would lose the count for every pt-BR reader the moment C3 makes the reply Portuguese. The sentence never reaches a screen. step-up.tsx:229 and step-up-screen.tsx:194 render stepUp.attemptsOne and stepUp.attemptsMany from their own localized plurals, and the wrong phase shows t('stepUp.wrong'), not the API message.

That also answers C7, the ungrammatical "1 tries": the number is no longer rendered by this API in prose, so no plural form is written here.

Checked by TheAttemptsCountStaysReadableByTheShippedClients and TheAttemptsVariantKeepsTheTokenTheShippedClientsParse, both of which assert the code and match the same regex the clients use.

C3, HIGH. The localized copy never reached the Android app

RequestLanguageResolver reads the signed in account's stored User.Language and keeps Accept-Language as the anonymous fallback, which is what ResendEmailService and ProactiveCheckinSchedulerService already do. The filter became IAsyncResultFilter to allow the lookup, which runs only on a response that already carries an error body.

It is deliberately not cached: a cached language answers in the old one for as long as the entry lives after somebody changes it, and the read is one indexed row on a path that is rare by construction.

D6 is fixed in the same place. LocaleHelper.IsPortugueseAcceptLanguage ranks the header's tags by quality value, so pt;q=0.1,en;q=0.9 resolves to English.

C4, HIGH. The slip alert asserted a time the scheduler contradicts

CalculateAlertTime sends two hours before the peak, and at 08:00 when SlipPattern.PeakHour is null. The prompt now states when the push lands, not only when the pattern is, and adds one rule: never say or imply that it is now the usual time. The fallback branches on the peak hour and makes no time claim without one.

The mediums

  • D1. Taken: narrow the doc comment. ValidationExceptionHandler is an IExceptionHandler that writes its own ValidationFailure body and never produces an ObjectResult, so the filter cannot see it. Routing it through ErrorCopy needs an error code on every rule across 139 validator files and 285 WithMessage sites, which is its own ticket rather than a line in this one. Both claims that read wider than the code are now narrowed, in LocalizedErrorResultFilter and in ErrorCopy.
  • D2. Not fixed here, and it is the one thing that still needs a paired UI ticket. See Cross repo below.
  • C6. The 429 body is built from ErrorMessages.TooManyRequests.ToErrorBody() at both sites in DistributedRateLimitAttribute, so the filter matches it and RATE_LIMITED is reachable. The 429 now carries an error code for the first time. limit, count and retryAfterUtc leave the body; Retry-After and X-Orbit-Request-Id already carry the timing and the request id, and no client reads the three fields.
  • C5. The check in fallback counts: one open habit reads as one, and nothing promises automatic logging, because LogHabitTool records only the habit a person names.
  • C8. The sync copy states the circumstance and an action that exists, instead of promising a reload and a later send that no caller performs.
  • C9. CHALLENGE_CLOSED English reads "This challenge is no longer open to new people."
  • C10. RECAP_MONTH_NOT_CLOSED points at coming back rather than at an arrival that only happens for a reader with a PushSubscriptions row.
  • D3. AiSummaryService.BuildSummaryPrompt is internal static and joins both voice theory sources, so the guard written for its doubled hyphen leak reaches it in both languages.
  • D4. The comment said a missing entry fails at startup. ErrorMessages is a static class of static readonly fields, so it is a TypeInitializationException on the first request that touches the class, and ErrorCopyTests is what keeps it out of production.
  • D5. AppError.Equals and GetHashCode walk Args element by element. MaxTagsPerHabit.Format(5).Equals(MaxTagsPerHabit.Format(5)) is true again, covered at equal counts, different counts, and against the unformatted template.

The change detector tests

TheVoiceContractBansEveryCharacterClassTheOldPromptsLeaked, TheVoiceContractBansTheSellingRegister and TheVoiceContractNamesEveryBannedHypeWord are replaced rather than defended. The three assertions now run over every prompt a model actually receives, through EveryGeneratedUserPrompt, so they redden when a generator stops carrying the contract rather than only when somebody edits the constant they were reading.

EveryLocalizedEmailDiffersBetweenTheTwoLanguages pairs by (Email, Field) instead of by list index, and asserts both key sets match, so a copy record that gains a property cannot make it compare mismatched pairs.

Cross repo

D2 needs a ticket in orbit-ui-mobile and this pull request should wait for it. packages/shared/src/utils/error-utils.ts:252-305 maps errors to i18n keys by matching the backend's English sentence, and getFriendlyErrorKey returns the caller's generic fallbackKey when nothing matches. The rewritten copy stops matching, so TITLE_REQUIRED, UNIT_REQUIRED, TARGET_VALUE_INVALID, TAG_NAME_REQUIRED, DUPLICATE_SCHEDULED_REMINDERS, MAX_SCHEDULED_REMINDERS, MAX_HABITS_PER_GOAL, MAX_TAGS_PER_HABIT and GENERAL_HABIT_IS_BAD fall to a generic toast. The ticket adds each to ERROR_CODE_TO_KEY and deletes the text matching rules those codes depend on.

I did not file it. This repository has no tools/create-ticket.mjs; the adapter lives in orbit-ui-mobile, and the brief for this worker forbids touching a second repository. The orchestrator owns filing it, and the paragraph above is the ticket body.

A second ticket covers D1: give every FluentValidation rule an error code so ValidationExceptionHandler can resolve copy through ErrorCopy. 139 validator files, 285 WithMessage or AbstractValidator sites. VerifyCodeCommandValidator.cs:19 ships "Code must be a 6-digit number" verbatim to a pt-BR reader today.

Not closed

  • Astra tool results surface Result.Error through IAiTool.cs:34. The ticket assigns that rendering to ORB-17 and ORB-43, so the code to copy mapping is here and the rendering is theirs.
  • Screenshots are not attached. Per D13 only a human grants visual completion, and the no screenshot rule stands.

Assumptions

  • The deletion email states the grace window as an upper bound read from AppConstants.MaxDeletionGraceDays rather than branching free against Pro. Rejected: adding a plan flag to SendAccountDeletionCodeEmailJob, whose arguments Hangfire persists, for a distinction the reader cannot act on differently.
  • The counted attempts copy keeps its English Remaining attempts: token in the pt-BR string. Rejected: translating it, which satisfies the catalog's voice rule and then drops the count for every pt-BR reader, because the shipped clients parse the English token and never display the sentence.
  • RequestLanguageResolver reads the row on every error response instead of caching the language. Rejected: a memory cache keyed by user id, which answers in the old language for the life of the entry after somebody changes it.
  • C3 is covered by a unit test over the real filter and the real resolver with a substituted repository, not by an integration test. Rejected: a real database harness, which tests/CLAUDE.md forbids.
  • The 429 body drops limit, count and retryAfterUtc. Rejected: keeping them by widening ErrorResponse, which would add three fields to every error body in the API.
    Equal quality values retain header order; focused tests passed before and after refactoring.
    Kept redesign/main’s Sunday or Monday recap anchor; current week preference no longer controls it.

Test evidence

Full suite, after the fixes, at 63f77942:

dotnet build Orbit.slnx                        0 Errors
dotnet test Orbit.slnx --no-build
  Orbit.Analyzers.Tests            32 passed, 0 failed
  Orbit.Domain.Tests              585 passed, 0 failed
  Orbit.Application.Tests        4070 passed, 0 failed
  Orbit.Infrastructure.Tests     2404 passed, 0 failed
                                 ----
                                 7091 passed, 0 failed

Red first, for each blocking finding

The four defects were reintroduced together in the working tree on top of 147746ad, the suite was rebuilt, and the new tests were run with the defects present. Command:

dotnet test Orbit.slnx --no-build --filter "FullyQualifiedName~EmailCopyTests|FullyQualifiedName~ErrorCopyTests|FullyQualifiedName~LocalizedErrorResultFilterTests|FullyQualifiedName~AiSlipAlertMessageServiceTests"

Observed with the defects present, exit code 1:

Finding Test With the defect After the fix
C1 EmailCopyTests.TheDeletionEmailNamesTheGraceWindowAndTheWayOutOfIt, both languages FAIL PASS
C1 EmailCopyTests.TheDeletionEmailNeverCallsTheRequestIrreversible, both languages FAIL PASS
C2 ErrorCopyTests.TheAttemptsVariantKeepsTheTokenTheShippedClientsParse, both languages FAIL PASS
C2 LocalizedErrorResultFilterTests.TheAttemptsCountStaysReadableByTheShippedClients, both languages FAIL PASS
C3 LocalizedErrorResultFilterTests.APortugueseAccountSendingNoHeader_GetsThePortugueseCopy FAIL PASS
C3 LocalizedErrorResultFilterTests.TheStoredLanguageWinsOverTheHeader FAIL PASS
C4 AiSlipAlertMessageServiceTests.TheFallbackWithoutAPeakHour_MakesNoTimeClaim, both languages FAIL PASS
C4 AiSlipAlertMessageServiceTests.TheFallbackWithAPeakHour_PlacesTheHabitLaterThanTheSend FAIL PASS
The unchanged test that did not catch C3 is LocalizedErrorResultFilterTests.MissingAcceptLanguage_FallsBackToEnglish. It passed with the defect present in that same run, and it still passes after the fix: a request with no header and no signed in account is anonymous, which is the one case the header only rule answered correctly.

Gates checked locally

  • node tools/check-dashes.mjs --files <changed> exit 0, --check-baseline exit 0.
  • ORBIT0001 does not run under the locally installed SDK, so every changed .cs file was scanned for bare // and /* */. Zero hits.
  • No new #pragma warning disable or [SuppressMessage], so tools/suppression-allowlist.json is unchanged.
  • node tools/arch-map.mjs regenerated architecture.json and architecture.html, committed with the source change.
    Build: 0 errors. Plain dotnet test: 9 culture failures. LC_ALL=en_US.UTF-8 dotnet test: 7,286 passed.
    architecture.html: regenerated twice; output was identical.
    architecture.json: regenerated twice; output was identical.
    ErrorMessages.cs: kept localized mappings for every error added by main.
    Google auth tests: kept the copy assertion and sign-in retry checks. Commits: 65ff5f5d, d0573474. Build: 0 errors; EF: no pending changes. All 7,412 tests pass with LC_ALL=en_US.UTF-8; the default locale fails nine formatting tests.

Manual steps

None. Nothing outside the repository has to change for this pull request to take effect.
None.
None.

thomasluizon and others added 4 commits September 16, 2026 11:11
* Start #372

* Add habit widget empty reason
* chore: start ORB-223

* feat: generate gating matrix

* fix: fail closed on unreadable plan gates

* fix: close gating matrix parser gaps

* fix: preserve plans in combined guards

* fix: distinguish quota lifted plans

* fix: carry plan provenance through quota aliases

Derived quota variables were recorded unscoped and only the variable that
literally contains the plan ternary was relabelled afterwards, so plan
provenance did not survive a local alias. A semantics-preserving
`var selectedLimit = user.HasProAccess ? proLimit : freeLimit;
var messageLimit = selectedLimit;` generated cleanly and changed
CanSendAiMessage.quotaLiftedByPlan from "Pro" to null.

The directly plan-selected variables are now labelled first, and the
fixed-point propagation carries the source variable's exact plan instead of
null. An assignment deriving from two different plans cannot have its
provenance proven, so generation fails closed with the same
"cannot derive plan requirement" error the other unprovable shapes use.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: fail closed on unknown gating sources

* fix: derive feature flags from migrations

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
)

Mirrors thomasluizon/orbit-ui-mobile#1022.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes thomasluizon/orbit-tickets#75 (R20).

Emails. All 7 types plus the shared wrapper move to the granted canon: the
violet #7F46F7 becomes the warm orange #C4530F, the gradient header band and
every CTA glow are gone (DESIGN.md bans both), Rubik and Inter become the
Geist Sans and Geist Mono stacks, and the planet emoji and sparkle bullets
give way to the mark and a plain numbered list. Emails now author light and
override dark through prefers-color-scheme, which matches "light mode
MANDATORY, dark primary" and survives a forced inversion far better than the
dark only document did. Copy is rewritten in both languages to say what
happened before what to do next. Support copy moves out of its template into
EmailCopy so markup holds no strings.

AI push copy. The slip alert, proactive check in and daily summary prompts
demonstrated an exclamation mark and a doubled hyphen inside their own worked
examples, and at temperature 0.9 the model reproduced both. One shared voice
contract, NotificationVoice, now states each ban and carries no example at
all. The fallback strings carried the same two tells and are rewritten.

Error messages. A domain guard firing is an ordinary event, but its message
is the sentence a developer wrote, and on 2026-08-05 a tester read "Days can
only be set when frequency quantity is 1." verbatim, in English, inside a
pt-BR interface. ErrorCopy now maps every error code to user facing copy in
both languages, and one result filter swaps the message at the response
boundary. DomainErrors stays developer facing, as the ticket requires. The
catalog is total over ErrorCodes, ErrorMessages and DomainErrors, and a
reflection test fails the build when a constant arrives without copy, so the
raw message fallback is unreachable. ErrorMessages now takes its English from
the same catalog rather than repeating 145 sentences in a second file.

AppError and Result carry the arguments behind a {0} placeholder so the
localized template can be formatted with the same values, which the eagerly
formatted message alone no longer exposed.

Also removed: five pay gate strings ending in "Upgrade to unlock!", a banned
word and an exclamation on a sell; inlined copies of error sentences in three
chat tools; and a hardcoded 500 envelope message.

Tests: 6980 passing, 0 failing. Line coverage 86.40%.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@pullfrog

pullfrog Bot commented Sep 18, 2026 •

Copy link
Copy Markdown

Run failed. View the logs →

Pullfrog  | Rerun failed job ➔ | View workflow run | via Pullfrog | Using GPT Sol | 𝕏

… map

Two red checks on PR 532 at c50f052.

Dash Ban. The three new voice tests detected a banned dash by embedding one,
and the gate scans a test file like any other. The two constants are now built
from their code points, ((char)0x2014) and ((char)0x2013), so no literal
character sits in the source. The escape form these files were written with
did not survive: the lefthook pre-commit step runs dotnet format, which
normalized "—" back into the raw character before the commit landed. A
code point arithmetic expression has no literal for a formatter to normalize.

The assertions are unchanged. Verified by injecting a real em dash into one
ErrorCopy entry and one EmailCopy string: EveryEntryObeysTheErrorVoice and
NoEmailStringCarriesADashCharacter both went red, then green again once the
injection was reverted.

drift. architecture.json and architecture.html were stale against the new
test classes and the changed file counts. Regenerated with node
tools/arch-map.mjs and committed here, in the change that requires them. The
generator is deterministic: two consecutive runs are byte identical.

tools/dash-baseline.json is untouched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@pullfrog

pullfrog Bot commented Sep 18, 2026

Copy link
Copy Markdown

Run failed. View the logs →

Pullfrog  | Rerun failed job ➔ | View workflow run | via Pullfrog | Using GPT Sol | 𝕏

1 similar comment
@pullfrog

pullfrog Bot commented Sep 18, 2026

Copy link
Copy Markdown

Run failed. View the logs →

Pullfrog  | Rerun failed job ➔ | View workflow run | via Pullfrog | Using GPT Sol | 𝕏

@thomasluizon

Copy link
Copy Markdown
Owner Author

Independent review at head dd862844: REQUEST CHANGES. This pull request had never been reviewed by anyone. Four findings are blocking: two ship a user-visible regression to production and one means the headline feature does nothing on Android.

Codex and Pullfrog share one exhausted OpenAI allowance until 2026-09-22, so a separate Claude agent reviewed this against the producing code in orbit-api and the consuming code in orbit-ui-mobile. Stating the substitution rather than hiding it: this is not a Pullfrog verdict, and protected main still needs pullfrog-approval before this can merge.

Copy findings

C1, CRITICAL: the deletion email says deletion is permanent, and the product deactivates

EmailCopy.cs now reads "You asked to delete your Orbit account. This cannot be undone. Your habits, history, conversations and settings are removed for good.", and the pt-BR twin says "Isso não pode ser desfeito. ... são apagados de vez."

The producing rule: ConfirmAccountDeletionCommand.cs:30-37 calls user.Deactivate(scheduledDate), where the date is the EARLIER of plan expiry plus seven days and now + AppConstants.MaxDeletionGraceDays, which is 30 at AppConstants.cs:47. User.cs:443-458 carries Deactivate and CancelDeactivation. Signing in cancels it, at VerifyCodeCommand.cs:105-110, GoogleAuthCommand.cs:140-144 and OAuthController.cs:233-243.

Somebody confirms deletion, reads "this cannot be undone", signs in three days later out of habit, and the account comes back with every habit intact. The mirror case is worse: somebody who wanted a real wipe believes it already happened while the row lives for up to 30 days.

This is the exact defect #71's acceptance criterion carries and that pull request 954 already corrected once in the app's own copy. The email must not reintroduce it. Say the grace window and the sign-in cancel, and read the number from AppConstants.MaxDeletionGraceDays rather than typing it.

C2, HIGH: the new INVALID_CODE_ATTEMPTS_REMAINING code breaks the step-up screen on both shipped clients

ErrorMessages.cs moves InvalidDeletionCode and InvalidApiKeyCreationCode off ErrorCodes.InvalidVerificationCode onto a new code, raised at EmailChallengeService.cs:137-142. Both shipped clients branch on the old value and have no branch for the new one:

  • apps/mobile/app/step-up.tsx:197: if (errorCode !== 'INVALID_VERIFICATION_CODE') { setFieldError(t('stepUp.genericError')); setPhase('challenge'); return }
  • apps/web/app/step-up/step-up-screen.tsx:162-177: the same shape, falling to t('genericError').
  • packages/shared/src/utils/error-utils.ts:398 and auth-login.ts:11 map only INVALID_VERIFICATION_CODE, and the new code is absent from ERROR_CODE_TO_KEY at error-utils.ts:310-382.

The message FORMAT is load-bearing too and it also changed. packages/shared/src/utils/step-up.ts:117-123 parses the count with /remaining attempts:\s*(\d+)\s*$/i. The old string was "Invalid code. Remaining attempts: {0}"; the new one is "That code is not correct. You have {0} tries left." No match, so remaining is null.

So a wrong deletion code shows a generic error instead of "wrong code, 2 tries left", the wrong phase never renders, and the client-side exhaustion counter at step-up.tsx:202-208 never advances. That breaks users on live 1.3.31 the day this deploys, which is what the root CLAUDE.md append-only rule and .claude/rules/core.md rule 5 exist to prevent.

Either keep the existing code and add the attempts copy under it, or file the paired UI ticket that this one blocks and hold the change behind it. Do not ship the rename alone.

C3, HIGH: the localized copy never reaches the Android app at all

LocalizedErrorResultFilter.cs takes the language from the request header only, passing Request.Headers.AcceptLanguage.ToString() into LocaleHelper.IsPortuguese at LocaleHelper.cs:26-32.

Mobile sends no Accept-Language. apps/mobile/lib/api-client.ts:81-104 builds its header set from buildClientTimeZoneHeaders(), which returns only X-Orbit-Time-Zone (packages/shared/src/utils/client-context.ts:14-19), plus buildAppVersionHeaders(), Authorization, Content-Type and Idempotency-Key. So IsPortuguese("") is false and every pt-BR Android user gets English for all 222 codes, which is the exact defect ErrorCopy.cs's own header comment says this work exists to fix.

Web Server Actions send none either (apps/web/lib/server-fetch.ts:34-39), and apps/web/CLAUDE.md routes every mutation through a Server Action, so every create, update and delete error on web is English. Only the BFF catch-all forwards it, through apps/web/app/api/_utils/forwarded-client-context.ts:11-14. Even there the language is wrong for anyone who set the in-app locale, because apps/web/i18n/request.ts:9-14 prefers the i18n_locale cookie over the header.

It also contradicts the rest of the product, which keys off the stored user.Language in ResendEmailService and ProactiveCheckinSchedulerService.cs:146.

Resolve the locale from the authenticated user's Language, with the header as the anonymous-request fallback, and cover it with an integration test that a pt-BR user sending no Accept-Language gets pt-BR copy.

C4, HIGH: the slip-alert copy asserts a time pattern the scheduler contradicts

AiSlipAlertMessageService.cs now titles the push "Your usual time for {habit}" and bodies it "This is around the time it usually comes up."

SlipAlertSchedulerService.cs:136,168-171 computes CalculateAlertTime as Math.Clamp(peakHour.Value - 2, 8, 22), so it sends two hours before the peak. Even with a time pattern, the send time is by construction not the usual time. And SlipPattern.PeakHour is int? (SlipPattern.cs:6); when it is null the doc at SlipAlertSchedulerService.cs:165-167 says the push goes at 08:00, and the prompt the same service builds says literally "They tend to slip on {dayOfWeek}s (no specific time pattern)."

A Friday-only smoker with no hourly pattern gets an 08:00 lock-screen push titled "Your usual time for Smoking". Cover it with a fallback test at peakHour: null asserting the copy makes no time claim.

C5, MEDIUM: the check-in fallback says "A few habits" when one is the common case, and promises Astra will log the rest

AiProactiveCheckinMessageService.GenerateFallback now reads "A few habits are still open today. Pick the easiest one and Astra records the rest." ProactiveCheckinSchedulerService.cs:143-148 sends whenever offTrackTitles.Count != 0, so one open habit is common and the plural is then false. Second defect in the same sentence: nothing auto-logs a habit. LogHabitTool records only what the person names, so "Astra records the rest" promises behaviour that does not exist.

C6, MEDIUM: the new RATE_LIMITED copy is unreachable and the live 429 is still hardcoded English

ErrorMessages.TooManyRequests has zero call sites in src/; the only other reference anywhere is a test mock at OAuthControllerTests.cs:215. The real 429 body is written at DistributedRateLimitAttribute.cs:90-100 and :304-311 as an anonymous object with error = "Too many requests", and the filter only matches ObjectResult { Value: ErrorResponse }, so it cannot rewrite it. That file is not in this diff. Build the body from ErrorMessages.TooManyRequests.ToErrorBody().

C7, MEDIUM: the attempts copy is ungrammatical at exactly the values the code produces

EmailChallengeService.cs:79 computes Math.Max(0, AppConstants.MaxVerificationAttempts - attempts) with MaxVerificationAttempts = 3 at AppConstants.cs:43, so the only values anyone sees are 2, 1 and 0. The string renders "You have 1 tries left.", "You have 0 tries left." and "Você tem mais 1 tentativas." The replaced copy had no agreement to get wrong. At 0 the sentence also offers no next action, against the catalog's own stated rule.

C8, MEDIUM: the sync copy promises client behaviour neither client implements

"This device was offline too long to catch up. Orbit will load everything again." and "Orbit syncs {0} changes at a time. It will send the rest next." A grep for SYNC_WINDOW_EXCEEDED|TOO_MANY_MUTATIONS across all of orbit-ui-mobile returns no matches, and the sync endpoints at packages/shared/src/api/endpoints.ts:174-176 have no app caller. Nothing re-loads and nothing chunks and resends.

C9, LOW: CHALLENGE_CLOSED English is broken

"This challenge is no longer taking part." A challenge does not take part; participants do. The pt-BR beside it is correct and the replaced English was correct.

C10, LOW: the recap promise is conditional on push being on

"That month has not finished yet. Its recap arrives once it does." PeriodCloseNotificationService.cs:51-56 selects only users with a row in PushSubscriptions. With notifications off, nothing arrives.

Code findings

D1, HIGH: "the raw-message fallback stays unreachable" is false

ValidationExceptionHandler.cs:22-42 writes { type, status, requestId, errors } directly to the response as an IExceptionHandler. It never produces an ObjectResult, so the filter never sees it. There are 139 validator files and 285 WithMessage/AbstractValidator occurrences under src/Orbit.Application/**/Validators. Concrete instance: VerifyCodeCommandValidator.cs:19 ships "Code must be a 6-digit number" verbatim to a pt-BR user. Either route the validation handler through ErrorCopy too, or narrow the doc comment to what the code supports.

D2, HIGH: the rewrite silently disables the clients' message-text to i18n-key mapping

packages/shared/src/utils/error-utils.ts:252-282 and :300-305 map errors to i18n keys by matching the backend's ENGLISH SENTENCE, and codes absent from ERROR_CODE_TO_KEY depend on that match entirely.

Create a habit with an empty title: TITLE_REQUIRED plus "Give this a title." The rule at error-utils.ts:300 needs both 'title' and 'required', so no match, and TITLE_REQUIRED is absent from ERROR_CODE_TO_KEY, so a generic toast replaces habits.form.titleRequired. Before this pull request the message was "Title is required." and it matched. The same break hits UNIT_REQUIRED, TARGET_VALUE_INVALID, TAG_NAME_REQUIRED, DUPLICATE_SCHEDULED_REMINDERS, MAX_SCHEDULED_REMINDERS, MAX_HABITS_PER_GOAL, MAX_TAGS_PER_HABIT and GENERAL_HABIT_IS_BAD. And for any client that does receive Portuguese, every one of these rules fails, including those whose English still matches.

The paired UI ticket adds the missing ERROR_CODE_TO_KEY entries and deletes the text-matching rules. File it and block this on it, per the cross-repo rule.

D3, MEDIUM: the one prompt that caused this bug is excluded from the guard written for it

NotificationVoiceTests.EveryPromptSentToAModel covers AiSummaryService.SystemPrompt but not BuildSummaryPrompt, which stays private static. That user prompt is precisely the one this pull request strips doubled hyphens from. Make it internal static and add it to both theory sources.

D4, LOW: the catalog does not fail at startup, as its comment claims

ErrorMessages is a static class of static readonly fields, so its initializer runs lazily on first access. A missing entry becomes a TypeInitializationException on a live request, not a startup failure. ErrorCopyTests catches it in CI, so the risk is low and the comment is still wrong about the mechanism.

D5, LOW: AppError structural equality quietly became reference-based for formatted errors

AppError.cs now stores the params array as Args, and the record's generated Equals compares IReadOnlyList<object?> by reference, so MaxTagsPerHabit.Format(5).Equals(MaxTagsPerHabit.Format(5)) returns false where it used to return true. The new AnErrorWithoutArgumentsStaysEqualToItself covers only the empty case, so the case that broke is untested. No current caller compares whole AppErrors, so this is latent.

D6, LOW: IsPortuguese on a raw header ignores q-values

LocaleHelper.cs:31 is language.StartsWith("pt", OrdinalIgnoreCase) applied to the whole header value, so "pt;q=0.1,en;q=0.9" resolves to Portuguese although English is preferred. Narrow, but it is now the sole language input for all 222 strings.

The totality test, and the vacuous-test sweep

The reflection test exists and is not vacuous. ErrorCopyTests.EveryDeclaredErrorCodeHasCopy reflects over typeof(ErrorCodes).GetFields(Public|Static).Where(f => f.IsLiteral && f.FieldType == typeof(string)) and asserts every value is a key of ErrorCopy.All. It goes red if a new public const string lands without copy. Two siblings do the same over ErrorMessages and DomainErrors. What it proves is TOTALITY over the three catalogs, never reachability, and D1 and C6 are two whole response paths the catalog never touches.

None of the new tests is strictly vacuous, but three are change-detectors rather than defect-detectors: the three NotificationVoice contract tests each assert that a constant contains string literals copied out of that same constant, so only editing NotificationVoice.Rules turns them red. One is fragile rather than weak: EveryLocalizedEmailDiffersBetweenTheTwoLanguages pairs two independent reflection passes by list index, so it holds only while StringsOf returns properties in a stable order and would compare mismatched pairs if a copy record gained a property.

Verified clean, with the evidence

  • Bare // comments: none. ^\+\s*//[^/] over the 5,577-line diff returns 0 matches; only /// XML doc was added, which orbit-api/CLAUDE.md:29 permits. This matters because ORBIT0001 does not run under the locally installed SDK.
  • DateTime.UtcNow without ORBIT0004: none. ^\+.*DateTime\.UtcNow returns 0 matches.
  • Em dash and en dash: none. ^\+.*[—–] returns 0 matches, and the three copy suites assert absence by code point rather than by embedding one. The pull request also removes the doubled hyphens from four prompts and two fallback bodies.
  • "Orbit" and "Astra" are never translated. The pt-BR strings use "o Orbit" and "a Astra", an article rather than a translation.
  • Locale completeness and placeholder symmetry are both complete. All 14 catalog groups and 222 entries were diffed against the 145 constants in ErrorCodes.cs plus the two this pull request adds; every {n} matches across both languages, and BothLanguagesTakeTheSamePlaceholders enforces it. The reverse hazard was checked too, a template with {0} raised with empty Args, and there is none: MaxDepthReached.Format(maxDepth) at MoveHabitParentCommand.cs:81 and CreateSubHabitCommand.cs:58 passes the limit rather than the current depth, so "Habits nest 5 levels deep" is correct.
  • No response field is removed or retyped. ErrorResponse pins [JsonPropertyName("error")] and [JsonPropertyName("errorCode")] and marks Args [JsonIgnore], so the wire shape is byte-identical. The contract break in C2 is the VALUE of errorCode, not the shape.
  • Naming, swallowed errors and dead code are all clean, and the pull request deletes some: EmailLayout.GradientHeader and the CanvasColor and GradientTopColor constants go with all six call sites and the two tests that covered them.

thomasluizon and others added 3 commits September 18, 2026 17:05
C1: the deletion email promised a permanent wipe while the product only
deactivates. The intro now states the grace window, read from
AppConstants.MaxDeletionGraceDays, and that signing in again cancels it.

C2: the copy pass had moved the wrong-code failure onto a new error code
both shipped clients have no branch for, which sends Orbit 1.3.31 and the
web step-up screen to a generic error. The code returns to
INVALID_VERIFICATION_CODE, and the count rides in a counted copy variant
that keeps the trailing token those clients parse.

C3: the reply language came from Accept-Language alone, which neither the
Android app nor the web Server Actions send, so every signed-in pt-BR
reader got English. RequestLanguageResolver reads the stored
User.Language first and keeps the header as the anonymous fallback. The
header parse now honours quality values.

C4: the slip alert claimed the usual time had arrived, while the
scheduler sends two hours early and at 08:00 when no peak hour exists.
The prompt states when the push lands and the fallback makes no time
claim without a peak hour.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
C5: the check-in fallback called one open habit "a few" and promised that
Astra records the rest. The scheduler sends as soon as one habit is off
track, and LogHabitTool records only the habit a person names.

C6: the 429 body was an anonymous object the localizing filter could not
match, so the RATE_LIMITED copy was unreachable. It is built from
ErrorMessages.TooManyRequests now, which also gives the 429 an error code
for the first time. Retry-After and the request id already ride in
headers.

C8: the sync copy promised a reload and a later send that no caller
performs. C9: a challenge does not take part, people do. C10: the recap
arrives only for a reader with a push subscription, so the copy points at
coming back instead.

D1 and D4: two doc comments claimed more than the code does. The filter
covers ErrorResponse bodies only, and a FluentValidation message never
reaches it. A missing catalog entry throws on the first request, not at
startup.

D3: BuildSummaryPrompt is internal and joins both voice theory sources,
so the guard written for its doubled-hyphen leak now reaches it.

D5: AppError compared Args by reference, so two errors formatted with the
same count came out unequal. Equality walks the arguments now.

The three voice tests that read NotificationVoice.Rules' own literals
back out of it now assert the bans over every prompt a model receives.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@pullfrog

pullfrog Bot commented Sep 18, 2026

Copy link
Copy Markdown

Run failed. View the logs →

Pullfrog  | Rerun failed job ➔ | View workflow run | via Pullfrog | Using GPT Sol | 𝕏

1 similar comment
@pullfrog

pullfrog Bot commented Sep 18, 2026

Copy link
Copy Markdown

Run failed. View the logs →

Pullfrog  | Rerun failed job ➔ | View workflow run | via Pullfrog | Using GPT Sol | 𝕏

thomasluizon and others added 3 commits September 24, 2026 13:14
* fix: let the executor own the MCP confirmation gate (#599)

Every MCP tool whose capability requires a confirmation or a step-up was
refused forever. The selective-auth middleware evaluated the policy a second
time, in front of the executor, and it never received the caller's
confirmation token. A client that stepped up and retried with a valid token
got the identical refusal, because the token never reached that evaluation.

Carrying the token into the middleware cannot fix it. A confirmation token is
single use and is bound to the pending operation's fingerprint, and the
middleware computes a different fingerprint from the raw MCP arguments than
the executor computes from the operation id and its snake_case argument
object. One token cannot satisfy two gates.

So the middleware now steps aside for a confirmation-gated capability, the
same way it already steps aside for execute_agent_operation_v2. Both reach
IAgentOperationExecutor, which evaluates access and confirmation together,
holds the token, and writes the audit row.

Two guard tests pin the invariants the deferral rests on: a confirmation
requirement always sits on a mutation, and every confirmation-gated MCP tool
reaches the executor through McpExecutorBridge.

Also corrected: the step-up message and the three tool parameter descriptions
named verify_step_up_agent_operation_v2 as the source of the confirmation
token. It does not return one. confirm_agent_operation_v2 does.

Removed AgentPolicyEvaluationContext.StepUpSatisfied. Nothing ever set it and
nothing ever read it; the real step-up state lives on
PendingAgentOperationState.StepUpSatisfiedAtUtc.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix: thread the confirmation token through the bulk habit tools (#599)

bulk_log_habits and bulk_skip_habits declared no confirmationToken
parameter and their shared helper hardcoded confirmationToken: null, so
their HabitsBulkWrite capability (FreshConfirmation) could never see a
token and both tools stayed permanently refused. Both now declare the
parameter and ExecuteBulkHabitOperationAsync forwards it.

Three new guards in ConfirmationGatedMcpToolsRouteThroughExecutorTests
pin what the old one missed. The scan now asserts it reaches exactly the
catalog's gated MCP tools, that each tool's forwarded operation id
resolves to a capability with the same ConfirmationRequirement, and that
each tool accepts and forwards a confirmation token. The source scan
walks subfolders and keys members by their declaration rather than by the
first invocation-shaped token in the chunk.

AgentOperationExecutor writes an AgentAuditLogs row before returning
UnknownOperation, restoring the trail the middleware used to leave.

McpConfirmationGateTests now drives the real AgentTools recovery methods
end to end and pins that all three refuse an API-key credential.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix: resolve the forwarded MCP confirmation token back to the tool parameter (#599)

The token guard whitelisted one spelling of null. It flagged a bridge call only
when the confirmation-token argument was absent or the literal `null`, so
`confirmationToken: default`, `null!`, `(string?)null` and `""` all passed while
the tool stayed refused forever. A longer literal list does not close that,
because the next spelling is not on the list either.

Assert the structural property instead: a gated tool declares a
`string? confirmationToken` parameter, and the expression it forwards into the
executor's token slot resolves back to that identifier, directly or through one
helper hop. Every other expression fails, whatever it spells.

The declaration check now reads the tool's parameter list rather than its whole
body, so a local named `confirmationToken` no longer satisfies it.

Also add `forwarded.Id == tool.Capability.Id` to the operation-id guard. It
compared only the confirmation requirement, so a gated tool could forward
another gated capability's operation id, keep confirmation firing, and have the
executor enforce the wrong scope.

The file's doc comment said four invariants and listed four; the file holds
five. Name the source-scan invariant the other three rest on.

Six mutations, each red on its own, real tree green at every step:
`confirmationToken: default`, `null!`, `(string?)null`, `""`, a gated tool
forwarding a different local, and `delete_tag` forwarding `delete_goal`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix: strip only a real named-argument prefix in the MCP guard parser (#599)

ArgumentAt treated any colon as a named-argument separator, so a conditional
expression in the token slot or the operation-id slot collapsed to its
else-branch. `ids.Count > 0 ? null : confirmationToken` read as
`confirmationToken` and `tagId.Length > 0 ? "delete_goal" : "delete_tag"` read
as `"delete_tag"`, and both guards stayed green over the restored defect.

Match `^\w+\s*:(?!:)` instead, so only a leading `name:` prefix is stripped and
every other expression reaches the resolver whole. The anchor keeps a colon
inside a string literal and a `::` qualifier from matching.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix: separate an omitted caller argument from a non-parameter expression (#599)

ResolveExpression returned the expression itself both when it was not a callee
parameter and when the caller supplied nothing at that index. The second case
handed back the helper's own parameter name, which then compared equal to
`confirmationToken` and passed. Giving the helper an optional
`string? confirmationToken = null` and calling it without that argument restored
the #599 defect with the guard green.

Return the `<omitted>` sentinel for the second case instead. It can never equal
the parameter name, and it resolves to no capability in the operation-id slot,
so both guards fail closed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix: read a gated tool's direct and helper bridge calls together (#599)

CollectBridgeCalls returned early as soon as the tool body held one bridge
call, so a correct call in a dead branch hid every helper call in the same
tool. A decoy `if (tagId.Length == 0)` block forwarding `confirmationToken`
plus a same-file helper forwarding `null` restored the #599 defect with the
guard green.

Collect both sets instead. Every bridge call a gated tool can reach, directly
or through one helper hop, now has to forward the parameter.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix: refuse a gated tool that assigns to its own confirmationToken (#599)

The rule compared the forwarded expression's text only, so
`confirmationToken = null;` above the bridge call restored the #599 defect with
the forwarded identifier unchanged and the guard green.

Add an offender when the tool body, or a same-file helper it reaches, assigns
to the parameter. MemberSource now carries its statements with the parameter
list excluded, so the declaration's own `string? confirmationToken = null`
default does not match. The pattern `\bconfirmationToken\s*=[^=>]` leaves `==`,
`!=`, `>=`, `<=`, `??=` and a lambda arrow alone.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs: say what the MCP token resolver models, not more (#599)

Invariant 5 claimed "any other expression in the token slot refuses the tool
forever" and that the tool forwards the identifier "directly or through a
helper it calls". A conditional refuses the tool on some paths only, and the
resolver models exactly one same-file helper hop matched by argument position.

Replace both sentences with what the scan actually does, and state its limits:
two or more hops read as no bridge call and fail the routing guard, and
aliasing, reflection and an interface call are outside the model.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix: catch a compound assignment to confirmationToken (#599)

The assignment rule was anchored on `\s*=`, and `\s*` cannot cross the `??`
of `??=` or the `+` of `+=`. So `confirmationToken ??= tagId;` above the
bridge call compiled green with all five guards green, and a later change
that writes `confirmationToken ??= await ResolveStoredTokenAsync(...)` would
hand the executor a token the MCP caller never sent: HasFreshConfirmation is
then satisfied from state the caller does not control, while the middleware
has already stepped aside.

Admit the two compound operators a `string?` can carry. `==` still fails on
the second `=`, `!=`, `>=` and `<=` still fail because those characters break
`\s*` and are not in the alternation, `=>` still fails on the trailing
character class, and a bare `??` with no `=` still fails.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix: read every same-named helper an MCP tool could be calling (#599)

The member index was a dictionary keyed on the bare name and built with
group.First(), so two same-named members collapsed to whichever was declared
first. The scan then analysed a method the tool never calls. It broke both
ways: a decoy overload declared after the real helper and forwarding null was
green, and a gated tool calling a correct overload reddened when an unrelated
same-named member happened to be declared first.

Hold every member under its name and, at each call site, read every candidate
whose parameter count can admit that many arguments. One candidate resolves
the call; more than one is ambiguous, so all of them are read and any bad one
reddens the tool. That fails closed instead of guessing, and it costs no false
red in the case above, where only the real helper carries a bridge call. A
name that admits no candidate stays unresolved, which hides nothing: the
bridge call inside it is not collected either, so the routing guard reddens
the tool.

Arity, not the parameter type, is what decides a candidate. The scan is a text
scan, so it cannot type-check an argument.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs: say what invariant 5 now refuses and what it still cannot see (#599)

The general claim "refuses a tool that assigns to the parameter" is now exact:
`=`, `??=` and `+=`. The helper hop no longer claims a "matching argument
position", because the resolver pairs by position alone and strips a
named-argument prefix without reading the name, so arguments named out of the
declared order are read wrong. The comment said nothing about overloads, so
say that an ambiguous name is read as every candidate and reddens the tool.

Close with the honest limit: a text scan is defeatable by an author who sets
out to defeat it, and what the guard closes is every shape an ordinary
refactor produces.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* Start calendar timezone fix

* Fix calendar event timezone projection

* Fix projected calendar end times

* Fix calendar end time during DST fallback

* Fix calendar end time minute precision

* Fix projected calendar recurrence rule

* Guard calendar recurrence across timezone shifts

* Project calendar recurrence weekdays

* Keep calendar recurrence unchanged

* Omit unrepresentable recurring calendar events

* Gate projected calendar recurrence on the whole fetch window

Closes the six review findings on pull request 521.

1. The weekday gate now walks every expanded occurrence Google returned for a
   recurring master, not the one sampled instance. The fetcher collects each
   instance start with the source calendar's own offset into a JsonIgnore'd
   ExpandedOccurrences list, so a Lisbon BYDAY=TH series that is stable in
   January and shifts to Wednesday in July is refused. The empty list still
   falls back to the sampled instance for an all-day series and for a
   suggestion row read back from the database.
2. The legacy title plus date plus time key only excludes a suggestion when
   exactly one candidate carries it. Inside a fall-back repeated hour two
   events project to the same local time, so the key proves nothing and both
   stay. This mirrors the group.Count() == 1 guard the auto sync reconciler
   already applies to the same key.
3. An omitted end time now logs its reason at Debug with the event id and the
   user id. EndUtc already ships the real duration, so the client keeps it.
4. The refusal here and the clamp in HabitScheduleService.IsMonthlyMatch are
   reconciled in a doc comment: Orbit clamps a rule it owns and refuses a rule
   it imports and cannot re-express without inventing a weekday.
5. Both new call sites pass the logger and the user id to FindTimeZone, which
   also catches InvalidTimeZoneException so a corrupt zone stops escaping as a
   500. User.SetTimeZone trims at the boundary and rejects a blank id.
6. Asia/Kathmandu at plus 05:45 and Pacific/Chatham at plus 12:45 cover the
   sub hour offset gap.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Prove calendar recurrence stability from both zones

Round 6 gated a BYDAY series on the expanded instances Google returned, and
GoogleCalendarApi only asks for sixty days, so a series whose transition falls
outside that window was admitted and a stored suggestion row never carried the
instances at all.

The gate now reads the source calendar's own timezone from the recurring master
the fetcher already fetches, and walks a year of dates at the occurrence's source
wall clock through both zones' rules. A series it cannot prove stable is withheld
from both feeds. StoredCalendarEventJson carries the source zone beside the stored
suggestion, so the suggestion feed judges a row on the same evidence the events
feed had.

Also in scope: the auto-sync reconciler projects a fetched event into the account
timezone before matching a legacy habit, the end-time omission logs whatever EndUtc
holds, and an all-day event no longer reports Google's exclusive end date as an
end instant.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Keep calendar series a spring forward gap cannot move

A probe date whose source wall clock a spring forward gap removes is a date
the series does not fire on, not evidence the series moves. The walk ended
there with "unproved" and the gate withheld the whole series, so an account
reading a calendar kept in its own zone lost an hour wide band in thirteen
DST zones even though the projection is the identity.

HasUnrepresentableRecurrenceAfterProjection now answers false as soon as
TimeZoneInfo.HasSameRules holds, and the walk skips a wall clock its own zone
removes instead of ending. RFC 5545 section 3.3.5 does define the missing
case, but probing that normalized instant changes no decision for any of the
14,752 zone pairs that could distinguish it, and asserting an expansion
Google does not document would let one guessed date withhold a year.

Also pins what earlier rounds argued: auto-sync refusing to store or notify a
withheld series, JsonIgnore keeping SourceTimeZone off the response body, the
repeated hour needing both of its instants, the probe reaching a full year,
and an all day event logging no dropped end time. A stored SourceTimeZone key
holding a number now reads as no source zone rather than failing the request.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Fix recurring calendar timezone proof and legacy refresh

* Avoid refreshing legacy calendar rows with duplicate event IDs

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Bumps FluentAssertions from 8.10.0 to 8.11.0
Bumps Google.Apis.AndroidPublisher.v3 from 1.75.0.4246 to 1.76.0.4277
Bumps Hangfire.AspNetCore from 1.8.24 to 1.8.25
Bumps Hangfire.Core from 1.8.24 to 1.8.25
Bumps Microsoft.AspNetCore.Authentication.JwtBearer from 10.0.11 to 10.0.12
Bumps Microsoft.AspNetCore.OpenApi from 10.0.11 to 10.0.12
Bumps Microsoft.EntityFrameworkCore from 10.0.11 to 10.0.12
Bumps Microsoft.EntityFrameworkCore.Design from 10.0.11 to 10.0.12
Bumps Microsoft.EntityFrameworkCore.InMemory from 10.0.11 to 10.0.12
Bumps Microsoft.EntityFrameworkCore.Relational from 10.0.11 to 10.0.12
Bumps Microsoft.EntityFrameworkCore.Sqlite from 10.0.11 to 10.0.12
Bumps Microsoft.Extensions.ApiDescription.Server from 10.0.11 to 10.0.12
Bumps Microsoft.Extensions.Caching.Abstractions from 10.0.11 to 10.0.12
Bumps Microsoft.Extensions.Caching.Memory from 10.0.11 to 10.0.12
Bumps Microsoft.Extensions.Caching.StackExchangeRedis from 10.0.11 to 10.0.12
Bumps Microsoft.Extensions.Http from 10.0.11 to 10.0.12
Bumps Microsoft.IdentityModel.JsonWebTokens from 8.22.0 to 8.23.0
Bumps Microsoft.NET.Test.Sdk from 18.9.0 to 18.10.1
Bumps OpenAI from 2.13.0 to 2.14.0
Bumps PostHog from 2.14.0 to 2.15.7
Bumps PostHog.AspNetCore from 2.9.0 to 2.9.7
Bumps Scalar.AspNetCore from 2.17.1 to 2.17.9
Bumps Sentry.AspNetCore from 6.9.0 to 6.11.1
Bumps Stripe.net from 52.3.0 to 52.4.2

---
updated-dependencies:
- dependency-name: Google.Apis.AndroidPublisher.v3
  dependency-version: 1.76.0.4277
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: nuget-minor-patch
- dependency-name: Hangfire.AspNetCore
  dependency-version: 1.8.25
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Hangfire.Core
  dependency-version: 1.8.25
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Hangfire.Core
  dependency-version: 1.8.25
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.AspNetCore.Authentication.JwtBearer
  dependency-version: 10.0.12
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.AspNetCore.OpenApi
  dependency-version: 10.0.12
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.EntityFrameworkCore
  dependency-version: 10.0.12
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.EntityFrameworkCore.Design
  dependency-version: 10.0.12
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.EntityFrameworkCore.Relational
  dependency-version: 10.0.12
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.Extensions.ApiDescription.Server
  dependency-version: 10.0.12
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.Extensions.Caching.Abstractions
  dependency-version: 10.0.12
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.Extensions.Caching.StackExchangeRedis
  dependency-version: 10.0.12
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.Extensions.Http
  dependency-version: 10.0.12
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.IdentityModel.JsonWebTokens
  dependency-version: 8.23.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: nuget-minor-patch
- dependency-name: OpenAI
  dependency-version: 2.14.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: nuget-minor-patch
- dependency-name: PostHog
  dependency-version: 2.15.7
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: nuget-minor-patch
- dependency-name: PostHog.AspNetCore
  dependency-version: 2.9.7
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Scalar.AspNetCore
  dependency-version: 2.17.9
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Sentry.AspNetCore
  dependency-version: 6.11.1
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: nuget-minor-patch
- dependency-name: Stripe.net
  dependency-version: 52.4.2
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.NET.Test.Sdk
  dependency-version: 18.10.1
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: nuget-minor-patch
- dependency-name: FluentAssertions
  dependency-version: 8.11.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.EntityFrameworkCore.InMemory
  dependency-version: 10.0.12
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.Extensions.Caching.Memory
  dependency-version: 10.0.12
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.NET.Test.Sdk
  dependency-version: 18.10.1
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: nuget-minor-patch
- dependency-name: FluentAssertions
  dependency-version: 8.11.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.NET.Test.Sdk
  dependency-version: 18.10.1
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: nuget-minor-patch
- dependency-name: FluentAssertions
  dependency-version: 8.11.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.EntityFrameworkCore.InMemory
  dependency-version: 10.0.12
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.EntityFrameworkCore.Sqlite
  dependency-version: 10.0.12
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: nuget-minor-patch
- dependency-name: Microsoft.NET.Test.Sdk
  dependency-version: 18.10.1
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: nuget-minor-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
thomasluizon and others added 6 commits September 24, 2026 20:07
* Start #529 API key step up

* Require API key management step up

* chore: regenerate the architecture map

The step up change moved the API key endpoints' shape, so `architecture.json`
and `architecture.html` no longer matched the tree and the `drift` check
failed on `git diff --exit-code`. Regenerated with `node tools/arch-map.mjs`,
which reports 45 entities and 0 untested feature folders.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Give API key management one authorization concept with two doors

Listing, creating and revoking API keys now read one grant. Two doors open
it, and both end in a six-digit code emailed to the account owner: the HTTP
challenge, and a verified agent step-up.

Creating key material spends the grant. Listing and revoking only read it,
so one emailed code authorizes a management session instead of exactly one
revoke. A revoke no longer locks the person out of the key list.

get_api_keys gets a door. RequireApiKeyCreationStepUp raises the read
capability to a step-up inside the executor, so the read opens a pending
operation the caller can step up against, and the MCP read routes through
McpExecutorBridge like the mutation does.

AppConfigService now parses a stored row strictly and throws by name when a
row is malformed, so a typo cannot leave the gate off while the read-back
reports it on.

Both controller actions declare 403 and 428, and openapi.json carries them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Cover API key step up through merged MCP gate

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Open ticket 513 implementation review

* Expose habit emoji across MCP tools (#513)
* Start #659 workflow concurrency change

* Group core PR workflows by pull request for #659

* Group remaining PR workflows by pull request for #659
* Start #657 session family fix

* Revoke the full auth session after logout (#657)

* Explain temporary legacy token allowance (#657)

* Handle duplicate refresh history race (#657)
# Conflicts:
#	architecture.html
#	architecture.json
#	src/Orbit.Application/Common/ErrorMessages.cs
* Start #588 bulk interval weeks fix

* Fix #588 bulk habit interval weeks mapping
pullfrog[bot]
pullfrog Bot previously approved these changes Sep 25, 2026

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes I reviewed merge commit 642a50ef added after 63f77942 and checked it against the complete PR context.

  • Merged current main: Integrated the upstream API key management authorization changes and regenerated architecture artifacts.
  • Aligned bilingual challenge copy: Broadened API_KEY_CREATION_CHALLENGE_REQUIRED from creation-only wording to API key management wording, which now accurately covers listing, creating, and revoking keys.
  • Verified the error catalog: The focused ErrorCopyTests run passed all 677 generated cases.

Pullfrog  | View workflow run | Using GPT Sol | 𝕏

thomasluizon and others added 4 commits September 25, 2026 02:31
* Add crisis guidance to Astra static prompt for #319

* Add curated crisis detection and regression cases for #319

* Wire crisis guard across chat delivery and metrics for #319

* Refine crisis delivery and cover fallback regression for #319

* Return static crisis support when AI chat fails for #319

* Keep crisis FAQ regression isolated for #319

* Fix crisis replies to bypass AI quota and provider
* Start #571 freeze source work

* Record streak freeze origin for #571

* Require explicit origin for new streak freezes

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes I reviewed the two commits added after the prior Pullfrog review at 642a50ef and checked them against the complete PR context.

  • Merged current main: Integrated the latest base changes and regenerated the architecture artifacts without introducing feature-specific behavioral drift.
  • Refactored locale weighting: Extracted Accept-Language quality parsing while preserving priority and header-order semantics, with focused coverage for weighted, refused, invalid, and parameterized entries.
  • Simplified notification fallbacks: Replaced nested locale and state conditionals with tuple switch expressions and added exact coverage for the previously missing slip-alert combinations.
  • Verified the delta: Focused runs passed 33 application tests and 22 infrastructure tests.

Pullfrog  | View workflow run | Using GPT Sol | 𝕏

thomasluizon and others added 10 commits September 25, 2026 04:11
* Start #628 sub habit title validation fix

* Use sub habit title messages in standalone validation (#628)
)

A signed-out person who set a weekly repeat interval during onboarding lost
it at sign-up. ApplyHabitInput carried no interval field at all, so the apply
path dropped the value between the screen and the record.

Add an optional IntervalWeeks to ApplyHabitInput, additive only, and pass it
to Habit.Create alongside the other schedule options. No existing field
changes name, shape or nullability, so a client that sends nothing keeps the
behaviour it has today: IntervalWeeks stays null and the habit repeats every
week.

Validate the new field with the shared SharedHabitRules.AddIntervalWeeksRules
range, the same 1 to 52 bound every other create path uses.

Regenerate openapi.json and architecture.json/html for the new field and for
the test class that now touches HabitScheduleService.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* Start #591 recurrence time zone work

* Expose recurring calendar event time zone for #591

* Restore recurrence time zone in legacy suggestions for #591
* Start calendar BYDAY projection (#569)

* Project uniform calendar BYDAY recurrences (#569)

* Cover alternate week recurrence projection (#569)

* Keep shifted calendar rules importable by installed clients
* chore: start ticket 324 review

* fix: retry concurrent Google sign-in updates (#324)
* Start fix for ticket 671 confirmation race

* Claim agent confirmation once under concurrent saves

* chore: regenerate the architecture map for the confirmation claim migration (#671)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* chore: start ticket 391

* fix: expose latest habit completion date in profile

* fix: exclude bad habit slips from last completion date

* fix: pass the freeze origin in the profile freeze test after #540 (#391)

The merge with main brought StreakFreeze.Create's required origin
parameter (#540). The test now asserts that a freeze without a
completion leaves lastCompletionDate null for both origins.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: preserve last completion across habit cleanup (#391)

* Preserve descendant completions during sync cleanup

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@thomasluizon
thomasluizon changed the base branch from main to redesign/main September 25, 2026 16:00
@thomasluizon

Copy link
Copy Markdown
Owner Author

@pullfrog review

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Important

The latest merge integration needs a workflow correction before merge. Two workflows cannot load, leaving the current PR check set incomplete.

Reviewed changes

I reviewed the commits added after the prior Pullfrog review at ddd45f23, then checked their integration against the complete PR diff.

  • Branch integration: Merged current main and redesign/main, including their auth concurrency, recap, calendar projection, profile completion, agent confirmation, onboarding, streak, and support-intent changes.
  • Error catalog coverage: Added localized recap, session, validation, and domain errors while retaining totality checks for public error codes.
  • Merged test adaptations: Kept copy-specific assertions aligned across auth, calendar, gamification, onboarding, and profile tests.
  • Generated architecture artifacts: Refreshed the JSON and HTML architecture views for the merged tree.
  • Workflow triggers: Added redesign/main to the affected workflow triggers, with the merge-resolution defect called out inline.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using GPT Sol | 𝕏

Comment thread .github/workflows/dependency-review.yml Outdated

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Important

The current head still contains the duplicate top-level concurrency mappings already identified in the unresolved review thread. .github/workflows/dependency-review.yml and .github/workflows/sonarcloud.yml do not load into the current PR check set, so that workflow blocker remains; I found no additional high-severity application defect.

Reviewed changes I reviewed the complete PR at d0573474, including the latest base integrations and both producing and consuming sides of the localized-error contract.

  • Transactional email redesign: Rebuilt seven bilingual email types and the shared layout around the current visual and voice rules.
  • Localized API errors: Added a total bilingual error catalog, response-boundary localization, placeholder propagation, and stored-language resolution while preserving wire shape and established error-code values.
  • AI notification copy: Centralized the notification voice contract and aligned generated and fallback proactive, slip-alert, and summary copy.
  • Cross-repository compatibility: Confirmed orbit-ui-mobile PR #1026 is merged into its matching redesign/main branch and resolves the rewritten form errors by stable code rather than English-message matching.
  • Verification: Focused localization and distributed-rate-limit tests exited successfully; all current required checks pass except Pullfrog approval, while Dependency Review and the SonarCloud workflow are absent because of the duplicate workflow keys.

Pullfrog  | Fix it ➔ | View workflow run | Using GPT Sol | 𝕏

…reated

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes I reviewed commit b44a829d, added after the prior Pullfrog review at d0573474, and checked it against the complete PR context.

  • Restored workflow validity: Removed the duplicate top-level concurrency mappings from the Dependency Review and SonarCloud workflows, returning both files to their base versions.
  • Confirmed workflow admission: Verified that each workflow now contains one concurrency mapping; Dependency Review passed and SonarCloud started on the current head.

Pullfrog  | View workflow run | Using GPT Sol | 𝕏

@sonarqubecloud

Copy link
Copy Markdown

@thomasluizon
thomasluizon merged commit 23396af into redesign/main Sep 25, 2026
23 checks passed
@thomasluizon
thomasluizon deleted the feature/ticket-75-emails branch September 25, 2026 16:56
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