Spiralgang patch 1#2
Conversation
Reviewer's GuideThis PR overhauls the repository by adding a self-contained, browser-based polyglot IDE and complementary GitHub Actions workflows for model vendoring, environment setup, and Android APK builds, along with updated documentation to guide reviewers. Sequence diagram for AI-assisted code generation and refactoring in the IDEsequenceDiagram
actor User
participant Terminal
participant AIOrchestrator
participant FileSystem
User->>Terminal: Enter 'generate <file> <description>'
Terminal->>AIOrchestrator: queryModel('deepseek', description)
AIOrchestrator-->>Terminal: AI-generated code
Terminal->>FileSystem: createOrUpdate(<file>, code)
Terminal-->>User: Notify file generated
User->>Terminal: Enter 'refactor <file> <instructions>'
Terminal->>FileSystem: Read file content
Terminal->>AIOrchestrator: queryModel('deepseek', instructions + code)
AIOrchestrator-->>Terminal: AI-refactored code
Terminal->>FileSystem: Update file with new code
Terminal-->>User: Notify file refactored
Class diagram for core IDE logic and AI orchestrationclassDiagram
class AIOrchestrator {
+models: dict
+queryModel(modelName, prompt)
+consensusQuery(prompt)
}
class FileSystem {
+fs: object
+navigateTo(pathParts)
+createOrUpdate(pathParts, content, isDir)
}
class Terminal {
+writeToTerminal(text, isHTML)
+updatePrompt()
+processCommand(command)
}
AIOrchestrator <.. Terminal : uses
FileSystem <.. Terminal : uses
Terminal o-- FileSystem
Terminal o-- AIOrchestrator
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @spiralgang, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request fundamentally redefines the project's scope and functionality. It transitions from a repository focused on Android superuser binaries to a sophisticated, browser-based polyglot IDE, complete with integrated AI capabilities and a new CI/CD pipeline for Android application builds. The changes aim to provide a versatile, self-contained development environment accessible directly within a web browser.
Highlights
- Introduction of a comprehensive browser-based IDE: A new single-file HTML application (WebLabs_MobIDE.html) has been added, featuring a polyglot development environment with real Python execution via Pyodide, a full Git client using isomorphic-git, AI-assisted code generation and refactoring, and simulated build/system tools.
- Implementation of a new CI/CD workflow: A GitHub Actions Build Matrix workflow (.github/workflows/Build.yml) has been introduced to automate the building and uploading of Android APKs, supporting both debug and release variants.
- Complete project re-scoping: The project has undergone a significant shift in focus, moving from its previous purpose as an Android su binary repository to a modern, web-centric development environment with integrated AI and build automation. The old README has been removed and replaced with a new README.md reflecting these changes.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- HTML escaping in writeToTerminal may be inconsistent. (link)
- YAML syntax for dependencies and on is invalid. (link)
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `WebLabs_MobIDE.html:319` </location>
<code_context>
+ }
+
+ // A more robust path resolver
+ function resolvePath(pathStr, base = currentPath) {
+ const parts = pathStr.split('/').filter(p => p);
+ let newPath;
+
+ if (pathStr.startsWith('/')) {
+ newPath = ['~', ...pathStr.substring(1).split('/')];
+ } else if (pathStr.startsWith('~/')) {
+ newPath = ['~', ...pathStr.substring(2).split('/')];
+ } else {
+ newPath = [...base];
+ }
+
+ for (const part of parts) {
+ if (part === '..') {
+ if (newPath.length > 1) newPath.pop();
+ } else if (part !== '.' && !pathStr.startsWith('/')) {
+ newPath.push(part);
+ }
+ }
+ return newPath.filter(p => p);
+ }
+
</code_context>
<issue_to_address>
Path resolution may not handle '.' and '..' correctly for absolute paths.
Refactor resolvePath to properly normalize both relative and absolute paths, ensuring '.' is ignored and '..' removes the correct segment in all cases.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
// A more robust path resolver
function resolvePath(pathStr, base = currentPath) {
const parts = pathStr.split('/').filter(p => p);
let newPath;
if (pathStr.startsWith('/')) {
newPath = ['~', ...pathStr.substring(1).split('/')];
} else if (pathStr.startsWith('~/')) {
newPath = ['~', ...pathStr.substring(2).split('/')];
} else {
newPath = [...base];
}
for (const part of parts) {
if (part === '..') {
if (newPath.length > 1) newPath.pop();
} else if (part !== '.' && !pathStr.startsWith('/')) {
newPath.push(part);
}
}
return newPath.filter(p => p);
}
=======
// A robust path resolver that normalizes '.', '..' for both absolute and relative paths
function resolvePath(pathStr, base = currentPath) {
let newPath;
if (pathStr.startsWith('/')) {
newPath = ['~'];
pathStr = pathStr.substring(1);
} else if (pathStr.startsWith('~/')) {
newPath = ['~'];
pathStr = pathStr.substring(2);
} else {
newPath = [...base];
}
const parts = pathStr.split('/').filter(p => p.length > 0);
for (const part of parts) {
if (part === '.') {
// Ignore
continue;
} else if (part === '..') {
if (newPath.length > 1) {
newPath.pop();
}
} else {
newPath.push(part);
}
}
return newPath.filter(p => p);
}
>>>>>>> REPLACE
</suggested_fix>
### Comment 2
<location> `WebLabs_MobIDE.html:305` </location>
<code_context>
+ const ai = new AIOrchestrator();
+
+ // --- Utility Functions ---
+ function writeToTerminal(text, isHTML = true) {
+ const line = document.createElement('div');
+ if (isHTML) line.innerHTML = text.replace(/</g, "<").replace(/>/g, ">").replace(/<span class="([^"]+)">([^<]+)<\/span>/g, '<span class="$1">$2</span>');
+ else line.textContent = text;
+ terminalOutput.appendChild(line);
+ terminalOutput.scrollTop = terminalOutput.scrollHeight;
+ }
+
</code_context>
<issue_to_address>
HTML escaping in writeToTerminal may be inconsistent.
Escaping < and > but then allowing span tags via regex may introduce XSS risks if input is not trusted. Recommend using a proper HTML sanitizer or rendering all output as text unless formatting is required.
</issue_to_address>
### Comment 3
<location> `WebLabs_MobIDE.html:438` </location>
<code_context>
+ else writeToTerminal(`cat: ${args[0]}: Not a file or not found`);
+ },
+ 'pwd': () => writeToTerminal(currentPath.join('/')),
+ 'mkdir': (args) => createOrUpdate(resolvePath(args[0]), {}, true),
+ 'touch': (args) => createOrUpdate(resolvePath(args[0]), ''),
+ 'python': async (args) => {
+ const pyScript = navigateTo(resolvePath(args[0]));
</code_context>
<issue_to_address>
No feedback is provided for mkdir and touch commands.
Consider adding terminal output to indicate success or failure for mkdir and touch, so users receive clear feedback.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
'mkdir': (args) => createOrUpdate(resolvePath(args[0]), {}, true),
'touch': (args) => createOrUpdate(resolvePath(args[0]), ''),
=======
'mkdir': (args) => {
const result = createOrUpdate(resolvePath(args[0]), {}, true);
if (result) {
writeToTerminal(`mkdir: created directory '${args[0]}'`);
} else {
writeToTerminal(`mkdir: failed to create directory '${args[0]}'`);
}
},
'touch': (args) => {
const result = createOrUpdate(resolvePath(args[0]), '');
if (result) {
writeToTerminal(`touch: created file '${args[0]}'`);
} else {
writeToTerminal(`touch: failed to create file '${args[0]}'`);
}
},
>>>>>>> REPLACE
</suggested_fix>
### Comment 4
<location> `WebLabs_MobIDE.html:454` </location>
<code_context>
+ 'git': async (args) => {
</code_context>
<issue_to_address>
Git command error handling is limited for status and log.
Wrap git status and log calls in try/catch blocks to handle errors and inform users when the directory is not a valid git repository.
</issue_to_address>
### Comment 5
<location> `WebLabs_MobIDE.html:533` </location>
<code_context>
+Simulated APK available at: build/outputs/apk/debug/app-debug.apk`);
+ },
+ 'clear': () => terminalOutput.innerHTML = '',
+ 'default': (cmd) => writeToTerminal(`${cmd}: command not found`)
+ };
+
</code_context>
<issue_to_address>
Default command handler may not display unknown command correctly.
Currently, the default handler receives the entire argument array, which may lead to incorrect error messages. Pass only the command name or original input to ensure the error message reflects the unknown command accurately.
</issue_to_address>
### Comment 6
<location> `.github/workflows/setup.yml:3` </location>
<code_context>
+name: Development Environment Setup
+
+dependencies:
+ on:
+ ubuntu:
+ - git-lfs
</code_context>
<issue_to_address>
YAML syntax for dependencies and on is invalid.
'dependencies:' and 'on:' are not valid keys in GitHub Actions workflows. Please update the workflow to use supported syntax.
</issue_to_address>
### Comment 7
<location> `.github/workflows/Build.yml:76` </location>
<code_context>
+ EOF
+ git add .gitattributes
+
+ - name: Strip upstream git metadata from vendored model
+ run: |
+ find ${{ inputs.vendor_path }} -maxdepth 3 -type d -name ".git" -exec rm -rf {} +
+
+ - name: Minimal model hygiene pass
</code_context>
<issue_to_address>
Use of ${{ inputs.vendor_path }} may not be valid in run scripts.
In shell scripts, use $VENDOR_PATH and ensure it's set in the environment, as ${{ inputs.vendor_path }} may not be available. Please verify the variable is accessible in your script context.
</issue_to_address>
### Comment 8
<location> `.github/workflows/Build.yml:112` </location>
<code_context>
+ body: 'Automated vendoring of models and configuration files.'
+ })
+
+ - name: Direct Commit
+ if: ${{ inputs.commit_mode == 'commit' }}
+ run: git merge HEAD
+
+ build:
</code_context>
<issue_to_address>
Direct commit step uses 'git merge HEAD', which is a no-op.
If you want to commit changes directly, replace 'git merge HEAD' with 'git push' to update the target branch.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| function writeToTerminal(text, isHTML = true) { | ||
| const line = document.createElement('div'); | ||
| if (isHTML) line.innerHTML = text.replace(/</g, "<").replace(/>/g, ">").replace(/<span class="([^"]+)">([^<]+)<\/span>/g, '<span class="$1">$2</span>'); | ||
| else line.textContent = text; | ||
| terminalOutput.appendChild(line); | ||
| terminalOutput.scrollTop = terminalOutput.scrollHeight; |
There was a problem hiding this comment.
🚨 issue (security): HTML escaping in writeToTerminal may be inconsistent.
Escaping < and > but then allowing span tags via regex may introduce XSS risks if input is not trusted. Recommend using a proper HTML sanitizer or rendering all output as text unless formatting is required.
| 'git': async (args) => { | ||
| const gitDir = `/${currentPath.slice(1).join('/')}`; | ||
| const corsProxy = 'https://cors.isomorphic-git.org'; | ||
| if (args[0] === 'clone' && args[1]) { | ||
| const url = args[1]; | ||
| const dirName = url.split('/').pop().replace('.git', ''); | ||
| const cloneDir = `/${dirName}`; | ||
| // Check if directory already exists | ||
| if (navigateTo(resolvePath(dirName)) !== undefined) { | ||
| writeToTerminal(`Error: Directory ${cloneDir} already exists. Aborting clone.`); |
There was a problem hiding this comment.
suggestion (bug_risk): Git command error handling is limited for status and log.
Wrap git status and log calls in try/catch blocks to handle errors and inform users when the directory is not a valid git repository.
| Simulated APK available at: build/outputs/apk/debug/app-debug.apk`); | ||
| }, | ||
| 'clear': () => terminalOutput.innerHTML = '', | ||
| 'default': (cmd) => writeToTerminal(`${cmd}: command not found`) |
There was a problem hiding this comment.
nitpick (bug_risk): Default command handler may not display unknown command correctly.
Currently, the default handler receives the entire argument array, which may lead to incorrect error messages. Pass only the command name or original input to ensure the error message reflects the unknown command accurately.
| dependencies: | ||
| on: |
There was a problem hiding this comment.
issue (bug_risk): YAML syntax for dependencies and on is invalid.
'dependencies:' and 'on:' are not valid keys in GitHub Actions workflows. Please update the workflow to use supported syntax.
| - name: Strip upstream git metadata from vendored model | ||
| run: | | ||
| find ${{ inputs.vendor_path }} -maxdepth 3 -type d -name ".git" -exec rm -rf {} + |
There was a problem hiding this comment.
issue (bug_risk): Use of ${{ inputs.vendor_path }} may not be valid in run scripts.
In shell scripts, use
| - name: Direct Commit | ||
| if: ${{ inputs.commit_mode == 'commit' }} | ||
| run: git merge HEAD |
There was a problem hiding this comment.
issue (bug_risk): Direct commit step uses 'git merge HEAD', which is a no-op.
If you want to commit changes directly, replace 'git merge HEAD' with 'git push' to update the target branch.
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Code Review
This pull request introduces an impressive single-file, browser-based polyglot IDE and removes the old README. The new WebLabs_MobIDE.html is a significant piece of work. My review focuses on several critical security vulnerabilities (Cross-Site Scripting), correctness bugs, and maintainability improvements within the new IDE's JavaScript code. I've also included suggestions to improve the formatting of the new README.md file.
| function resolvePath(pathStr, base = currentPath) { | ||
| let newPath; | ||
| if (pathStr.startsWith('/')) { | ||
| newPath = ['~']; | ||
| pathStr = pathStr.substring(1); | ||
| } else if (pathStr.startsWith('~/')) { | ||
| newPath = ['~']; | ||
| pathStr = pathStr.substring(2); | ||
| } else { | ||
| newPath = [...base]; | ||
| } | ||
|
|
||
| const parts = pathStr.split('/').filter(p => p.length > 0); | ||
|
|
||
| for (const part of parts) { | ||
| if (part === '.') { | ||
| // Ignore | ||
| continue; | ||
| } else if (part === '..') { | ||
| if (newPath.length > 1) { | ||
| newPath.pop(); |
There was a problem hiding this comment.
The resolvePath function has a bug in its logic for handling absolute paths, particularly those containing ... For an input like '/a/../b', it incorrectly returns ['~', 'a', '..'] instead of the expected ['~', 'b']. This can cause commands like cd and cat to fail on valid paths.
I've suggested a rewritten, simpler, and more correct implementation of the function.
function resolvePath(pathStr, base = currentPath) {
const parts = pathStr.split('/').filter(p => p && p !== '.');
let newPath;
if (pathStr.startsWith('/')) {
newPath = ['~'];
} else if (pathStr.startsWith('~/')) {
newPath = ['~'];
} else {
newPath = [...base];
}
for (const part of parts) {
if (part === '..') {
if (newPath.length > 1) {
newPath.pop();
}
} else {
newPath.push(part);
}
}
return newPath;
}| > * Run assemble tasks and upload APK artifacts for each variant | ||
| > | ||
| > `.github/workflows/Build.yml` | ||
| > Tips and commands |
There was a problem hiding this comment.
This heading is dangling without any content below it. It should either be removed or populated with the relevant tips and commands.
| } | ||
|
|
||
| async function processCommand(command) { | ||
| // ... command processing logic ... (see below) | ||
| } | ||
|
|
||
| // COMMAND PROCESSOR | ||
| async function processCommand(command) { | ||
| if (isBusy) return; | ||
| isBusy = true; | ||
|
|
||
| const trimmedCommand = command.trim(); | ||
| if (!trimmedCommand) { isBusy = false; return; } | ||
|
|
||
| commandHistory.unshift(trimmedCommand); | ||
| historyIndex = -1; | ||
|
|
||
| writeToTerminal(`${terminalPrompt.innerHTML} ${trimmedCommand}`); | ||
|
|
||
| const parts = trimmedCommand.match(/(?:[^\s"']+|"[^"]*"|'[^']*')+/g)?.map(p => p.replace(/^["']|["']$/g, '')) || []; | ||
| const cmd = parts[0]?.toLowerCase(); | ||
| const args = parts.slice(1); | ||
|
|
||
| try { | ||
| switch (cmd) { | ||
| // ... Cases for all commands ... (see below) | ||
| } | ||
| } catch(e) { | ||
| writeToTerminal(`<span class="text-red-400">Error: ${e.message}</span>`); |
There was a problem hiding this comment.
This processCommand function is defined here and then completely redefined later on lines 537-559. This first implementation is redundant and appears to be leftover from a previous stage of development. It should be removed to avoid confusion and improve code maintainability.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
spiralgang
left a comment
spiralgang
left a comment
There was a problem hiding this comment.
@copilot USE NONPREMIUM REQUESTS AND YOURE FULLY MOBILE AGAIN
1 participant
Summary by Sourcery
Add a standalone browser-based polyglot IDE and corresponding CI workflows and documentation.
New Features:
CI:
Documentation:
Chores: