Skip to content

feat: Standalone App Packaging & Setup Flow - #96

Open
karan-vk wants to merge 34 commits into
Gururagavendra:mainfrom
karan-vk:main
Open

feat: Standalone App Packaging & Setup Flow#96
karan-vk wants to merge 34 commits into
Gururagavendra:mainfrom
karan-vk:main

Conversation

@karan-vk

@karan-vk karan-vk commented Feb 12, 2026

Copy link
Copy Markdown
Collaborator

This PR introduces packaging support and a user-friendly setup flow to run the application as a standalone desktop executable, removing the strict Docker dependency for end users.

Changes

  • Core: Refactored config.py to use platformdirs for OS-correct data storage (e.g., ~/Library/Application Support/gmail-cleaner).
  • Setup API: Added POST /api/setup to handle credentials.json uploads via the UI.
  • Frontend: Implemented a "First Run" Setup View that prompts for credentials if missing.
  • Packaging: Added pyinstaller spec and build_app.py script.
  • CI/CD: Added GitHub Actions workflow to build binaries for Windows, macOS, and Linux.

How to Test

  1. Run uv run python build_app.py.
  2. Launch the executable from dist/gmail-cleaner/.
  3. On a fresh machine, it should prompt for credentials.json.
  • Core: refactored app/core/config.py to use platformdirs; added Settings.data_dir, ensure data_dir creation with fallback to CWD, and resolve credentials/token paths under data_dir.
  • Setup API: added app/api/setup.py with POST /api/setup to accept, validate (requires "installed" or "web") and store credentials.json; exported setup_router from app.api and included it in the app.
  • Frontend: added a "First Run" Setup View in templates/index.html (duplicated in two locations) and client-side flows in static/js/auth.js (handleFileSelect, uploadCredentials); auth checks can surface the setup view when credentials are missing.
  • Packaging: added gmail-cleaner.spec and build_app.py to produce PyInstaller desktop executables (includes static/templates data and required hidden imports); added resource_path() and frozen-aware cache-bust behavior in app/main.py for bundled runs; adjusted resource path logic for packaged apps.
  • Runtime behavior & security: app now enters a user-facing Setup Mode when credentials are missing (logs WARNING rather than exiting), and server binds to HOST env var (default 127.0.0.1) for standalone builds; POST /api/setup prevents overwrites (returns 409) and setup responses avoid leaking absolute paths.
  • CI/CD & tooling: added .github/workflows/build.yml to build ZIP artifacts for Linux/Windows/macOS and publish on tags; updated .gitignore to exclude PyInstaller build/ and dist/; added platformdirs and python-multipart to main deps and pyinstaller to dev deps in pyproject.toml.
  • Docs & developer guides: updated README with standalone ZIP distribution, build output path (dist/gmail-cleaner/), platform support and detailed standalone build/install instructions; added multiple AGENTS.md docs and GMAIL_SERVICES_ANALYSIS.md describing architecture and recommended refactors.

new onboarding ui:
image

@coderabbitai

coderabbitai Bot commented Feb 12, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds a cross-platform GitHub Actions build workflow and PyInstaller packaging, a FastAPI setup endpoint plus frontend upload UI for uploading credentials.json, introduces a user data directory with PyInstaller-aware path handling, and adds build tooling and multiple documentation files.

Changes

Cohort / File(s) Summary
CI / Build
​.github/workflows/build.yml, build_app.py, gmail-cleaner.spec, pyproject.toml, .gitignore
New GitHub Actions workflow (ubuntu/windows/macos), PyInstaller spec and build script, added build/dev deps, and .gitignore updated to exclude PyInstaller artifacts.
Setup API & Exports
app/api/setup.py, app/api/__init__.py
New POST /api/setup endpoint to accept/validate and persist credentials.json; setup_router exported from app.api and included in app.
Config & Startup
app/core/config.py, app/main.py, main.py
Adds data_dir via platformdirs, resolves credentials/token under data_dir, introduces resource_path for frozen builds, computes startup cache-bust, and adjusts startup messaging and host binding.
Frontend setup UI & JS
static/js/auth.js, templates/index.html, static/css/components.css
Adds Setup view markup and styles, client handlers handleFileSelect and uploadCredentials, surfaces setup flow when credentials are missing, and wires upload to /api/setup.
Packaging / Spec
gmail-cleaner.spec, build_app.py
Spec includes static/templates data and hidden imports; build script cleans artifacts, adjusts macOS PATH, and invokes PyInstaller to produce standalone builds.
Docs & Guides
AGENTS.md, GMAIL_SERVICES_ANALYSIS.md, app/services/gmail/AGENTS.md, static/js/AGENTS.md, tests/AGENTS.md, README.md
Adds architecture docs, Gmail services analysis, frontend patterns, testing guidance, and expanded README with standalone build/install instructions.
Tests / Misc
tests/AGENTS.md
New testing guidance and fixture documentation; no test code changes.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant Browser as Client (browser)
participant UI as Frontend UI
participant Server as FastAPI (setup_router)
participant Disk as Data Dir (disk)
Browser->>UI: select credentials.json
UI->>Browser: build FormData with file
UI->>Server: POST /api/setup (multipart/form-data)
Server->>Server: parse & validate JSON ("installed" or "web")
Server->>Disk: write file to data_dir/credentials.json
Server-->>UI: 200 OK (message + path)
UI-->>Browser: reload / proceed

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

enhancement

Suggested reviewers

  • adhi85

Poem

🏗️ From source to exe the builders sing,
Upload your keys, let setup bells ring,
Data dir chosen, frozen paths made right,
Docs and scripts guide the overnight,
Small steps to launch from local to flight.

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding standalone app packaging capabilities and a user-friendly setup flow for the application.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai 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.

Actionable comments posted: 10

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
main.py (1)

59-59: ⚠️ Potential issue | 🟠 Major

Binding to 0.0.0.0 exposes the desktop app on all network interfaces.

For a standalone desktop application, binding to 127.0.0.1 (localhost) is safer — it prevents other devices on the network from accessing the app (and its unauthenticated setup endpoint). Reserve 0.0.0.0 for Docker/cloud deployments.

Proposed fix
-    uvicorn.run(app, host="0.0.0.0", port=port, log_level="warning")
+    host = os.environ.get("HOST", "127.0.0.1")
+    uvicorn.run(app, host=host, port=port, log_level="warning")
🤖 Fix all issues with AI agents
In `@AGENTS.md`:
- Around line 78-81: Update the Data Persistence note to reflect the
platformdirs refactor: mention that token.json is now stored using platformdirs
(via the refactored config.py) in the OS-specific user data directory (e.g.,
macOS: ~/Library/Application Support/gmail-cleaner), and also note the
Docker-mode behavior where token.json remains in ./data/ when running with a
volume mount; reference config.py and the use of platformdirs and token.json so
readers can find the implementation.
- Around line 13-14: Update AGENTS.md to document the new setup route: add
"setup.py" to the app/api/ routes list alongside "actions.py" and "status.py"
and add an entry for the /api/setup endpoint in the "Where to Look" table (or
wherever routes are enumerated) so the doc references the new app/api/setup.py
file and briefly describes the /api/setup endpoint's purpose.
- Around line 3-5: The AGENTS.md file contains a literal placeholder "{DYNAMIC}"
in the Generated/Commit header that is not being replaced by any hook; remove or
automate it by either replacing "{DYNAMIC}" with a static commit hash or
timestamp, or add a CI/pre-commit step to inject the current commit id (or
render the date) into the Generated/Commit field (update the generation step
that writes the "Generated:" block to substitute the {DYNAMIC} token), ensuring
the file no longer contains the unreplaced "{DYNAMIC}" placeholder.

In `@app/api/setup.py`:
- Around line 50-54: The API currently returns the full server filesystem path
via settings.credentials_file in the response object; change the upload
credentials endpoint so it no longer leaks the absolute path — either remove the
"path" field entirely or replace it with only the filename using
os.path.basename(settings.credentials_file). Update the return value where the
dict is constructed (the block that currently returns {"message": "Credentials
uploaded successfully", "path": settings.credentials_file}) to return a safe
value (e.g., {"message": "Credentials uploaded successfully"} or {"message":
"...", "file": os.path.basename(settings.credentials_file")}) and ensure any
references to settings.credentials_file in the response are removed or
sanitized.
- Around line 19-20: The POST /setup handler implemented by
router.post("/setup") in setup_credentials currently allows unauthenticated
overwrites of credentials.json; modify setup_credentials to first check for the
existence of the stored credentials (e.g., check if credentials.json or
equivalent persistent store exists/contains valid data) and immediately reject
the request with a 403/409 response when credentials are already present, only
proceeding to accept the uploaded UploadFile and write credentials when no
credentials exist; ensure the response is clear (forbidden/already configured)
and do not change credentials if the guard fails.

In `@app/core/config.py`:
- Around line 52-72: The constructor __init__ currently overwrites any
user-provided DATA_DIR set by super().__init__ by unconditionally setting
self.data_dir; change __init__ to first call super().__init__(**kwargs) and then
only perform the auto-detection (the os.path.exists("/app/data") check and
platformdirs.user_data_dir call) if self.data_dir is not already set/truthy, and
likewise only attempt os.makedirs/fallback to os.getcwd() when you have chosen
an auto-detected path—this preserves user-specified self.data_dir while keeping
the existing fallback behavior.

In `@build_app.py`:
- Around line 1-4: Remove the unused top-level import of the module symbol "sys"
in the file (it's imported but never referenced); simply delete the "import sys"
line so only the used imports (PyInstaller.__main__, shutil, os) remain to avoid
the unused-import warning.

In `@gmail-cleaner.spec`:
- Around line 5-10: The PyInstaller Analysis hiddenimports list in the
Analysis(...) call is missing the multipart module required for FastAPI
UploadFile; update the hiddenimports argument (the list passed to the Analysis
constructor that currently contains 'uvicorn', 'fastapi', 'jinja2.ext',
'pydantic_settings') to include 'multipart' so python-multipart is bundled for
main.py's /api/setup file uploads.

In `@static/js/auth.js`:
- Around line 28-35: The handler handleFileSelect currently hides the
".setup-actions" element when a file is picked which prevents re-selecting if
upload fails; update handleFileSelect to either not hide ".setup-actions" or add
a visible "Change file" affordance next to the displayed file (elements:
'fileName', 'fileInfo', '.setup-actions') and ensure the upload failure path
(the code that runs on failed upload around lines handling response errors)
restores or re-enables the select button so users can pick a different
credentials.json; make the change so the UI always presents a way to choose a
new file after a failed upload.
- Around line 10-17: The fetch to '/api/web-auth-status' currently parses the
body without checking HTTP status; update the logic around webStatusResp and
webStatus so you first check webStatusResp.ok and if false handle it
(throw/return/show an error) instead of proceeding to webStatus = await
webStatusResp.json() and potentially calling GmailCleaner.UI.showView('setup');
ensure you reference webStatusResp (the Response), inspect response.ok, and only
parse JSON into webStatus when ok, surfacing or logging non-2xx responses rather
than silently treating them as a missing-setup case.
🧹 Nitpick comments (15)
app/services/gmail/AGENTS.md (1)

7-16: Add a language identifier to the fenced code block.

The fenced code block starting at Line 7 has no language specified, which triggers an MD040 lint warning. Use ```text for plain-text structure diagrams.

Proposed fix
-```
+```text
 app/services/gmail/
tests/AGENTS.md (1)

7-17: Add a language identifier to the fenced code block (same MD040 lint issue).

Use ```text for the directory tree.

app/api/__init__.py (1)

5-7: LGTM — new router export is clean.

Ruff flags __all__ as unsorted (RUF022). Consider sorting alphabetically for consistency:

-__all__ = ["status_router", "actions_router", "setup_router"]
+__all__ = ["actions_router", "setup_router", "status_router"]
static/js/AGENTS.md (2)

7-17: Add a language identifier to the fenced code block.

Same MD040 pattern — use ```text for the directory structure block.


38-43: Add blank lines around the table (MD058).

Markdown tables should be surrounded by blank lines for consistent rendering across parsers.

app/core/config.py (1)

67-72: Silent fallback to cwd on OSError — consider logging a warning.

If directory creation fails and the app silently falls back to os.getcwd(), credentials could end up in an unexpected location. A log message would help users debug this.

Proposed fix
         try:
             os.makedirs(self.data_dir, exist_ok=True)
         except OSError:
-            # Fallback to current directory if we can't create the data dir
-            # This might happen in some restricted environments
+            import logging
+            logging.getLogger(__name__).warning(
+                "Could not create data dir '%s', falling back to cwd", self.data_dir
+            )
             self.data_dir = os.getcwd()
app/api/setup.py (2)

29-32: Chain the exception for proper tracebacks (raise ... from).

When re-raising as HTTPException inside an except block, use from to preserve the original traceback context.

Proposed fix
         except json.JSONDecodeError:
-            raise HTTPException(
+            raise HTTPException(
                 status_code=status.HTTP_400_BAD_REQUEST,
                 detail="Invalid JSON file",
-            )
+            ) from None

58-63: Chain the exception and avoid leaking internal error details.

str(e) in the response can expose internal paths or stack details. Use raise ... from e for proper chaining, and consider a generic message for the client.

Proposed fix
     except Exception as e:
         logger.exception("Error uploading credentials")
         raise HTTPException(
             status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
-            detail=f"Failed to upload credentials: {str(e)}",
-        )
+            detail="Failed to upload credentials. Check server logs for details.",
+        ) from e
static/js/auth.js (1)

37-68: Consider basic client-side validation of the selected file.

The accept=".json" attribute on the file input is only a hint — browsers don't enforce it strictly. A quick sanity check (e.g., verifying the file name ends with .json or attempting a JSON.parse of its contents) before uploading would give faster feedback and avoid a round-trip for obviously wrong files.

app/main.py (1)

22-30: Replace bare except Exception with getattr for cleaner _MEIPASS access.

The try/except Exception is flagged by Ruff (BLE001) and is broader than necessary. The standard PyInstaller idiom can be expressed more precisely with getattr, which avoids the catch-all entirely.

Proposed fix
 def resource_path(relative_path):
     """Get absolute path to resource, works for dev and for PyInstaller"""
-    try:
-        # PyInstaller creates a temp folder and stores path in _MEIPASS
-        base_path = sys._MEIPASS
-    except Exception:
-        base_path = os.path.abspath(".")
-
+    # PyInstaller creates a temp folder and stores path in _MEIPASS
+    base_path = getattr(sys, '_MEIPASS', os.path.abspath("."))
     return os.path.join(base_path, relative_path)
build_app.py (1)

16-18: PATH override is macOS-specific but applied unconditionally.

The /usr/bin prefix fix targets a macOS issue where /usr/local/bin/arch shadows /usr/bin/arch. On other platforms this is a harmless no-op, but guarding it makes the intent clearer and avoids accidentally shadowing user-installed tools on Linux.

Proposed platform guard
+import sys
+
     # Fix for macOS where /usr/local/bin/arch might shadow /usr/bin/arch
     # PyInstaller relies on /usr/bin/arch behavior
-    os.environ["PATH"] = "/usr/bin:" + os.environ.get("PATH", "")
+    if sys.platform == "darwin":
+        os.environ["PATH"] = "/usr/bin:" + os.environ.get("PATH", "")
.github/workflows/build.yml (2)

14-18: Consider fail-fast: false so all platform builds report independently.

By default, GitHub Actions cancels remaining matrix jobs when one fails. Adding fail-fast: false lets you see which platforms succeed and which don't — useful when debugging platform-specific packaging issues.

Proposed fix
     strategy:
+      fail-fast: false
       matrix:

35-39: Add if-no-files-found: error to catch silent build failures.

If build_app.py exits 0 but produces no output (e.g., spec misconfiguration), upload-artifact will quietly upload an empty artifact. Failing explicitly on missing files surfaces the issue immediately.

Proposed fix
     - name: Upload Artifacts
       uses: actions/upload-artifact@v4
       with:
         name: gmail-cleaner-${{ matrix.os }}
         path: dist/gmail-cleaner/
+        if-no-files-found: error
+        retention-days: 30
templates/index.html (1)

176-215: Setup view looks good functionally; consider extracting inline styles to CSS.

The setup view provides clear guidance and a clean upload flow. However, it uses ~10 inline style attributes (lines 187, 197, 198, 204–206) while the rest of the template relies on CSS classes. Moving these to a stylesheet (e.g., .setup-actions, .setup-instructions, .file-info) would keep the template consistent and make future restyling easier.

gmail-cleaner.spec (1)

3-3: Remove PyInstaller 6.x deprecated parameters for cleaner spec file.

block_cipher and the cipher parameter were removed in PyInstaller 6.0.0 (no longer work). win_no_prefer_redirects and win_private_assemblies are deprecated and will raise errors if set to True. Clean these up when upgrading PyInstaller or use a version that supports them.

Proposed cleanup
-block_cipher = None
-
 a = Analysis(
     ['main.py'],
     pathex=[],
     binaries=[],
     datas=[('static', 'static'), ('templates', 'templates')],
     hiddenimports=['uvicorn', 'fastapi', 'jinja2.ext', 'pydantic_settings'],
     hookspath=[],
     hooksconfig={},
     runtime_hooks=[],
     excludes=[],
-    win_no_prefer_redirects=False,
-    win_private_assemblies=False,
-    cipher=block_cipher,
     noarchive=False,
 )
-pyz = PYZ(a.pure, a.zipped_data, cipher=block_cipher)
+pyz = PYZ(a.pure, a.zipped_data)

Comment thread AGENTS.md Outdated
Comment thread AGENTS.md Outdated
Comment thread AGENTS.md Outdated
Comment thread app/api/setup.py
Comment thread app/api/setup.py
Comment thread app/core/config.py
Comment thread build_app.py
Comment thread gmail-cleaner.spec Outdated
Comment thread static/js/auth.js Outdated
Comment thread static/js/auth.js

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @.github/workflows/build.yml:
- Around line 52-56: Update the GitHub Action release step to use the maintained
major by changing the action from softprops/action-gh-release@v1 to
softprops/action-gh-release@v2 and ensure the workflow grants the release step
the required token scope by adding a top-level permissions block that includes
contents: write for GITHUB_TOKEN; keep the existing with: files:
gmail-cleaner-${{ matrix.os }}.zip and the if: startsWith(github.ref,
'refs/tags/') condition intact so the Release step uses the new action and has
explicit write permission to create releases.
🧹 Nitpick comments (1)
.github/workflows/build.yml (1)

14-18: Consider fail-fast: false so one OS failure doesn't cancel the others.

With the default fail-fast: true, a build failure on e.g. Ubuntu will cancel the in-progress Windows and macOS builds. For a packaging matrix you typically want all platforms to finish independently so you can triage per-OS issues.

Suggested change
     strategy:
+      fail-fast: false
       matrix:

Comment thread .github/workflows/build.yml
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

@coderabbitai coderabbitai 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.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@README.md`:
- Around line 417-436: In the "🛠️ Development & Building" section of the
README, update the build output path sentence so it correctly points to the
PyInstaller output folder: change the line that currently reads "The built
application will be available in the `dist/` folder." to "The built application
will be available in the `dist/gmail-cleaner/` folder." so the documentation
matches the actual build output.
- Around line 14-24: The README's "Download & Install" section incorrectly lists
.exe/.dmg/AppImage installers; update README.md to reflect that the CI Build.yml
currently produces gmail-cleaner-*.zip artifacts containing PyInstaller-built
executables (instructions: list ZIP download, how to unzip and run the bundled
executable, and how to upload credentials.json), or alternatively modify the CI
(Build.yml) to add packaging steps (Inno Setup, create-dmg, appimagetool) to
actually produce the claimed .exe/.dmg/AppImage formats—pick one approach and
make README.md and Build.yml consistent.
🧹 Nitpick comments (2)
README.md (2)

82-84: Clarify prerequisites by installation method.

The prerequisites section lists Docker and Python as requirements, but users choosing the standalone app (Option A) don't need either. Consider restructuring this section to clarify which prerequisites apply to each installation option:

 ## Prerequisites
 
-- **Docker**: [Docker Desktop](https://www.docker.com/products/docker-desktop/)
-- **Local (Python)**: [Python 3.9+](https://www.python.org/downloads/) and [uv](https://docs.astral.sh/uv/getting-started/installation/)
+- **Standalone App (Option A)**: No prerequisites - just download and run
+- **Docker (Option B)**: [Docker Desktop](https://www.docker.com/products/docker-desktop/)
+- **Python Development (Option C)**: [Python 3.9+](https://www.python.org/downloads/) and [uv](https://docs.astral.sh/uv/getting-started/installation/)

42-51: Consider adding standalone app to platform support table.

The platform support table shows Docker and Python options but omits the new standalone app (now Option A - Recommended). Adding it would provide complete platform support information:

-| Platform | Docker | Local (Python) |
-|----------|--------|----------------|
+| Platform | Standalone App | Docker | Local (Python) |
+|----------|----------------|--------|----------------|
+| Linux (x86_64) | ✅ | Native | Native |
+| Windows (x86_64) | ✅ | Native | Native |
+| macOS Intel | ✅ | Native | Native |
+| macOS Apple Silicon (M1/M2/M3/M4) | ✅ | Native | Native |

Comment thread README.md
Comment thread README.md
karan-vk and others added 8 commits February 12, 2026 17:00
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Address CodeRabbit review: bind to localhost, validate creds overwrite, safe config fallback
Address CodeRabbit review: macOS path guard, multipart support, resource path logic
Address CodeRabbit review: retry upload, file type check, move inline styles to CSS
- Update build output path to dist/gmail-cleaner/
- Clarify ZIP archive downloads instead of installers
- Update prerequisites for standalone app
- Add standalone app column to platform support table

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
@karan-vk

Copy link
Copy Markdown
Collaborator Author

Thank you for the detailed review! I've addressed all the feedback in the latest commits:

🛡️ Backend & Security

  • Secure Binding: now binds to by default for the standalone app (safer for desktop use). Added env var support for Docker/Cloud overrides.
  • Setup API Protection: now checks if already exists and returns to prevent overwrites.
  • Data Leakage Prevention: The setup response no longer returns the absolute file path. 500 errors use generic messages.
  • Config Safety: now correctly prioritizes user-defined and warns on fallback.

📦 Build & Packaging

  • Cross-Platform Fix: Guarded the macOS-specific override in .
  • Dependencies: Added to hidden imports (required for file uploads).
  • Clean Spec: Removed deprecated params from the spec file.
  • Resource Path: Updated to use the standard check for reliable resource loading.

🎨 Frontend & UX

  • Better Upload UX: Added client-side validation for files.
  • Retry Capability: The setup UI now allows re-selecting the file if an upload fails (doesn't hide the input).
  • Refactoring: Moved inline styles from the Setup View in to .

⚙️ CI/CD

  • Optimized Workflow: Added to the build matrix so one platform failure doesn't cancel others.
  • Updated Action: Bumped to .
  • Artifacts: Added error checking () for artifact uploads.

📚 Documentation

  • Updated README: Clarified that the Standalone App requires no prerequisites and is distributed as a ZIP. Added "Standalone App" to the platform support table.
  • Formatting: Fixed language identifiers in code blocks.
  • API Docs: Sorted in and documented the new setup route.

Ready for another look! 🚀

@coderabbitai coderabbitai 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.

Actionable comments posted: 3

🤖 Fix all issues with AI agents
In `@gmail-cleaner.spec`:
- Around line 3-16: The hiddenimports list in the Analysis object is missing
packages that are imported at runtime: add "platformdirs" (used by
app/core/config.py), add the Google libraries "google.auth",
"google_auth_oauthlib", and "googleapiclient" (used by app/services/auth.py),
and change the entry "python-multipart" to the actual import name "multipart";
update the hiddenimports array in the Analysis(...) call accordingly so
PyInstaller will bundle these modules.
- Line 8: The hiddenimports list currently contains the incorrect module name
'python-multipart'; change that entry to the actual import name
'python_multipart' so PyInstaller includes the correct module at build time
(update the hiddenimports array where it lists ['uvicorn', 'fastapi',
'jinja2.ext', 'pydantic_settings', 'python-multipart'] to use 'python_multipart'
instead). Ensure you only change the string token and keep the rest of the
hiddenimports list intact.

In `@tests/AGENTS.md`:
- Around line 19-23: Update the docs to reference the actual Settings attribute
name: replace `settings.WEB_AUTH` with `settings.web_auth` in the KEY FIXTURES
(`conftest.py`) section so it matches the Settings class in `config.py` and the
mock target used by the `mock_gmail_auth` fixture.
🧹 Nitpick comments (8)
app/core/config.py (1)

70-78: Fallback to os.getcwd() on directory creation failure is reasonable but worth a log level bump.

If makedirs fails, the app silently falls back to cwd. This is fine for resilience, but cwd may not be writable either (e.g., inside a packaged app launched from /). Consider logging the fallback path so users can diagnose issues.

The current logging.warning is adequate for now — just flagging the edge case.

AGENTS.md (2)

12-13: Parenthetical route list on line 13 still only mentions actions.py, status.py.

The "Where to Look" table on line 35 correctly includes setup.py, but the inline note on line 13 omits it. Minor consistency nit — consider updating to (actions.py, status.py, setup.py).


56-60: Anti-pattern "No try/except" conflicts with the new setup endpoint.

Line 58 says to avoid try/except in favor of HTTPException, but app/api/setup.py legitimately uses try/except to catch json.JSONDecodeError and generic exceptions before raising HTTPException. Consider rewording this to clarify the intent — e.g., "Don't use bare try/except to swallow errors silently; always raise HTTPException or a custom exception."

app/api/setup.py (2)

35-39: Chain the exception with raise ... from for proper traceback context.

Ruff B904 is valid here. When re-raising a different exception inside an except block, use from to preserve the exception chain for debugging.

Proposed fix
         try:
             data = json.loads(content)
         except json.JSONDecodeError:
             raise HTTPException(
                 status_code=status.HTTP_400_BAD_REQUEST,
                 detail="Invalid JSON file",
-            )
+            ) from None

29-60: Consider a size limit on the uploaded file.

There's no guard against excessively large uploads. A malicious or accidental multi-GB upload would be read entirely into memory on line 30. A credentials.json file is typically a few KB — adding a reasonable size check (e.g., 1 MB) would harden this endpoint.

Proposed fix — add after line 30
         content = await file.read()
+
+        # credentials.json should be small
+        if len(content) > 1_000_000:  # 1 MB
+            raise HTTPException(
+                status_code=status.HTTP_400_BAD_REQUEST,
+                detail="File too large. credentials.json should be a few KB.",
+            )
static/css/components.css (1)

1071-1092: Hardcoded colors break theming consistency.

The rest of this stylesheet uses CSS custom properties (var(--hover-bg), var(--border-color), var(--text-secondary), etc.), but these new rules use hardcoded hex values (#f8f9fa, #dadce0, #5f6368). This will look wrong if/when a dark theme is applied. The existing .web-auth-steps block (line 601) is a near-identical pattern that already uses the correct variables.

♻️ Proposed fix: use existing CSS custom properties
 .setup-instructions {
     margin-top: 30px;
     text-align: left;
     max-width: 500px;
     margin-left: auto;
     margin-right: auto;
-    background: `#f8f9fa`;
+    background: var(--hover-bg);
     padding: 20px;
-    border-radius: 8px;
-    border: 1px solid `#dadce0`;
+    border-radius: var(--radius-md);
+    border: 1px solid var(--border-color);
 }

 .setup-instructions ol {
     padding-left: 20px;
-    color: `#5f6368`;
+    color: var(--text-secondary);
     margin-bottom: 0;
 }
templates/index.html (1)

188-188: Consider adding aria-label to the hidden file input for screen reader users.

The hidden input has no label association. While the button text provides visual context, screen readers that focus the input (e.g., via keyboard navigation in some browsers) won't have a description.

Proposed fix
-<input type="file" id="credentialsFile" accept=".json" style="display: none;" onchange="GmailCleaner.Auth.handleFileSelect(this)">
+<input type="file" id="credentialsFile" accept=".json" style="display: none;" aria-label="Select credentials.json file" onchange="GmailCleaner.Auth.handleFileSelect(this)">
static/js/AGENTS.md (1)

1-45: Useful developer guide — good addition for onboarding.

The architecture overview, conventions, and anti-patterns sections clearly communicate the project's frontend philosophy. This will save contributors time.

A few minor markdown lint fixes to keep things clean:

Fix markdown lint warnings (MD058)

Add blank lines around the table at line 40, per markdownlint MD058:

 ## COMMON TASKS
 
+
 | Task | implementation |
 |------|----------------|
 | **Add View** | Add HTML to `index.html`, add nav in `main.js`, add show() in `ui.js`. |
 | **New API Call** | Use `fetch('/api/...')`, handle errors, update UI. |
 | **Show Toast** | `GmailCleaner.UI.showToast(message, type)`. |
+

Comment thread gmail-cleaner.spec
Comment thread gmail-cleaner.spec Outdated
Comment thread tests/AGENTS.md Outdated
@karan-vk

Copy link
Copy Markdown
Collaborator Author

Thank you for the detailed review! I have addressed all the feedback in the latest commits.

(Reposting with fixed formatting):

🛡️ Backend & Security

  • Secure Binding: main.py now binds to 127.0.0.1 by default for the standalone app (safer for desktop use). Added HOST env var support for Docker/Cloud overrides.
  • Setup API Protection: POST /api/setup now checks if credentials.json already exists and returns 409 Conflict to prevent overwrites.
  • Data Leakage Prevention: The setup response no longer returns the absolute file path. 500 errors use generic messages.
  • Config Safety: app/core/config.py now correctly prioritizes user-defined DATA_DIR and warns on fallback.

📦 Build & Packaging

  • Cross-Platform Fix: Guarded the macOS-specific PATH override in build_app.py.
  • Dependencies: Added python-multipart to gmail-cleaner.spec hidden imports (required for file uploads).
  • Clean Spec: Removed deprecated cipher params from the spec file.
  • Resource Path: Updated app/main.py to use the standard sys._MEIPASS check for reliable resource loading.

🎨 Frontend & UX

  • Better Upload UX: Added client-side validation for .json files.
  • Retry Capability: The setup UI now allows re-selecting the file if an upload fails (doesnt hide the input).
  • Refactoring: Moved inline styles from the Setup View in index.html to static/css/components.css.

⚙️ CI/CD

  • Optimized Workflow: Added fail-fast: false to the build matrix so one platform failure doesnt cancel others.
  • Updated Action: Bumped softprops/action-gh-release to v2.
  • Artifacts: Added error checking (if-no-files-found: error) for artifact uploads.

📚 Documentation

  • Updated README: Clarified that the Standalone App requires no prerequisites and is distributed as a ZIP. Added "Standalone App" to the platform support table.
  • Formatting: Fixed language identifiers in AGENTS.md code blocks.
  • API Docs: Sorted __all__ in app/api/__init__.py and documented the new setup route.

Ready for another look! 🚀

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@README.md`:
- Around line 20-22: Update README.md lines describing release artifacts: change
the macOS entry to indicate it contains a "binary" (not a .app) to match
gmail-cleaner.spec (no BUNDLE), and update the example archive filenames to
match CI output by using the matrix suffix pattern (use names like
gmail-cleaner-{matrix.os}.zip — e.g., gmail-cleaner-windows-latest.zip,
gmail-cleaner-macos-14.zip, gmail-cleaner-ubuntu-latest.zip) so the README
reflects actual release artefacts and filenames.
🧹 Nitpick comments (3)
README.md (3)

83-83: Consider clarifying "No prerequisites required."

While technically accurate (the packaged executable runs without installing Python/Docker), users still need to obtain credentials.json from Google Cloud Console. The current phrasing works since the Setup section covers credentials separately and the Standalone App uploads them via UI, but you could optionally add a clarifying note.

Optional clarification
-- **Standalone App**: No prerequisites required.
+- **Standalone App**: No prerequisites required (credentials.json is uploaded via the app's UI).

150-153: Document the retry capability for credentials upload.

The PR objectives mention "retry capability on upload failure" as an implemented feature, but the documentation doesn't explain what happens if the upload fails or how users can retry. Adding this information will improve the user experience when troubleshooting upload issues.

Suggested addition after step 3
 3.  When prompted, upload your `credentials.json` file.
+    > **Note**: If the upload fails, you can retry by clicking the upload button again. The app will validate the file format before accepting it.
 4.  Click **"Sign In"** and follow the OAuth flow in your browser.

23-23: Consider aligning wording with line 152 for consistency.

Line 23 says "Follow the on-screen instructions to upload your credentials.json" while line 152 more specifically says "When prompted, upload your credentials.json file." Using consistent phrasing makes the documentation easier to follow.

Optional consistency improvement
-3.  Extract the ZIP archive and run the executable. Follow the on-screen instructions to upload your `credentials.json`.
+3.  Extract the ZIP archive and run the executable. When prompted, upload your `credentials.json` file.

Comment thread README.md Outdated
- Add CODE_OF_CONDUCT.md with Contributor Covenant v2.1
- Add SECURITY.md with vulnerability reporting policy
- Add ROADMAP.md with project vision and future plans
- Add dependabot.yml for automated dependency updates
- Enhance README with 12 badges (coverage, tests, issues, etc.)
- Expand CONTRIBUTING with quick start, testing guide, PR checklist
- Improve PR template with type checkboxes and detailed checklist
- Update issue template config with security and discussion links
Add to .gitignore as these are AI assistant context files
- Add 3-step wizard UI with visual progress indicator
- Implement drag-and-drop file upload with visual feedback
- Add card-based instruction layout with direct links to Google Cloud
- Replace alerts with toast notifications for better UX
- Add success states and smooth animations
- Improve mobile responsiveness
- Add GmailCleanerError base class with structured error codes
- Create specific exceptions: NetworkError, AuthError, GmailApiError
- Add QuotaExceededError, ResourceNotFoundError, ValidationError
- Implement error_handler.py with decorators for Gmail API error handling
- Add @handle_gmail_errors decorator for automatic error translation
- Add @with_retry decorator with exponential backoff for transient failures

Part of roadmap: Better error handling and user feedback
- Add BATCH_SIZE setting (default: 100, max: 200)
- Add MAX_WORKERS setting for parallel processing (default: 4)
- Add CHUNK_SIZE for streaming mode (default: 1000 emails)
- Add CHECKPOINT_INTERVAL for progress persistence (default: 5000)
- Add enable_streaming flag for memory-efficient processing
- Add adaptive_rate_limit for dynamic rate limiting

Part of roadmap: Performance optimizations for large inboxes
- Add streaming mode for processing >1000 emails without loading all IDs
- Implement chunked processing with configurable chunk sizes
- Add checkpoint/resume functionality for long operations
- Use memory-efficient UnsubscribeData class with __slots__
- Add _scan_streaming() and _scan_standard() methods
- Implement adaptive rate limiting during batch processing
- Support parallel processing with ThreadPoolExecutor

Part of roadmap: Performance optimizations for large inboxes (100k+ emails)
- Add @handle_gmail_errors decorator to archive operations
- Add error handling to delete operations with proper logging
- Integrate retry logic in label management operations
- Add error handling to mark-read operations

Part of roadmap: Better error handling and user feedback
- Update action endpoints to catch custom exceptions
- Return appropriate HTTP status codes (401, 429, 502, etc.)
- Add user-friendly error messages with actionable guidance
- Include error codes for client-side error handling

Part of roadmap: Better error handling and user feedback
- Create Notifications module with success, error, warning, info types
- Add animated toast notifications with icons
- Include CSS styles with proper positioning and animations
- Support auto-dismiss and manual close functionality

Part of roadmap: Better error handling and user feedback
- Update UI module to delegate toast calls to Notifications system
- Add notifications.js script to base template
- Maintain backward compatibility with existing showToast calls
- Ensure global availability of Notifications module

Part of roadmap: Better error handling and user feedback
- Add test_api_flows.py for end-to-end API workflows (scan, auth, delete)
- Add test_gmail_operations.py for Gmail service operations
- Add test_error_scenarios.py for error handling flows
- Cover rate limiting, API errors, and auth failures
- Test batch efficiency and concurrent operations
- Include 10 new integration tests, all passing

Part of roadmap: Integration tests suite
- Integration tests suite: 10 new tests added
- Performance optimizations: Streaming mode, chunked processing
- Error handling: Custom exceptions, retry logic, toast notifications

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

this abvoe specc and anaysis file are created by agents to make changes right and yes they will be useful can we move to specific folder so lets keep these under

.gituhb/agent/reference

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done

- Change macOS entry from .app to binary to match gmail-cleaner.spec
- Update filenames to use matrix suffix pattern:
  - gmail-cleaner-windows-latest.zip
  - gmail-cleaner-macos-14.zip
  - gmail-cleaner-ubuntu-latest.zip

Reflects actual CI build artifact names.
Change 'python-multipart' to 'python_multipart' (underscore) in PyInstaller
hiddenimports. Python module names use underscores, not hyphens.
- Add 'Configure Consent Screen' step to setup wizard
- Add 'Add Test Users' step to setup wizard
- Update terminology to 'Google Auth Platform' matching Google Console UI
- Re-order steps: Console -> Project -> API -> Consent -> Test Users -> Credentials -> Download
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.

2 participants