diff options
Diffstat (limited to 'BUGFIX-SUGGESTIONS.md')
| -rw-r--r-- | BUGFIX-SUGGESTIONS.md | 355 |
1 files changed, 355 insertions, 0 deletions
diff --git a/BUGFIX-SUGGESTIONS.md b/BUGFIX-SUGGESTIONS.md new file mode 100644 index 0000000..1d10fb6 --- /dev/null +++ b/BUGFIX-SUGGESTIONS.md @@ -0,0 +1,355 @@ +# newOS4 Bug Fix Suggestions + +**Generated:** 2026-05-24 +**Status:** Pending Review +**Priority:** High (13 issues across 7 files) + +--- + +## Executive Summary + +| Category | Issues | Files | Severity | +|----------|--------|-------|----------| +| File Structure | 5 | 4 | High | +| Agent System | 5 | 2 | High | +| AI Routing | 2 | 1 | Medium | +| Cache | 1 | 1 | Low | +| **Total** | **13** | **7** | | + +--- + +## 1. File Structure Issues + +### 1.1 Splash Fallback Ext List Incomplete +- **File:** `index.html` +- **Line:** 770 +- **Issue:** Splash fallback hardcodes 5 ext modules but `modules/ext/` directory has 8 files +- **Missing:** `mindmap.js`, `webdav.js`, `skill-pack-core.js` +- **Fix:** + ```javascript + // Before (5 modules) + ['Extensions', ['calc.js','voice.js','cron.js','theme.js','wpm.js'], false] + + // After (8 modules) + ['Extensions', ['calc.js','voice.js','cron.js','theme.js','wpm.js','mindmap.js','webdav.js','skill-pack-core.js'], false] + ``` + +--- + +### 1.2 Preview Iframe Blocks JavaScript +- **File:** `index.html` +- **Line:** 684 +- **Issue:** `<iframe sandbox="allow-same-origin">` missing `allow-scripts` +- **Impact:** HTML previews with JavaScript don't execute +- **Fix:** + ```html + <!-- Before --> + <iframe sandbox="allow-same-origin"></iframe> + + <!-- After --> + <iframe sandbox="allow-same-origin allow-scripts"></iframe> + ``` + +--- + +### 1.3 Dead Code: EXT_FILES Array +- **File:** `module-loader.js` +- **Lines:** 35-37 +- **Issue:** `EXT_FILES` array defined but never referenced (dead code, wrong count) +- **Impact:** Maintenance confusion, wrong module count (6 vs actual 8) +- **Fix:** Remove entirely — ext loading uses separate `extKeys` array on line 159 + +--- + +### 1.4 Packages.json URL Errors +- **File:** `packages.json` +- **Lines:** 7-167 +- **Issues:** + - All 18 package URLs point to `modules/<name>.js` (wrong path) + - Actual files are in `modules/ext/<name>.js` + - 12 packages reference files that don't exist anywhere +- **Impact:** WPM installs fail with 404 errors +- **Fix Options:** + 1. **Clean:** Remove 12 phantom packages, fix 6 real URLs to `modules/ext/` + 2. **Stub:** Create missing files in `modules/ext/` +- **Recommendation:** Option 1 (clean) — reduces bloat + +--- + +### 1.5 Search Uses Intrusive Alert() +- **File:** `help.html` +- **Line:** 1149 +- **Issue:** `alert('No results found')` blocks UI +- **Impact:** Poor UX, modal interruption +- **Fix:** + ```javascript + // Before + if (!found) alert('No results found for: ' + query); + + // After + if (!found) { + const msg = document.getElementById('search-result-msg'); + if (msg) { + msg.textContent = 'No results found for: ' + query; + msg.style.display = 'block'; + setTimeout(() => { msg.style.display = 'none'; }, 3000); + } + } + ``` + +--- + +## 2. Agent System Issues + +### 2.1 Agent Loop Caps at 1 Turn (Not 5) +- **File:** `module-aiassist.js` +- **Lines:** 537-547 +- **Issue:** When model returns text + tool_calls, agent executes tools then `return`s (exits loop) +- **Impact:** Agent stops after 1 tool turn instead of advertised `MAX_TOOL_TURNS = 5` +- **Root Cause:** Follow-up text-response block returns instead of continuing loop +- **Fix:** + ```javascript + // Before (lines 537-548) + if (pendingText) { + const followUp = await this.terminal.ollamaManager.chatWithTools(messages, []); + if (followUp.message && followUp.message.content) { + this.terminal.printAIResponse(followUp.message.content); + return { finalAnswer: followUp.message.content, toolTurns }; // ← exits loop + } + } + continue; + + // After + if (pendingText) { + messages.push({ role: 'assistant', content: pendingText }); + } + continue; // ← loop continues, allows up to 5 turns + ``` + +--- + +### 2.2 edit_module Not Checked as Destructive +- **File:** `module-aiassist.js` +- **Line:** 730 +- **Issue:** `isDestructive()` doesn't include `edit_module` but system prompt (line 227) says it's `(DESTRUCTIVE)` +- **Impact:** AI told to ask permission but code never confirms +- **Fix:** + ```javascript + // Before + isDestructive(name) { + return name === 'write_file' || name === 'delete_file' || + name === 'run_command' || name === 'make_directory'; + } + + // After + isDestructive(name) { + return name === 'write_file' || name === 'delete_file' || + name === 'run_command' || name === 'make_directory' || + name === 'edit_module'; + } + ``` + +--- + +### 2.3 Empty tools:[] Sent to Ollama +- **File:** `module-aiassist.js` +- **Line:** 539 +- **Issue:** `chatWithTools(messages, [])` sends empty tools array +- **Impact:** Some Ollama models reject `tools: []`, triggers unnecessary fallback request +- **Fix:** Remove follow-up call; push text as `assistant` message then `continue` loop (see 2.1) + +--- + +### 2.4 Default Model Blocks Auto-Selection +- **File:** `ollama-manager.js` +- **Lines:** 8, 42-45 +- **Issue:** Default model `'gemma4:31b-cloud'` is truthy, so auto-selection skipped even if model doesn't exist +- **Impact:** Users get silent model errors until manually selecting +- **Fix:** + ```javascript + // Before (lines 42-45) + if (!this.currentModel && this.models.length > 0) { + this.currentModel = this.models[0].name; + // ... + } + + // After + if (!this.currentModel || !this.models.some(m => m.name === this.currentModel)) { + if (this.models.length > 0) { + this.currentModel = this.models[0].name; + // ... + } + } + ``` + +--- + +### 2.5 abort() Misses chat() and chatStream() +- **File:** `ollama-manager.js` +- **Lines:** 70, 140, 176-183 +- **Issue:** `_abortController` only set in `chatWithTools()` (line 195), not in `chat()` or `chatStream()` +- **Impact:** `ai-stop` command only cancels tool-calling requests, not direct chat or streaming +- **Fix:** + ```javascript + // Add to chat() method (around line 70) + this._abortController = new AbortController(); + const signal = this._abortController.signal; + const response = await fetch(`${this.endpoint}/api/generate`, { + // ... + signal + }); + this._abortController = null; + + // Add to chatStream() method (around line 140) - same pattern + ``` + +--- + +## 3. AI Routing Issues + +### 3.1 Unknown Commands Not Awaited (executeCommand) +- **File:** `terminal.js` +- **Line:** 629 +- **Issue:** `aiMod.chat([rawInput.trim()])` called without `await` +- **Impact:** Fire-and-forget execution, errors uncaught, no sequencing +- **Fix:** + ```javascript + // Before + aiMod.chat([rawInput.trim()]); + return; + + // After + this.print('→ Routing to AI assistant...', 'info'); + await aiMod.chat([rawInput.trim()]); + return; + ``` + +--- + +### 3.2 Unknown Commands Not Awaited (dispatchCommand) +- **File:** `terminal.js` +- **Line:** 691 +- **Issue:** Same as 3.1 but in `dispatchCommand()` (used for pipe chains) +- **Fix:** Same as 3.1 + +**Combined Impact:** Unknown commands like `hello` route to AI but without proper error handling or user feedback + +--- + +## 4. Cache Invalidation + +### 4.1 Cache Version Stale +- **File:** `module-loader.js` +- **Line:** 102 +- **Issue:** `CACHE = '?v=17'` serves old cached modules +- **Impact:** Browser doesn't re-fetch updated module files +- **Fix:** + ```javascript + // Before + const CACHE = '?v=17'; + + // After + const CACHE = '?v=18'; + ``` +- **Note:** Requires users to hard refresh (Ctrl+F5 / Cmd+Shift+R) + +--- + +## Implementation Priority + +### High Priority (Critical Bugs) +1. **2.1** Agent loop caps at 1 turn — breaks multi-step AI tasks +2. **1.4** Packages.json 404 errors — WPM installs broken +3. **2.4** Default model blocks auto-selection — invisible failures +4. **2.5** abort() incomplete — `ai-stop` doesn't work for chat/stream + +### Medium Priority (Functional Issues) +5. **1.1** Splash fallback incomplete — minor UX issue +6. **2.2** edit_module destructive check — security inconsistency +7. **3.1 + 3.2** AI routing without await — error handling gaps +8. **1.2** Preview iframe sandbox — JS previews broken + +### Low Priority (Cleanup) +9. **1.3** Dead EXT_FILES code — maintenance clarity +10. **1.5** Search alert() → inline — UX polish +11. **4.1** Cache version — required after applying fixes + +--- + +## Tradeoffs & Considerations + +### Packages.json Cleanup (1.4) +- **Option A (Recommended):** Remove 12 phantom packages + - ✅ Reduces bloat, cleaner codebase + - ❌ Breaks backward compatibility if anyone installed them +- **Option B:** Create stub files for missing packages + - ✅ Preserves compatibility + - ❌ Adds dead weight, maintenance burden + +### Agent Loop Change (2.1) +- Removing follow-up call changes message structure slightly +- Text+tool hybrid responses now push text as `assistant` message, then loop continues +- This is correct behavior but differs from original design + +### AI Routing (3.1 + 3.2) +- Routing all unknown commands to AI may increase token usage +- Consider adding `ai-mode` toggle or `--no-ai` flag for opt-out + +### Cache Version (4.1) +- Bumping version requires all users to hard refresh +- Consider adding release notes or announcement + +--- + +## Testing Checklist + +After applying fixes: + +- [ ] Hard refresh page (Ctrl+F5) +- [ ] Test splash screen shows all 8 ext modules +- [ ] Test HTML preview with JavaScript executes +- [ ] Test WPM package installs succeed +- [ ] Test search shows inline message (no alert) +- [ ] Test multi-step AI task (5 tool turns) +- [ ] Test `edit_module` prompts for confirmation +- [ ] Test auto-select model on connect +- [ ] Test `ai-stop` cancels chat/stream +- [ ] Test unknown command routes to AI (`hello`) + +--- + +## Files to Modify + +``` +spaces/chatstudio/newos4/ +├── index.html (lines 684, 770) +├── help.html (line 1149) +├── packages.json (lines 7-167) +└── modules/core/ + ├── module-loader.js (lines 35-37, 102) + ├── module-aiassist.js (lines 537-547, 730) + ├── ollama-manager.js (lines 8, 42-45, 70, 140) + └── terminal.js (lines 629, 691) +``` + +--- + +## Estimated Effort + +| Category | Time | Complexity | +|----------|------|------------| +| File fixes | 15 min | Low | +| Agent fixes | 30 min | Medium | +| AI routing | 10 min | Low | +| Testing | 20 min | Medium | +| **Total** | **~75 min** | | + +--- + +## Approval Required + +**Ready to execute?** Reply with: +- ✅ "Execute all" — apply all 13 fixes +- ✅ "Execute high priority" — apply top 4 fixes only +- ✅ "Skip #X" — exclude specific fixes +- ❓ "Discuss #X" — more details on specific issue |