Fix/365 enforce latest oauth gate from 351#421
Merged
M3gA-Mind merged 10 commits intoApr 7, 2026
Merged
Conversation
… flag in handler Consolidate tauriCommands imports and drop redundant mock cast. Handle granted accessibility in focus/visibility callback instead of a follow-up effect. Made-with: Cursor
…sai#365) - Add semver helpers and VITE_MINIMUM_SUPPORTED_APP_VERSION / download URL in app config - Block openhuman://oauth/success when desktop build is below minimum; enqueue error, open latest-release URL, dispatch oauth:stale-app - Pass new Vite env vars through release.yml and build-windows.yml Build frontend - Document policy in docs/RELEASE_POLICY.md; note vars in app/.env.example Closes tinyhumansai#365 Made-with: Cursor
…est-oauth-gate-from-351
Contributor
📝 WalkthroughWalkthroughAdds a build-time Vite env var injection and implements an OAuth deep-link version gate: semver utilities, gate evaluator, integration into deep-link handling (blocks stale desktop apps), CI changes, tests, and release documentation. (≤50 words) Changes
Sequence DiagramsequenceDiagram
participant User
participant DesktopApp as "Desktop App"
participant Listener as "Deep Link Listener"
participant Gate as "OAuth Version Gate"
participant Tauri as "Tauri Runtime"
participant Reporter as "Error Reporter"
User->>DesktopApp: Click OAuth callback (openhuman://oauth/success)
DesktopApp->>Listener: Deliver deep link
Listener->>Listener: Validate params (integrationId, skillId)
Listener->>Gate: evaluateOAuthAppVersionGate()
Gate->>Tauri: getVersion() (if Tauri)
Tauri-->>Gate: current version (or error)
Gate->>Gate: compare current vs MINIMUM_SUPPORTED_APP_VERSION
alt allowed (minimum unset/invalid or current >= minimum or non-Tauri)
Gate-->>Listener: { ok: true }
Listener->>DesktopApp: Continue OAuth completion flow (clientKeyShare, startSkill...)
else blocked (current < minimum)
Gate-->>Listener: { ok: false, current, minimum, downloadUrl }
Listener->>Reporter: enqueueError(buildManualSentryEvent(...))
Listener->>DesktopApp: openUrl(downloadUrl) (try)
DesktopApp->>User: Browser opens download page
Listener->>DesktopApp: dispatch CustomEvent 'oauth:stale-app' with metadata
Listener-->>DesktopApp: return early (block OAuth completion)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
senamakel
previously approved these changes
Apr 7, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/release.yml (1)
305-314:⚠️ Potential issue | 🟠 MajorFail the release if the configured OAuth floor is above the version being shipped.
These variables are manual. If
VITE_MINIMUM_SUPPORTED_APP_VERSIONis accidentally set higher thanneeds.prepare-release.outputs.version, the freshly published build will block its own OAuth flow on first use. Please add a pre-build guard that aborts whenminimum > release version.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 305 - 314, Add a pre-build guard step before the "Build frontend" run that reads the prepared release version (needs.prepare-release.outputs.version) and the configured VITE_MINIMUM_SUPPORTED_APP_VERSION and fails the job if the minimum required version is greater than the release version; implement the check by running a small version-compare script (e.g., node/python using a semver-aware compare) that exits non-zero when VITE_MINIMUM_SUPPORTED_APP_VERSION > needs.prepare-release.outputs.version and emits a clear error message so the workflow aborts before the yarn build.
🧹 Nitpick comments (2)
app/src/utils/semver.ts (1)
6-29: Preferconstarrow exports for new utility helpers.Small consistency nit for
app/srcutilities.As per coding guidelines,
**/*.{js,jsx,ts,tsx}: Prefer arrow functions over function declarations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/utils/semver.ts` around lines 6 - 29, Convert the three exported function declarations to exported const arrow functions to follow the module style: replace "export function parseSemverParts(...)", "export function compareSemver(...)" and "export function isVersionAtLeast(...)" with "export const parseSemverParts = (...) => { ... }", "export const compareSemver = (...) => { ... }" and "export const isVersionAtLeast = (...) => { ... }" respectively, preserving the same parameter names, return types, internal logic (including regex, parseInt behavior, and fallback returns) and export names so callers and semantics of parseSemverParts, compareSemver and isVersionAtLeast remain unchanged.app/src/utils/oauthAppVersionGate.ts (1)
17-47: Harden env-string handling to avoid brittle gate behavior under misconfiguration.
minimumanddownloadUrlare used as already-valid strings. Adding nullish normalization + download URL fallback makes this flow safer if config defaults change or are blank.♻️ Suggested hardening diff
export async function evaluateOAuthAppVersionGate(): Promise<OAuthAppVersionGateResult> { - const minimum = MINIMUM_SUPPORTED_APP_VERSION.trim(); + const minimum = (MINIMUM_SUPPORTED_APP_VERSION ?? '').trim(); if (!minimum) { return { ok: true }; } @@ - console.warn('[oauth-app-version] blocked OAuth success deep link', { current, minimum }); - return { ok: false, current, minimum, downloadUrl: LATEST_APP_DOWNLOAD_URL }; + const downloadUrl = + (LATEST_APP_DOWNLOAD_URL ?? '').trim() || + 'https://github.com/tinyhumansai/openhuman/releases/latest'; + console.warn('[oauth-app-version] blocked OAuth success deep link', { current, minimum, downloadUrl }); + return { ok: false, current, minimum, downloadUrl }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/utils/oauthAppVersionGate.ts` around lines 17 - 47, Normalize and guard MINIMUM_SUPPORTED_APP_VERSION and LATEST_APP_DOWNLOAD_URL before using them: ensure MINIMUM_SUPPORTED_APP_VERSION is nullish-checked and trimmed (fallback to empty string) before calling parseSemverParts/minimum checks, and normalize the result of getVersion similarly; additionally provide a safe fallback for download URL (fallback LATEST_APP_DOWNLOAD_URL to a known constant or empty string) before returning it; update checks around parseSemverParts, isTauri, getVersion, and isVersionAtLeast in oauthAppVersionGate so blank or undefined env values don’t cause brittle behavior and the returned downloadUrl is never undefined.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/utils/config.ts`:
- Around line 48-59: The current approach exports MINIMUM_SUPPORTED_APP_VERSION
and LATEST_APP_DOWNLOAD_URL from utils/config.ts using build-time env vars,
which freezes the minimum-version gate into shipped installers; change the
gating to a runtime check instead by adding logic that fetches the current
minimum supported version from a server/updater endpoint at app startup and
compares it to the local app version (use the existing
MINIMUM_SUPPORTED_APP_VERSION constant as a fallback/default only), and ensure
the code that enforces the block (wherever OAuth deep-link completion is
checked) consults the runtime-fetched value first and falls back to the bundled
value; also update any UI or redirect logic that uses LATEST_APP_DOWNLOAD_URL to
accept a server-provided URL with the bundled constant as fallback.
In `@app/src/utils/semver.ts`:
- Around line 6-17: Update the semver regex to require a full-string match by
adding an end anchor so malformed inputs like "0.51.x" or "1.2beta" are rejected
(change the regex in parseSemverParts to use
/^v?(\d+)(?:\.(\d+))?(?:\.(\d+))?$/). Also refactor parseSemverParts,
compareSemver, and isVersionAtLeast from function declarations to arrow function
expressions to match style guidelines, keeping their current behavior and return
types.
---
Outside diff comments:
In @.github/workflows/release.yml:
- Around line 305-314: Add a pre-build guard step before the "Build frontend"
run that reads the prepared release version
(needs.prepare-release.outputs.version) and the configured
VITE_MINIMUM_SUPPORTED_APP_VERSION and fails the job if the minimum required
version is greater than the release version; implement the check by running a
small version-compare script (e.g., node/python using a semver-aware compare)
that exits non-zero when VITE_MINIMUM_SUPPORTED_APP_VERSION >
needs.prepare-release.outputs.version and emits a clear error message so the
workflow aborts before the yarn build.
---
Nitpick comments:
In `@app/src/utils/oauthAppVersionGate.ts`:
- Around line 17-47: Normalize and guard MINIMUM_SUPPORTED_APP_VERSION and
LATEST_APP_DOWNLOAD_URL before using them: ensure MINIMUM_SUPPORTED_APP_VERSION
is nullish-checked and trimmed (fallback to empty string) before calling
parseSemverParts/minimum checks, and normalize the result of getVersion
similarly; additionally provide a safe fallback for download URL (fallback
LATEST_APP_DOWNLOAD_URL to a known constant or empty string) before returning
it; update checks around parseSemverParts, isTauri, getVersion, and
isVersionAtLeast in oauthAppVersionGate so blank or undefined env values don’t
cause brittle behavior and the returned downloadUrl is never undefined.
In `@app/src/utils/semver.ts`:
- Around line 6-29: Convert the three exported function declarations to exported
const arrow functions to follow the module style: replace "export function
parseSemverParts(...)", "export function compareSemver(...)" and "export
function isVersionAtLeast(...)" with "export const parseSemverParts = (...) => {
... }", "export const compareSemver = (...) => { ... }" and "export const
isVersionAtLeast = (...) => { ... }" respectively, preserving the same parameter
names, return types, internal logic (including regex, parseInt behavior, and
fallback returns) and export names so callers and semantics of parseSemverParts,
compareSemver and isVersionAtLeast remain unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 12e94415-0134-4f41-b678-673aa69b38f5
📒 Files selected for processing (9)
.github/workflows/build-windows.yml.github/workflows/release.ymlapp/.env.exampleapp/src/utils/config.tsapp/src/utils/desktopDeepLinkListener.tsapp/src/utils/oauthAppVersionGate.tsapp/src/utils/semver.test.tsapp/src/utils/semver.tsdocs/RELEASE_POLICY.md
- Anchor semver regex to full string; arrow-style exports; tests for bad inputs - Never throw from evaluateOAuthAppVersionGate; try/catch in deep link + omit raw URL from error logs - Document build-time vs runtime policy in config JSDoc and RELEASE_POLICY - Remove unused React import in SkillSetupWizard (tsc) Made-with: Cursor
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/utils/oauthAppVersionGate.ts`:
- Around line 16-52: Issue: evaluateOAuthAppVersionGate currently "fails open"
for unknown/malformed desktop versions; it should "fail closed" once a minimum
is configured. Fix: inside evaluateOAuthAppVersionGate, when getVersion() throws
or parseSemverParts(current) returns falsy (i.e., current is unparseable) change
the early returns that currently return { ok: true } to return { ok: false,
current?: current, minimum, downloadUrl: LATEST_APP_DOWNLOAD_URL } so unknown
versions are blocked; also change the outer catch to return { ok: false,
minimum, downloadUrl: LATEST_APP_DOWNLOAD_URL } instead of allowing OAuth. Use
the existing symbols getVersion, parseSemverParts, current, minimum,
LATEST_APP_DOWNLOAD_URL and preserve non-throwing behavior while returning the
failure payload.
In `@docs/RELEASE_POLICY.md`:
- Around line 5-9: Add an explicit retirement checklist entry under the
"Distribution" section: ensure deprecated installers, old updater manifests, and
stale release artifacts are removed or redirected and verify they are
unreachable from primary distribution surfaces (website, CDN, GitHub Releases,
and the Tauri updater configured by prepareTauriConfig.js); include concrete
steps to (1) identify obsolete artifacts by tag/asset name, (2) delete or
replace artifacts on GitHub Releases and CDN, (3) add HTTP redirects or 410
responses where removal isn’t possible, and (4) run a verification step that
attempts to fetch known-old installer/update URLs and fails, and duplicate this
checklist addition in the other distribution-related section (lines referenced
in review).
- Around line 12-21: The tauri build step is missing the
VITE_MINIMUM_SUPPORTED_APP_VERSION and VITE_LATEST_APP_DOWNLOAD_URL env vars so
shipped installers bake unset defaults; update the tauri-action step in both
`.github/workflows/release.yml` and `.github/workflows/build-windows.yml` to add
VITE_MINIMUM_SUPPORTED_APP_VERSION: ${{ vars.VITE_MINIMUM_SUPPORTED_APP_VERSION
}} and VITE_LATEST_APP_DOWNLOAD_URL: ${{ vars.VITE_LATEST_APP_DOWNLOAD_URL }} to
the env block of the tauri-action invocation (the same env used by the
standalone yarn build), and also update the release runbook to state that both
the standalone frontend build and the tauri-action rebuild must receive these
two variables so the gate in app/src/utils/oauthAppVersionGate.ts and
desktopDeepLinkListener (app/src/utils/desktopDeepLinkListener.ts) is enabled in
shipped installers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2ac5e0c7-ac5f-4a6a-a7ff-3b38870cd413
📒 Files selected for processing (6)
app/src/utils/config.tsapp/src/utils/desktopDeepLinkListener.tsapp/src/utils/oauthAppVersionGate.tsapp/src/utils/semver.test.tsapp/src/utils/semver.tsdocs/RELEASE_POLICY.md
✅ Files skipped from review due to trivial changes (3)
- app/src/utils/config.ts
- app/src/utils/semver.test.ts
- app/src/utils/semver.ts
- Block OAuth when minimum is set but getVersion fails or version is unparseable - Pass VITE_MINIMUM_* through tauri-action env (release + Windows) so bundles match yarn build - Expand RELEASE_POLICY: artifact retirement, dual workflow env note - Friendlier copy when current version is unknown Made-with: Cursor
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/RELEASE_POLICY.md (1)
13-13: Good documentation of the build-time limitation.The note about runtime API as future work is valuable. The current approach means users who never upgrade will still have whatever minimum was baked into their build. Consider opening an issue to track this enhancement if it's not already captured.
Would you like me to draft an issue for tracking the runtime minimum version API enhancement?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/RELEASE_POLICY.md` at line 13, Create a new issue tracking the "runtime minimum version API" enhancement referenced in RELEASE_POLICY.md: title it something like "Implement runtime API to enforce moving minimum supported app semver", include a clear description referencing the build-time limitation text in RELEASE_POLICY.md and the desired runtime behavior, list motivation and acceptance criteria (API surface, fallback to bundled value, migration plan, tests), add relevant labels (enhancement, backlog) and link the new issue from RELEASE_POLICY.md or the release docs so the future work is discoverable; assign or suggest owners if known.app/src/utils/desktopDeepLinkListener.ts (1)
127-134: Consider aligning catch fallback with fail-closed intent.The gate function (
evaluateOAuthAppVersionGate) is designed to never throw and handles all internal errors. However, if it somehow does throw, this catch block allows OAuth to proceed (ok: true). For consistency with the fail-closed principle documented inoauthAppVersionGate.ts, consider blocking here as well when the gate unexpectedly throws—especially since a minimum version is presumably configured in production.That said, since the gate is explicitly non-throwing by design, this is a defensive edge case. If the current fail-open behavior is intentional for this outer fallback, a brief inline comment clarifying the rationale would help future maintainers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/utils/desktopDeepLinkListener.ts` around lines 127 - 134, The catch currently sets versionGate = { ok: true } which fails open; change the fallback to fail closed by setting versionGate = { ok: false } (or an appropriate blocking shape expected by downstream logic) so that when evaluateOAuthAppVersionGate unexpectedly throws the OAuth flow is blocked; update the catch in desktopDeepLinkListener around evaluateOAuthAppVersionGate() and add a short inline comment explaining this defensive fail-closed behavior (or, if the original fail-open was intentional, replace with a brief comment clarifying that choice).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/utils/desktopDeepLinkListener.ts`:
- Around line 127-134: The catch currently sets versionGate = { ok: true } which
fails open; change the fallback to fail closed by setting versionGate = { ok:
false } (or an appropriate blocking shape expected by downstream logic) so that
when evaluateOAuthAppVersionGate unexpectedly throws the OAuth flow is blocked;
update the catch in desktopDeepLinkListener around evaluateOAuthAppVersionGate()
and add a short inline comment explaining this defensive fail-closed behavior
(or, if the original fail-open was intentional, replace with a brief comment
clarifying that choice).
In `@docs/RELEASE_POLICY.md`:
- Line 13: Create a new issue tracking the "runtime minimum version API"
enhancement referenced in RELEASE_POLICY.md: title it something like "Implement
runtime API to enforce moving minimum supported app semver", include a clear
description referencing the build-time limitation text in RELEASE_POLICY.md and
the desired runtime behavior, list motivation and acceptance criteria (API
surface, fallback to bundled value, migration plan, tests), add relevant labels
(enhancement, backlog) and link the new issue from RELEASE_POLICY.md or the
release docs so the future work is discoverable; assign or suggest owners if
known.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e72734db-2f63-471a-926b-b5722e3ac54e
📒 Files selected for processing (5)
.github/workflows/build-windows.yml.github/workflows/release.ymlapp/src/utils/desktopDeepLinkListener.tsapp/src/utils/oauthAppVersionGate.tsdocs/RELEASE_POLICY.md
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/build-windows.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/release.yml
3 tasks
6 tasks
AusAgentSmith
pushed a commit
to AusAgentSmith/openhuman
that referenced
this pull request
May 23, 2026
* feat: display app version in settings panel * fix(onboarding): auto-refresh accessibility state after grant (tinyhumansai#351) * style(onboarding): apply formatter for issue tinyhumansai#351 fix * fix(onboarding): ESLint + typed mock for ScreenPermissionsStep; clear flag in handler Consolidate tauriCommands imports and drop redundant mock cast. Handle granted accessibility in focus/visibility callback instead of a follow-up effect. Made-with: Cursor * fix(release): gate OAuth deep links on minimum app version (tinyhumansai#365) - Add semver helpers and VITE_MINIMUM_SUPPORTED_APP_VERSION / download URL in app config - Block openhuman://oauth/success when desktop build is below minimum; enqueue error, open latest-release URL, dispatch oauth:stale-app - Pass new Vite env vars through release.yml and build-windows.yml Build frontend - Document policy in docs/RELEASE_POLICY.md; note vars in app/.env.example Closes tinyhumansai#365 Made-with: Cursor * fix: import React in SkillSetupWizard component * fix: address CodeRabbit review (OAuth gate, semver, logging) - Anchor semver regex to full string; arrow-style exports; tests for bad inputs - Never throw from evaluateOAuthAppVersionGate; try/catch in deep link + omit raw URL from error logs - Document build-time vs runtime policy in config JSDoc and RELEASE_POLICY - Remove unused React import in SkillSetupWizard (tsc) Made-with: Cursor * fix: CodeRabbit — fail-closed OAuth gate, tauri-action Vite env, runbook - Block OAuth when minimum is set but getVersion fails or version is unparseable - Pass VITE_MINIMUM_* through tauri-action env (release + Windows) so bundles match yarn build - Expand RELEASE_POLICY: artifact retirement, dual workflow env note - Friendlier copy when current version is unknown Made-with: Cursor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #365
Summary
Problem
Solution
Submission Checklist
app/) and/orcargo test(core) for logic you add or changeapp/test/e2e, mock backend,tests/json_rpc_e2e.rsas appropriate)//////!(Rust), JSDoc or brief file/module headers (TS) on public APIs and non-obvious modules(Any feature related checklist can go in here)
Impact
Related
Summary by CodeRabbit