Skip to content

Feat/pro feature registry#386

Open
dishit-wednesday wants to merge 33 commits into
mainfrom
feat/pro-feature-registry
Open

Feat/pro feature registry#386
dishit-wednesday wants to merge 33 commits into
mainfrom
feat/pro-feature-registry

Conversation

@dishit-wednesday

@dishit-wednesday dishit-wednesday commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

What this does

Introduces the Pro feature layer for Off Grid and moves the email/calendar tools out of the open core into the private pro submodule, behind the Pro entitlement.

The branch history was rebuilt into 7 focused commits so that no commit in the open repo ever contains the email/calendar handler implementation or any RevenueCat API keys.

Change set

Pro feature registry + license seam (d300dbb0)

  • Adds the @offgrid/pro submodule and the registry seam (screenRegistry, sectionRegistry, loadProFeatures) so pro features register themselves at boot through an activate(registry) bag, with a no-op stub when the submodule is absent.

Tool extension seam + MCP wiring (26db7c0d)

  • Adds ToolExtension (with optional getToolDefinitions()) and wires extension-provided tools into the generation tool loop, the chat input popovers, and the chat screen.

RevenueCat pro license service + unlock modal (3d5613bf)

  • Web-billing flow: email is both the payment identity and the unlock key. proLicenseService configures RevenueCat with Trusted Entitlements (INFORMATIONAL verification, FAILED signatures blocked), caches the license in the keychain, and revalidates online at launch so access can be revoked from the RC dashboard.
  • ProUnlockModal collects the email, opens the RC web checkout, and verifies the purchase. Replaces the auto-restart library with a manual "close and reopen" prompt.

RevenueCat config placeholders (66ff0c44)

  • Public placeholder config so the app compiles. Real iOS/Android keys stay in gitignored revenueCatKeys.local.ts and are never committed.

In-app debug log viewer (99718d62)

  • DebugLogsScreen + debugLogsStore for on-device log inspection.

Pro-gated email/calendar tools (fcbc9c99)

  • send_email, create_calendar_event, read_calendar_events now live in the pro submodule as a ToolExtension. The core picker renders core tools plus extension-provided ones, deduped. handlers.ts/registry.ts match main exactly — the handler code exists only in the pro package.

tsconfig path mapping (18ee7f1d)

  • Maps @offgrid/core/*src/* for tsc, mirroring the existing jest moduleNameMapper, so tsc resolves the pro extension's core type imports.

Verification

  • tsc --noEmit clean.
  • EmailCalendarExtension unit tests pass (16/16).
  • No email/calendar handler code (handleSendEmail, handleCreateCalendarEvent, ensureCalendarPermission, RNCalendarEvents) in any source file across the branch history.
  • No RevenueCat API keys in history.
  • handlers.ts / registry.ts are byte-identical to main.

Notes

  • Branch coverage is ~79.7% (gate 80%); coverage top-up is a tracked follow-up.
  • The pro submodule changes ship via offgrid-pro#2; the gitlink points at ed9b93b.

@greptile-apps greptile-apps 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.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a Pro feature integration system using a git submodule, runtime registries for screens and settings sections, and a Metro configuration fallback. It also adds new built-in tools for sending emails and managing calendar events, alongside integrating tool extensions (such as MCP tools) into the generation loop. Feedback on these changes highlights a duplicate rendering bug of Pro settings sections in SettingsScreen.tsx, potential runtime crashes due to missing date validation in the calendar tool handlers, potential duplicate registrations in the screen and settings registries, and a lack of error handling when opening the mail client via Linking.openURL.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +325 to +327
{/* Pro feature sections registered at runtime by @offgrid/pro */}
{getSettingsSections().map((Section, i) => <Section key={i} />)}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The getSettingsSections() map is duplicated in two different places on the SettingsScreen (once before the 'Community' section and once before the 'Reset Onboarding' section). This will cause all registered Pro settings sections to render twice on the screen. Remove this duplicate entry.

Comment thread src/services/tools/handlers.ts Outdated
Comment on lines +435 to +441
const result = await RNAddCalendarEvent.presentEventCreatingDialog({
title,
startDate: new Date(startDate).toISOString(),
endDate: new Date(endDate).toISOString(),
...(location ? { location } : {}),
...(notes ? { notes } : {}),
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

If the model provides an invalid date string, calling new Date(startDate).toISOString() or new Date(endDate).toISOString() will throw a RangeError: Invalid time value and crash the execution. Validate the parsed dates before calling .toISOString().

  const start = new Date(startDate);
  const end = new Date(endDate);
  if (isNaN(start.getTime()) || isNaN(end.getTime())) {
    throw new Error('Invalid date format. Please provide valid ISO 8601 dates.');
  }
  const result = await RNAddCalendarEvent.presentEventCreatingDialog({
    title,
    startDate: start.toISOString(),
    endDate: end.toISOString(),
    ...(location ? { location } : {}),
    ...(notes ? { notes } : {}),
  });

Comment thread src/services/tools/handlers.ts Outdated
Comment on lines +454 to +456
const startDt = startDateStr ? new Date(startDateStr) : new Date();
const endDt = endDateStr ? new Date(endDateStr) : new Date(startDt.getTime() + 7 * 24 * 60 * 60 * 1000);
const events = await RNCalendarEvents.fetchAllEvents(startDt.toISOString(), endDt.toISOString());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

If startDateStr or endDateStr are invalid date strings, calling .toISOString() on the resulting Date objects will throw a RangeError. Validate the dates defensively before converting them to ISO strings.

  const startDt = startDateStr ? new Date(startDateStr) : new Date();
  if (isNaN(startDt.getTime())) {
    throw new Error('Invalid start date format.');
  }
  const endDt = endDateStr ? new Date(endDateStr) : new Date(startDt.getTime() + 7 * 24 * 60 * 60 * 1000);
  if (isNaN(endDt.getTime())) {
    throw new Error('Invalid end date format.');
  }
  const events = await RNCalendarEvents.fetchAllEvents(startDt.toISOString(), endDt.toISOString());

Comment on lines +10 to +12
export function registerScreen(screen: RegisteredScreen): void {
screens.push(screen);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Since loadProFeatures registers screens on status changes, calling registerScreen multiple times will append duplicate screens to the registry. Add a check to prevent duplicate screen registrations.

export function registerScreen(screen: RegisteredScreen): void {
  if (!screens.some(s => s.name === screen.name)) {
    screens.push(screen);
  }
}

Comment on lines +5 to +7
export function registerSettingsSection(component: ComponentType<any>): void {
sections.push(component);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Since loadProFeatures registers settings sections on status changes, calling registerSettingsSection multiple times will append duplicate sections to the registry. Add a check to prevent duplicate section registrations.

export function registerSettingsSection(component: ComponentType<any>): void {
  if (!sections.includes(component)) {
    sections.push(component);
  }
}

Comment thread src/services/tools/handlers.ts Outdated
if (subject) parts.push(`subject=${encodeURIComponent(subject)}`);
if (body) parts.push(`body=${encodeURIComponent(body)}`);
const url = `mailto:${encodeURIComponent(to)}${parts.length ? `?${parts.join('&')}` : ''}`;
await Linking.openURL(url);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Linking.openURL can throw an error if no mail client is installed or configured on the device (especially on simulators or devices without email apps). Wrap the call in a try-catch block to handle this gracefully, log the error to prevent silent failures, and return a meaningful error message.

  try {
    await Linking.openURL(url);
  } catch (err) {
    console.error('Failed to open mail client:', err);
    throw new Error('Could not open the mail app. Please ensure a mail client is configured on your device.');
  }
References
  1. When catching errors, log them instead of swallowing them to ensure failures are visible and to aid in debugging.

@dishit-wednesday dishit-wednesday force-pushed the feat/pro-feature-registry branch from 94b4a03 to 76cec8d Compare June 12, 2026 10:03
@codecov

codecov Bot commented Jun 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 73.39901% with 54 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.78%. Comparing base (7ac1d68) to head (b0d85e0).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/screens/ChatScreen/useChatGenerationActions.ts 41.37% 14 Missing and 3 partials ⚠️
src/components/ChatInput/Popovers.tsx 10.00% 4 Missing and 5 partials ⚠️
src/services/generationToolLoop.ts 81.81% 4 Missing and 2 partials ⚠️
src/screens/ChatScreen/ChatMessageArea.tsx 37.50% 4 Missing and 1 partial ⚠️
src/utils/logger.ts 20.00% 3 Missing and 1 partial ⚠️
src/screens/SettingsScreen.tsx 25.00% 3 Missing ⚠️
src/services/tools/handlers.ts 94.23% 0 Missing and 3 partials ⚠️
src/stores/debugLogsStore.ts 40.00% 3 Missing ⚠️
src/services/proLicenseService.ts 94.73% 0 Missing and 2 partials ⚠️
src/components/CustomAlert.tsx 50.00% 0 Missing and 1 partial ⚠️
... and 1 more

❌ Your patch check has failed because the patch coverage (73.39%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #386      +/-   ##
==========================================
- Coverage   81.82%   81.78%   -0.04%     
==========================================
  Files         241      245       +4     
  Lines       12835    12954     +119     
  Branches     3535     3574      +39     
==========================================
+ Hits        10502    10595      +93     
- Misses       1403     1416      +13     
- Partials      930      943      +13     
Files with missing lines Coverage Δ
src/bootstrap/loadProFeatures.ts 100.00% <100.00%> (ø)
src/components/ChatInput/index.tsx 84.14% <100.00%> (+0.19%) ⬆️
src/components/DebugLogsScreen/index.tsx 34.61% <100.00%> (+34.61%) ⬆️
src/components/ToolPickerSheet.tsx 86.66% <ø> (ø)
src/screens/ChatScreen/index.tsx 65.59% <ø> (ø)
src/services/tools/extensions.ts 100.00% <100.00%> (ø)
src/services/tools/registry.ts 100.00% <ø> (ø)
src/components/CustomAlert.tsx 92.30% <50.00%> (-3.70%) ⬇️
src/components/settings/sectionRegistry.ts 80.00% <80.00%> (ø)
src/services/proLicenseService.ts 94.73% <94.73%> (ø)
... and 8 more

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

dishit-wednesday and others added 7 commits June 19, 2026 14:33
Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Public placeholder config so the app compiles; real iOS/Android keys
stay in gitignored revenueCatKeys.local.ts and are never committed.

Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
The EmailCalendarExtension test imports the pro module by path, pulling it
into tsc's program past the pro/** exclude. Without a paths mapping for
@offgrid/core/* (which jest.config.js already has), tsc could not resolve
the pro module's core type imports, cascading into TS18046/TS7006 errors.
Mirror the jest moduleNameMapper so tsc and jest agree.

Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
@dishit-wednesday dishit-wednesday force-pushed the feat/pro-feature-registry branch from 98b32bc to 18ee7f1 Compare June 19, 2026 09:26
Two CI-only failures (both pass locally where the pro submodule is checked
out and revenueCatKeys.ts has the full local copy):

- revenueCatKeys.ts committed an older placeholder missing RC_WEB_PURCHASE_URL,
  which proLicenseService imports. Add the (public, safe-to-commit) web purchase
  URL placeholder so the committed config exports every symbol the app uses.
- EmailCalendarExtension.test.ts imported the pro module by a static path that
  tsc/jest cannot resolve when pro/ is absent (open-core CI does not check out
  the private submodule). Load it via a computed-path require and skip the suite
  when missing; it still runs locally and in the pro repo CI.

Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
@sonarqubecloud

Copy link
Copy Markdown

dishit-wednesday and others added 16 commits June 19, 2026 17:31
- Success card gets a 'Got it' button (and onRequestClose) so the user
  can read the activation message and dismiss it, instead of being
  trapped with no way out but force-killing the app.
- Card now has a visible border (colors.border) so it reads in dark
  mode, where the shadow alone disappeared against the background.
- CTA (Continue to payment / Verify and unlock) is enabled only once
  non-whitespace text is entered; no format validation. Whitespace is
  stripped before use, matching the service-layer trim/lowercase.
- Drop the stale RNRestart comment in ProDetailScreen (the lib was
  removed); Pro now loads on next launch via checkProStatus.
- Tests updated for the disabled-until-typed CTA, whitespace handling,
  and the success-card dismiss.

Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Points the pro gitlink at af5aa10 (offgrid-pro feat/email-calendar-tools)
which includes the MCP servers screen header inset fix, removal of the
demo-servers link, and the now-required server name field.

Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Co-Authored-By: Dishit Karia hanmadishit74@gmail.com
Co-Authored-By: Dishit Karia hanmadishit74@gmail.com
change from support@offgridmobile.co to
support@offgridmobileai.co

Co-Authored-By: Dishit Karia hanmadishit74@gmail.com
reset pro button on release builds

Co-Authored-By: Dishit Karia hanmadishit74@gmail.com
Adds js-sha256 and react-native-get-random-values for PKCE (SHA-256 + secure
random). react-native-keychain and react-native-inappbrowser-reborn were
already present. Includes the pod lockfile + privacy manifest updates.

Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
- mcpOAuthNativeAdapters: secure browser (InAppBrowser.openAuth), Keychain
  token storage, and PKCE crypto (get-random-values + js-sha256), implementing
  the adapter seams pro exposes.
- loadProFeatures: inject the adapters via pro.configureOAuthAdapters after the
  entitlement check; loaded lazily so free builds never pull the native libs.
- Register the offgrid://oauth redirect scheme in Info.plist and AndroidManifest.
- Bump pro submodule to the MCP OAuth commits.

Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Covers base64url/PKCE derivation, WWW-Authenticate resource_metadata parsing,
access-token expiry logic, and the McpClient 401 refresh-and-retry path.

Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
website/ was moved to its own repo in #407, but this test was left behind
referencing the deleted website/assets/js/revenuecat-link.js, so the suite
fails to load. Remove the dead test (the code it covered now lives elsewhere).

Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
… log

- SettingsScreen: move the Debug Logs entry out of the __DEV__ block so issues
  can be captured and shared from release builds (capture path was never
  __DEV__-gated; only this button was).
- mcpOAuthNativeAdapters: log browser availability and openAuth result type.

Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Local tool calls (both MCP and built-in) were failing because the model got two
conflicting instructions: the <mcp_tool_call> text hint AND the engine's native
tool template. The model then emitted a hybrid format (e.g. <notion-search …/>)
that neither llama.cpp nor our parsers recognise, so calls fell through as text.

- useChatGenerationActions: stop appending the extension hint here (it was also
  added in the tool loop — double injection). Single source now.
- augmentSystemPromptForTools: only add the MCP text hint when the model lacks
  native tool calling; LiteRT/remote/llama-with-tool-template rely on the
  structured tools instead.
- Add a [ToolLoop] diagnostic log (tool count + total schema char size).
- Bump pro submodule (schema trim, OAuth logs, tool-count fix).

Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
…erved)

Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
The main CI doesn't check out the private pro/ submodule (and tsconfig excludes
pro/**), so __tests__/unit/pro/*.test.ts — which import pro source directly —
failed lint/typecheck/test in CI. The tests now live in pro/__tests__ (pushed in
the pro repo). Bumps the pro submodule pointer to include them.

Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
- Revert the release-visible Debug Logs button — it stays behind __DEV__ and is
  stripped from release builds. Also brings SettingsScreen back under the 500-line
  lint limit (the release-visible block had pushed it to 503).
- Bump pro submodule to 16e97aa (relocated pro-source tests).

Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
dishit-wednesday and others added 9 commits June 22, 2026 15:10
…dapters

Restores the CI test + coverage jobs that the fix-C change had broken:

- generationToolLoop.test.ts: stub llmService.supportsToolCalling in the mock
  (fix C now calls it in callLLMWithRetry — the missing stub failed all 42
  tests in the suite). Add tests for the text tool-call parsing fallbacks
  (Gemma <|tool_call> JSON/unquoted/unclosed/colon args, <tool_call>
  function-call/bare/standard-JSON styles) and the empty-query backfill — the
  robustness layer that was previously uncovered.
- mcpOAuthNativeAdapters.test.ts: new — covers the browser/storage/crypto
  adapters (browser availability, success/cancel, keychain get/set/remove,
  random bytes, sha256). Brings the new file to 100% and global branch
  coverage back over the 80% threshold.

Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
… IAP VerifyError)

Amazon IAP's com.amazon.device.iap.ResponseReceiver (pulled in transitively via
RevenueCat) ships pre-Java-7 bytecode without stackmap frames. Robolectric
instantiates every manifest-declared receiver during application setup, so that
class fails strict bytecode verification on JDK 17+
(VerifyError: "Expecting a stackmap frame at branch target"), which failed all 11
DownloadCompleteBroadcastReceiverTest cases. Add -noverify to the unit-test JVM so
the third-party class loads.

This surfaced only now because the Android test CI step was skipped in prior runs
(the Jest step failed first); it is unrelated to the JS changes in this PR.
Verified locally on JDK 17 (CI parity): full testDebugUnitTest suite — 4 classes,
82 tests — passes with 0 failures.

Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Both the "Get Pro" CTA and "Already paid? Unlock with email" called the same
openEmailModal() with no mode, and the modal always initialised to 'pay'. So
tapping "Already paid?" landed on "Continue to payment / $50" and the user had to
tap the in-modal "Already paid? Verify email instead" toggle again to reach email
verification.

ProUnlockModal now takes an initialMode and re-applies it whenever it opens (the
modal stays mounted, so a useEffect on `visible` is needed). ProDetailScreen opens
'pay' from Get Pro and 'verify' from "Already paid?". Adds tests for both paths.

Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
…rlay

Replace the bare spinner with an eased progress bar (creeps toward a 90% ceiling
over the expected load window, never claims "done" before the model loads) and a
rotating set of loading tips so the wait reads as "working" and teaches useful
behaviour. Tip copy follows the brand voice guide.

Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
…s logger

The merge took main's no-op-in-release logger (the ANR/perf fix), which dropped
setLogListener — leaving the dev Debug Logs viewer fed by nothing. Re-tap
logger.log/warn/error into the debug store inside a __DEV__ block so the dev
viewer works again. Release builds strip the block entirely, so logging stays a
no-op there (no file I/O, no listener) exactly as on main.

Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Merging main's newer code nudged global branch coverage to 79.93%. Cover the gaps:
- debugLogsStore: ring-buffer cap + clear (was 0%).
- proLicenseService: resetProIdentityForTesting (anonymous/non-anonymous/error)
  and activateProByEmail error paths (logIn failure, miss + logOut failure).

Branch coverage back to 80.06%.

Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Updates the Pro unlock modal and detail-screen copy to a membership
framing: 'Get Pro' / 'Verify membership' CTAs, 'Already a member?' /
'Not a member yet?' toggles, and membership-worded subtitles and errors.
Adds a testID to the modal CTA so tests target it unambiguously now that
the pay button label ('Get Pro') matches the screen buttons.

Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Small on-device models can't fit many tool schemas in context, so MCP-heavy
setups (e.g. Notion's ~17 tools) produce empty/garbled calls or no response. For
LiteRT only, add a fast tools-free routing pass: list the enabled tools' names +
one-line descriptions, ask the model which are relevant, then carry only the
selected schemas into the real generation pass.

- litert.generateToolSelection: one-shot routing on a throwaway native session
  (temp 0, no tools, no history) so it never enters chat or the real
  conversation's context; drops the session afterward.
- litertToolSelector.selectLiteRTTools: builds the catalog, lenient name match.
- runToolLoop: gated on isLiteRTActive() + >5 tools; filters toolSchemas to the
  selected set; falls back to all tools on any miss or failure.

Complements the existing per-tool schema trim (fewer tools + smaller each).
llama/remote paths untouched. Selector branches covered by tests.

Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
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