Repository navigation
Promote the API redesign with production compatibility checks - #723
Conversation
There was a problem hiding this comment.
Caution
Initial approval previews are not bound to their stored execution state, allowing a bulk delete to include habits absent from the displayed preview.
Reviewed changes This runtime-focused review examined the redesign promotion's production compatibility, approval flow, authorization boundaries, and migration/release safeguards.
- Chat approval and cards: Every chat write gains an approval hold, with editable previews, revision/refresh routes, optional response cards, follow-ups, and opt-in tool-step events.
- Habit and account behavior: Skip undo adds guarded persistence, while calendar imports, clock preferences, notification paging, and API-key metadata extend existing operations.
- Client compatibility: Localized errors preserve calendar reconnect detection, and historical color values remain accepted while profile reads use the granted accent.
- Persistence and delivery: Four additive migrations and compatibility proofs accompany the promotion; production health verification now requires the released commit SHA.
- Presentation and tooling: OAuth assets, email templates, notification wording, scheduler reads, and workflow branch coverage are also updated.
Verification: all 3,306 Infrastructure unit tests passed. A temporary regression test reproduced the initial-preview gap and was then removed; the working tree is clean. This was not an exhaustive line-by-line audit of every generated and test-only hunk in the promotion.
gpt-6.1-sol | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes Reviewed 9b7ffe55, the sole commit since the prior Pullfrog review, and verified the resolved finding against its discussion and current approval flow.
- Persisted initial preview bindings: Bound the displayed preview to stored pending-operation state before returning the approval card, using the existing guarded revision write.
- Added approval regression coverage: Verified that expanded target sets return
stale_previewwithout executing, while unchanged targets and legacy approvals remain executable. - Updated executor fixtures: Supplied the store dependency across chat, MCP, executor, and bulk-create tests.
Verification: all 3,309 Infrastructure unit tests and 26 focused Application unit tests passed, with no failures or skips. This was an incremental re-review, not a fresh exhaustive audit of the entire redesign promotion. The observed SonarCloud failure and remaining pending CI checks are separate merge gates.
gpt-6.1-sol | 𝕏
SonarCloud failed the quality gate on two reliability findings in code this pull request brings to main: the bulk skip conflict check read a flag only a lambda wrote (S2583), and the undo skip check loaded Tags and Goals in one query (S8733). The transaction now returns whether every habit refreshed, and the undo skip check splits its query. A new case covers the concurrent change conflict; it failed with the refresh outcome forced to true and passes with the fix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes Reviewed 00f9b823, the sole commit since the prior Pullfrog review, including the affected transaction, repository, and undo-validation paths.
- Returned the refresh outcome: Replaced the captured bulk-skip flag with the transaction's boolean result, preserving conflict handling before mutations or saves.
- Added conflict regression coverage: Verified failed refreshes return
ConcurrentUpdateConflictwithout saving or advancing the habit, and updated transaction mocks to the generic overload. - Made split loading explicit: Added
AsSplitQueryto undo-preview validation, consistent with the repository's existing query behavior.
Verification: all 5,453 Application tests and 3,309 Infrastructure tests passed, with no failures or skips. The working tree is clean. To keep this re-review focused on the requested incremental changes, I read the complete three-file commit diff and its full-PR hunks rather than repeating an exhaustive audit of the unchanged redesign promotion. Several CI checks were still pending when inspected; this approval does not certify those gates.
gpt-6.1-sol | 𝕏
…pi-redesign-to-main # Conflicts: # .github/workflows/mutation.yml
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes Reviewed the merge commit ff1fed1f since the prior Pullfrog review, including its complete three-file first-parent diff and the mutation workflow's current full-PR hunks.
- Merged mutation sharding: Applied the two Domain shards to PR runs, preserving the complete configured Domain mutation scope and passing each pattern as a separate
--mutateargument. - Preserved base-branch comparison: Retained
github.base_reffor PR fetches and Stryker comparison while adding CPU-based concurrency; nightly execution remains independent of a PR base. - Imported analyzer package centralization: Carried main's analyzer-test package configuration unchanged, including its transitive cryptography pin.
Verification: all 12 workflow shell-argument cases passed across the four matrix entries, both PR bases, and nightly execution. The harness stubbed Git fetch and Stryker; it did not run mutation testing. All 32 analyzer tests passed with no failures or skips. The working tree is clean.
This review deliberately follows the requested incremental scope rather than repeating an exhaustive audit of the unchanged redesign promotion. No API runtime or contract code changed; several CI gates were still queued when inspected.
gpt-6.1-sol | 𝕏
|
* fix: pin analyzer test Pkcs dependency to patched version (#728) (cherry picked from commit 61b0090) * fix: shard pull request domain mutation testing (#729) (cherry picked from commit bcfd81e) * Promote the API redesign with production compatibility checks (#723) * Bring the API redesign tree onto main for #1413 * Preserve calendar connection detection for shipped clients * Prove API and migrated schema compatibility with shipped clients * Trim whitespace in the archived main schema proof * Require the production health response to report the released commit * Bind initial approval previews to stored pending operations * fix: return the bulk skip refresh outcome and split the undo skip query SonarCloud failed the quality gate on two reliability findings in code this pull request brings to main: the bulk skip conflict check read a flag only a lambda wrote (S2583), and the undo skip check loaded Tags and Goals in one query (S8733). The transaction now returns whether every habit refreshed, and the undo skip check splits its query. A new case covers the concurrent change conflict; it failed with the refresh outcome forced to true and passes with the fix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 5526c9b) * Add per-account feature flag targeting and admin controls (#724) * feat: resolve feature flags per account * feat: administer account feature flags through MCP and REST * test: exclude feature flag targets from rate limit fixture * fix: reconcile account flag tests and migration metadata with redesign (cherry picked from commit 1b886bd) * Add header API versioning for v1 and v2 (#733) * Add header API versioning with v1 compatibility * Verify versioned OpenAPI and browser header compatibility * Preserve skip undo path contract with optional version header (cherry picked from commit ac8b34a) * fix: expose the config response in OpenAPI (#734) (cherry picked from commit a9d8dd5) * Route production redesign accounts through Cloudflare (#735) * Add cookie-based routing for the production redesign web service * Document web routing rollout, permissions, and rollback checks * Fix web routing coverage and disable infra install scripts * chore: regenerate the v2 OpenAPI document on main orbit-api#733 added openapi_v2.json and orbit-api#734 typed the /api/config response in parallel, so main carries a v2 document without the typed config schema and the Build check's stale-document step fails on every merge with main. dotnet build src/Orbit.Api regenerates it; openapi.json is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 9c64770) * Fix habit edit gating, end dates, and reminder updates (#736) * fix: validate habit updates with the resulting time range * fix: preserve end dates when creating root habits * fix: gate slip alert activation and preserve unchanged reminders * fix: compare effective reminder rules across stored formats (cherry picked from commit 7472203) * Reduce mutation and guard CI jobs (#737) * Select mutation shards from changed projects * Consolidate guards and filter artifact generation * Skip project documentation in mutation selection * Validate mutation base refs and restrict Git PATH * Read mutation shard changes from standard input (cherry picked from commit b802f9e) * test: strengthen habit and goal ownership coverage (#739) (cherry picked from commit 0e32628) * test: verify checkout session user metadata (#740) (cherry picked from commit da2f53a) * fix: protect OAuth code delivery with Turnstile (#741) (cherry picked from commit 1149606) * fix: enforce chat interaction scope before loading context (#743) (cherry picked from commit 98faf4e) * fix: erase accountability pairs during account deletion and reset (#745) * fix: erase accountability pairs during account deletion and reset * test: preserve migration compatibility checks after accountability cleanup (cherry picked from commit a9e5fb9) * Fix goal sync dates and deadline reminder counts (#747) * fix: compare streak sync dates in the user timezone * fix: show actual days remaining in goal deadline reminders * test: configure timezone for streak scheduler fixtures (cherry picked from commit 0e63842) * Manage staging landing routing in Terraform (#742) (cherry picked from commit abdd738) * Restrict Web Push endpoints to supported push services (#1503) (#751) Web Push subscriptions accept only HTTPS endpoints on the FCM, Mozilla, Apple and WNS push hosts, and the push HTTP client follows no redirects. Replaces orbit-api#748 as one commit so no commit carries the high-entropy sample fixture. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit d9f847a) * Erase account uploads before permanent deletion (#1507) (#750) * fix: erase uploaded objects before completing account deletion * fix: refuse upload reads after permanent account deletion * fix: grant uploads bucket deletion permissions and verify provider wiring * test: align storage startup checks with erasure requirements * fix: retire deleted Supabase upload storage * test: configure upload erasure providers in deletion fixtures Share the existing object-store fake across account deletion and scheduler fixtures after merging main. Preserve accountability assertions and production provider requirements for thomasluizon/orbit-tickets#1507. (cherry picked from commit fa33a94) * fix: limit anonymous auth and waitlist by email and source IP (ORB-108) (#749) * fix: enforce source IP ceilings alongside anonymous email limits * test: cover both rate limit dimensions before bot verification * fix: isolate BFF email rate limits by verified client IP * fix: generate and persist BFF forwarding secrets (cherry picked from commit de0f42f) * Limit access tokens to fifteen minutes (#738) * fix(auth): limit access tokens to fifteen minutes * fix(auth): align managed JWT lifetime and prove logout rejection (cherry picked from commit ade7016) * fix(auth): equalize account provisioning work (ORB-96) (#744) * test(auth): expose registration dependent sign-in work * fix(auth): perform uniform registration work before sign-in responses * test(auth): cover expired codes and tracked upsert conflicts * fix(auth): isolate signup retries and restore provisioning guards * fix(auth): recover signup work after dispatch failures * fix(auth): roll back failed signup referrals before retry (cherry picked from commit 1bd0d26) * fix: expose redesign controllers in both API versions Apply the carried version declarations to the Notion and proactive controllers and regenerate the v2 contract. Refs thomasluizon/orbit-tickets#746. * fix: honor account flags in proactive engine --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>




Closes thomasluizon/orbit-tickets#1413
The production API must accept the shipped web and Android requests while serving the redesign builds. This promotes the complete API redesign tree from
redesign/mainat0a32fab3e366d0029c8ea3d478841215ee1d8b88ontomainat8356d4debec5b48105734eb7d5dfd28559332da8, preserving the existing main reset and calendar-sync concurrency fixes. The PR is intended for squash merge.The runtime changes land across
src/Orbit.Api,src/Orbit.Application,src/Orbit.Domainandsrc/Orbit.Infrastructure, with their tests, migration designers, snapshot, embedded OAuth assets and generated OpenAPI document. Carrying the tree keeps dependent behavior and generated output together despite the divergent branch histories. The imported workflow and harness-test differences are unchanged from the redesign branch; the commit hook also formats imported C# files.Two delivery changes complete the compatibility and release requirements:
LocalizedErrorResultFilterkeeps calendar connection failures recognizable by both shipped clients. Their shared classifier inspects English message substrings, so localizing these two messages would hide the reconnect flow for Portuguese accounts. Error codes and statuses remain unchanged..github/workflows/release.ymlrequires the production/healthcommit to equal the released SHA, matching its existing staging check. Release remains a manual dispatch after merge.tests/Orbit.Infrastructure.Tests/Compatibility/README.mdrecords the audit and reproduction commands.RedesignMigrationCompatibilityTests.cschecks the real PostgreSQL model, migration operations and history guards.Compatibility/MainSchemaProbe.cs.txtruns against an archive of the actual main source to verify old binaries against the added schema.Compatibility table
The OpenAPI documents have 194 baseline operations and 201 promoted operations.
oasdiffv1.33.0, the version in the repository's pinned action, reports no breaking changes using the existing severity overrides. No removal is exempted based on assumed client usage.GET /api/Calendar/eventsincludeImported=false; declares the existing event array as its 200 responseSkipHabitRequestskipIdChatResponseperiodInsight,daySummary,streak,calendar,recordLists,accountRows,followUpsGoalListCardItemtrackingStatus,progressPercentage,projectedCompletionDate,daysToDeadlineMetricsCardseries,topHabitName,topHabitEmoji,habitId,habitTitle,weeklyCompletionRate,monthlyCompletionRate,lastCompletedDatePendingAgentOperationchanges,changeTargetCount,items,previewFingerprint,actionKeyProfileResponseactiveApiKeyCountGET /api/Chat/records/{kind}POST /api/Habits/{id}/skip/{skipId}/undoPUT /api/Profile/clock-formatPOST /api/ai/pending-operations/{id}/preview/refreshPOST /api/ai/pending-operations/{id}/reviseGET /api/notifications/subscriptionsGET /oauth/assets/{name}Source inspection also covers responses whose controllers do not declare their payload
types in OpenAPI:
totalCountitemsandunreadCount, default limit and offset remainhabitId,habitTitledateFrom,dateTo,metrics.completionSeries, andhabitIdon habit statisticsuses24HourClockerrorDetailswith codes and messagestype,status,requestId,errors: string[]remainstepvariant and optionaldomain,accessfieldssupportsToolSteps=true; existing event variants remainmessageOriginorangekeyAgentContextSnapshot.ColorSchemewas removed from the internal prompt context only.BuildAgentContextSnapshotandBuildDynamicSupplementare its only productionproducer and consumer; no controller or MCP tool returns this record.
The production web SHA and Android 1.3.57 (116) are both
b84bdc9ed91d57b147c7a8f76980c89cfe7ebdae. Deployment6803094988confirms the Android SHA, version, version code, open track and production API URL. The shared calendar classifier, web hook, Android hook, calendar schema, chat schemas and profile schema were read at that exact commit. Neither client repository was modified.Migration compatibility
AddPendingOperationRevisionPreviewFingerprint;RevisionNumber NOT NULL DEFAULT 0, so old inserts can omit bothLinkGamificationNotificationsToProgress/progressdestination; no existing schema is removedAddClockFormatPreferenceUsers.Uses24HourClockPreference, so old inserts and rows need no valueAddHabitSkipUndoThe complete model diff preserves all existing tables, columns, types, keys and foreign keys. Added columns are nullable or have defaults, and the new indexes are nonunique. The notification migration changes existing display data after archiving its originals; its existing batch and rollback tests pass. No existing migration or
MinSupportedVersionvalue is changed.AddPendingOperationRevisionsorts before main's last migration,AddMarketingContacts. EF applies missing migration IDs using history; generating a script only from the last main ID would omit this migration. The test generates the complete idempotent PostgreSQL script and verifies the history guard for all four missing IDs.The archived-main proof compiles the original main domain and persistence code, creates its SQLite schema, applies SQL emitted by EF from the three real structural migrations, then checks legacy reads, inserts, updates, omitted-column defaults and deletion with a populated undo table. It passes. The PostgreSQL notification rewrite was reviewed and covered by migration-operation tests; it was not executed against a live database. No application server was started.
Test evidence
dotnet build Orbit.slnx --nologo: passed, 0 errors, 18 existing warnings.dotnet test --no-build --nologo: passed all 9,451 tests: Domain 661, Application 5,452, Infrastructure 3,306, Analyzers 32; no failures or skips.oasdiff breaking origin/main:src/Orbit.Api/openapi.json src/Orbit.Api/openapi.json --fail-on ERR: passed with no breaking changes. The build leaves the committed document unchanged.dotnet test tests/Orbit.Infrastructure.Tests --filter FullyQualifiedName~RedesignMigrationCompatibilityTests --nologo: all 3 passed.dotnet test tests/Orbit.Infrastructure.Tests --filter FullyQualifiedName~RedesignSchemaProbeTests --nologo, from the archived main source with the committed proof copied in: passed. The compatibility report contains the full reproduction procedure.node --test tools/__tests__/check-timeless.test.mjs: all 19 passed. Workflow syntax, timeless text, suppression allowlist, root allowlist, dash baseline andgit diff --checkpass.Calendar regression observations:
dotnet test tests/Orbit.Infrastructure.Tests --no-build --filter 'FullyQualifiedName~LocalizedErrorResultFilterTests|FullyQualifiedName~CalendarControllerTests' --nologopassed all 28 existing tests with the defect present.dotnet test tests/Orbit.Infrastructure.Tests --filter FullyQualifiedName~CalendarConnectionFailures_StayRecognizableByShippedClients --nologoran the strengthened test through the actual error filter and account-language resolver. English passed; both Portuguese cases failed becausegoogle agendamessages do not match the shipped English classifier. Result: 2 passed, 2 failed for the intended reason.dotnet test tests/Orbit.Infrastructure.Tests --filter 'FullyQualifiedName~LocalizedErrorResultFilterTests|FullyQualifiedName~CalendarControllerTests' --nologopassed all 32 tests, including all 4 regression cases and the existing Portuguese localization checks for other errors.Release-check observations:
HealthCheckResponseTestspassed all 7 cases before the workflow fix; those tests do not inspect the release workflow.jq -e '.status == "Healthy"'accepted the observed healthy response while the intended release SHA differed: exit 0,true.jq -e --arg sha "$COMMIT_SHA" '.status == "Healthy" and .commit == $sha'rejected that mismatched SHA: exit 1,false. Extracted from the fixed workflow, it also accepted the observed matching SHA: exit 0,true.actionlint -ignore '^unexpected key "queue" for "concurrency" section\. expected one of "cancel-in-progress", "group"$': passed across all workflows, using the repository's existing queue compatibility rule.dotnet test tests/Orbit.Infrastructure.Tests --filter FullyQualifiedName~AgentChatWriteHoldTestspassed all 58 existing cases.dotnet test tests/Orbit.Infrastructure.Tests --filter FullyQualifiedName~ExecutePendingOperation_InitialPreview_BindsApprovalToTargetSetfailed as intended: stored fingerprint was null and changed targets incorrectly remained current.dotnet build Orbit.slnx: exit 0, zero errors.dotnet test: exit 0, 9,454 passed.origin/main: passed.External interface evidence
The new production release read of
/health.commitwas confirmed withcurl --fail-with-body --silent --show-error https://api.useorbit.org/health. The complete public response was:{"status":"Healthy","commit":"649c9dbe6b873d77ba071134b725a36e81a0a3b0","checks":[{"name":"background-services","status":"Healthy","description":"All background services running"},{"name":"database","status":"Healthy","description":"Database reachable"}]}The producer is also visible in
WebApplicationExtensions.WriteHealthCheckResponseAsync, which serializescommitfromRENDER_GIT_COMMIT. No unconfirmed Render response field was added.The EF checks were exercised against the installed EF Core 10.0.13 and PostgreSQL provider. The actual model diff produced
AddColumnOperationentries forUsers.Uses24HourClockPreference(nullable),PendingAgentOperations.PreviewFingerprint(nullable) andPendingAgentOperations.RevisionNumber(nonnullable, default 0), oneCreateTableOperationforHabitSkipUndos, and twoCreateIndexOperationentries. The real migration operations confirm the undo foreign key referencesHabitswith cascade deletion. No migration response fixture was invented; the unit tests re-derive this from the compiled migration classes and model.Assumptions
redesign/mainoutside the order. The redesign payload already exists there; only the calendar shim, production health check, proof artifacts and hook formatting need a later sync.PreviewFingerprintcolumn instead of adding a migration.Manual steps
thomasluizon/orbit-apiGitHub ActionsRelease APIworkflow (.github/workflows/release.yml) onmainand dispatch with exact inputsenvironment=production,branch=main. Success requires/health.status=Healthy,/health.commitequal to the merged SHA, and the existing Render live-commit check. No deployment was dispatched by this worker.thomasluizon/orbit-tickets#746: all redesign content promoted by this PR already exists onredesign/main, so the standing sync must not replay that payload. Only this branch's compatibility fix, production health check, proof artifacts and hook formatting are new to redesign. Completion is the note on Stop Stripe and Play renewals when account deletion is confirmed #746 and a sync containing only those delivery differences.main, withenvironment=productionandbranch=main. Verify/healthreports the merged commit.thomasluizon/orbit-tickets#746using the sync guidance in the existing compatibility README.