Skip to content

feat: polish app#2166

Merged
deadlyjack merged 2 commits into
mainfrom
ajit/fix-terminal-installation
Jun 9, 2026
Merged

feat: polish app#2166
deadlyjack merged 2 commits into
mainfrom
ajit/fix-terminal-installation

Conversation

@deadlyjack

Copy link
Copy Markdown
Member
  • Bump version to 1.12.3 (versionCode 973), sync package.json/lockfile
  • Add "terminal not installed prompt" and "terminal first launch prompt" i18n strings (32 locales)
  • Show terminal installation prompt on first launch; prompt on navigating to public/ directory
  • Auto-enable update checks without prompt for Play Store installs; guard against NaN versionCode
  • Move ad initialization to run only on non-Play Store builds after promotions fetch
  • Prevent eruda download failures from crashing developer tools init
  • Remove stray admob extraneous entry from lockfile

- Bump version to 1.12.3 (versionCode 973), sync package.json/lockfile
- Add "terminal not installed prompt" and "terminal first launch prompt" i18n strings (32 locales)
- Show terminal installation prompt on first launch; prompt on navigating to public/ directory
- Auto-enable update checks without prompt for Play Store installs; guard against NaN versionCode
- Move ad initialization to run only on non-Play Store builds after promotions fetch
- Prevent eruda download failures from crashing developer tools init
- Remove stray admob extraneous entry from lockfile
@github-actions github-actions Bot added the translations Anything related to Translations Whether a Issue or PR label Jun 8, 2026
@greptile-apps

greptile-apps Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR polishes the app with version bump to 1.12.3, adds terminal installation prompts (first-launch and public/ navigation), auto-enables update checks for Play Store installs, moves ad initialization post-promotions-fetch, guards against NaN versionCode, and adds i18n strings across 32 locales.

  • Terminal install prompts: A first-launch prompt in main.js and a navigation-gate in fileBrowser.js both call terminalManager.checkAndInstallTerminal(); main.js correctly checks !result.success || result.error, but fileBrowser.js checks only res.error, risking a silent failure that still navigates to public/.
  • Ad init & update-check refactor: startAd() is deferred until after promotions are fetched and skipped for Play Store builds; Play Store installs now auto-enable checkForAppUpdates without a prompt.
  • devTools hardening: An empty catch {} prevents CDN download failures from crashing developer-tools init (eruda script load error will still surface, but via a distinct path).

Confidence Score: 4/5

Safe to merge with one fix: fileBrowser.js should check !res.success alongside res.error to prevent a silently-failed terminal install from triggering navigation into public/.

The fileBrowser.js installation guard only tests res.error, while main.js tests both !result.success and result.error for the same API call. A failed install returning {success:false, error:undefined} would slip through undetected and navigate the user into public/ without terminal available. All other changes (ad deferral, NaN guard, i18n, devTools catch) look correct.

src/pages/fileBrowser/fileBrowser.js — the installation success-check logic at line 887-890

Important Files Changed

Filename Overview
src/pages/fileBrowser/fileBrowser.js Adds a terminal-install gate when navigating to public/; incomplete success check (only res.error, not !res.success) could let a silently-failed install proceed to navigation.
src/main.js Adds promptTerminalInstall() with proper await + error surfacing, moves ad init post-promotions, auto-enables updates for Play Store, and guards NaN versionCode before cache clear.
src/lib/devTools.js Empty catch block prevents eruda download errors from crashing dev-tools init; script load will still fail and surface a less-informative error.
src/lang/en-us.json Adds two new i18n keys for terminal prompts; applied consistently across all 32 locale files.

Sequence Diagram

sequenceDiagram
    participant User
    participant fileBrowser
    participant Terminal
    participant terminalManager
    participant loader

    User->>fileBrowser: Tap public/ folder
    fileBrowser->>Terminal: isInstalled()
    alt Already installed
        Terminal-->>fileBrowser: true
        fileBrowser->>fileBrowser: navigate(url, name)
    else Not installed
        Terminal-->>fileBrowser: false
        fileBrowser->>User: confirm("terminal not installed prompt")
        alt User accepts
            fileBrowser->>loader: create + show
            fileBrowser->>terminalManager: checkAndInstallTerminal()
            alt res.error set (checked)
                terminalManager-->>fileBrowser: "{success:false, error:"..."}"
                fileBrowser->>User: helpers.error(error)
                Note right of fileBrowser: returns, no navigation
            else "res.success=false, res.error=undefined (NOT checked)"
                terminalManager-->>fileBrowser: "{success:false}"
                fileBrowser->>fileBrowser: navigate(url, name) ⚠️
            else Success
                terminalManager-->>fileBrowser: "{success:true}"
                fileBrowser->>fileBrowser: navigate(url, name)
            end
            fileBrowser->>loader: destroy
        else User declines
            Note right of fileBrowser: returns, no navigation
        end
    end
Loading

Reviews (2): Last reviewed commit: "fix: terminal install error handling and..." | Re-trigger Greptile

Comment thread src/lib/devTools.js
Comment on lines +56 to 59
} catch {
} finally {
if (showLoader) loader.destroy();
}

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.

P2 Silent catch leaves init in a broken-but-continuing state

The empty catch {} swallows the CDN download error, so control falls through to helpers.toInternalUri(erudaPath) and the <script> tag load — on a file that was never written. The onerror callback then rejects the inner Promise, which is caught by the outer catch and re-thrown. Dev tools init still fails, but the caller now sees a cryptic "script load error" rather than the original CDN/network error, making the failure harder to diagnose. At minimum the caught error should be logged; better still, return early to avoid the misleading follow-on failure.

Comment thread src/pages/fileBrowser/fileBrowser.js
Comment thread src/main.js
@UnschooledGamer

Copy link
Copy Markdown
Member

We should not remove the opt-in prompt for checking updates on play store builds, it makes sense to let the user decide and have their consent before checking updates for them.

- Await and check terminal install result in promptTerminalInstall(), show error on failure
- Block navigation to public/ after failed terminal install in fileBrowser
- Fix undefined dirUrl/dirName references after inlining checkAndNavigate (use closure url/name)
@deadlyjack

Copy link
Copy Markdown
Member Author

@greptile_apps review again

@deadlyjack

Copy link
Copy Markdown
Member Author

We should not remove the opt-in prompt for checking updates on play store builds, it makes sense to let the user decide and have their consent before checking updates for them.

Most users don't to be bombarded with prompts after installing apps thats why I removed it, anyhow for most users it's a feature and won't mind it. Let's wait for user review if they don't like it we can always update.

Comment thread src/pages/fileBrowser/fileBrowser.js
@UnschooledGamer

Copy link
Copy Markdown
Member

Most users don't to be bombarded with prompts after installing apps thats why I removed it, anyhow for most users it's a feature and won't mind it. Let's wait for user review if they don't like it we can always update.

Those prompts are mostly one-time and similar to onboarding, and it would take a long time for a consent oriented user to notice that something like this exists in the background, I don't remember if they get a toast message or not.

If prompting afterwards is a problem, we can do it before, where the user has to click a checkbox that says something like the app connects with acode.app for related services and github.com for checking updates. Similar to Xed-Editor which prompts the user before loading the Editor or any components related.

@UnschooledGamer

UnschooledGamer commented Jun 9, 2026

Copy link
Copy Markdown
Member

At the end, whatever you want man.

@deadlyjack deadlyjack merged commit ce13108 into main Jun 9, 2026
10 checks passed
@github-project-automation github-project-automation Bot moved this from Backlog to Done in The Code Board - Acode Jun 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

translations Anything related to Translations Whether a Issue or PR

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants