Skip to content

refactor: split DevTools/Settings god files, fix async handlers, delete dead code#135

Merged
atomantic merged 9 commits intomainfrom
better/architecture
Apr 10, 2026
Merged

refactor: split DevTools/Settings god files, fix async handlers, delete dead code#135
atomantic merged 9 commits intomainfrom
better/architecture

Conversation

@atomantic
Copy link
Copy Markdown
Owner

@atomantic atomantic commented Apr 10, 2026

Better Audit — Architecture & SOLID

Summary

7 findings addressed across 20 files (11 new files created from splits).

Changes

  • [CRITICAL] cos.js: collapse duplicate description functions, fix 4 unhandled async EventEmitter listeners
  • [CRITICAL] taskSchedule.js: delete 14 unused backward-compat stubs (-59 lines)
  • [HIGH] Split DevTools.jsx (1808 lines) into 6 page files: HistoryPage, RunsHistoryPage, RunnerPage, ProcessesPage, UsagePage, AgentsPage
  • [HIGH] Extract Settings.jsx (1061 lines) into 5 tab components: BackupTab, DatabaseTab, TelegramTab, GeneralTab, ImageGenTab
  • [HIGH] Wrap all async socket handlers in socket.js with try/catch
  • [MEDIUM] Convert spawnViaRunner 10-param signature to options object in agentLifecycle.js
  • [MEDIUM] Fix cosEvents import path in memory.js and memoryDB.js (use cosEvents.js directly)

Files Modified

server/services/{cos,taskSchedule,socket,agentLifecycle,memory,memoryDB}.js, client/src/pages/{DevTools,Settings}.jsx + 11 new extracted files

Merge Order

Independent — can be merged in any order

Copilot AI review requested due to automatic review settings April 10, 2026 04:47
Copilot AI reviewed Apr 10, 2026
View reviewed changes
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors oversized DevTools/Settings UI “god files” into smaller components while cleaning up and hardening parts of the CoS backend (event wiring, socket handler error handling, and removal of dead compatibility code).

Changes:

  • Split client/src/pages/DevTools.jsx into individual page modules and split client/src/pages/Settings.jsx into tab components.
  • Improve backend robustness by handling async errors in CoS event listeners / socket handlers and removing dead compatibility exports.
  • Normalize CoS event emitter imports (cosEvents.js) and simplify spawnViaRunner invocation via an options object.

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
server/services/taskSchedule.js Removes backward-compat delegate exports (dead code cleanup).
server/services/socket.js Adds try/catch around several async socket handlers.
server/services/memoryDB.js Switches cosEvents import to cosEvents.js to avoid circular deps.
server/services/memory.test.js Updates mocks/imports to match cosEvents.js usage.
server/services/memory.js Switches cosEvents import to cosEvents.js.
server/services/cos.js Unifies improvement task description helpers; wraps async event listeners to avoid unhandled rejections.
server/services/agentLifecycle.js Converts spawnViaRunner call/signature to an options object.
client/src/pages/DevTools.jsx Converts DevTools into a barrel re-export file.
client/src/pages/HistoryPage.jsx New extracted DevTools History page module.
client/src/pages/RunsHistoryPage.jsx New extracted AI runs history page module.
client/src/pages/RunnerPage.jsx New extracted DevTools runner page module.
client/src/pages/ProcessesPage.jsx New extracted PM2 processes page module.
client/src/pages/UsagePage.jsx New extracted usage metrics page module.
client/src/pages/AgentsPage.jsx New extracted agents/process list page module.
client/src/pages/Settings.jsx Replaces monolith Settings page with a tab router that renders extracted tab components.
client/src/components/settings/BackupTab.jsx Extracted Settings “Backup” tab UI/logic.
client/src/components/settings/DatabaseTab.jsx Extracted Settings “Database” tab UI/logic.
client/src/components/settings/TelegramTab.jsx Extracted Settings “Telegram” tab UI/logic.
client/src/components/settings/GeneralTab.jsx Extracted Settings “General” tab UI/logic.
client/src/components/settings/ImageGenTab.jsx Extracted Settings “Image Gen” tab UI/logic.
Comments suppressed due to low confidence (1)

server/services/socket.js:81

  • Inside the standardize:start handler, the code within try { ... } is not indented consistently with the surrounding file. This makes the try/catch scope harder to visually verify and may fail linting/style checks. Please align indentation for the block.
    socket.on('standardize:start', async (rawData) => {
      try {
      const data = validateSocketData(standardizeStartSchema, rawData, socket, 'standardize:start');
      if (!data) return;
      const { repoPath, providerId } = data;
      console.log(`🔧 Starting PM2 standardization: ${repoPath}`);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@atomantic
fix: address Copilot review — emit operation-specific errors, guard n…
d2edbc2 
…ulls, wire expand buttons

- socket.js: emit standardize:complete, app:update:error/complete, app:standardize:error
  in outer catch blocks so clients don't get stuck in loading state
- socket.js: fix indentation inside standardize:start try block
- UsagePage.jsx: guard formatNumber against null/undefined, use nullish coalescing for tokens/cost
- RunsHistoryPage.jsx: wire onClick and aria-label on expand/collapse button
- HistoryPage.jsx: wire onClick on expand/collapse button (with stopPropagation)
- RunnerPage.jsx: add aria-label to dismiss context and remove screenshot buttons,
  add group-focus-within:opacity-100 for keyboard accessibility
@atomantic atomantic requested a review from Copilot April 10, 2026 04:58
Copilot AI reviewed Apr 10, 2026
View reviewed changes
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 9 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@atomantic
fix: use safe error message extraction in catch/catch handlers
066447f 
- socket.js: use err?.message ?? String(err) in all catch blocks to handle
  non-Error rejection values; emit detect:complete on detect:start failure
- cos.js: use err?.message ?? String(err) in event listener catch handlers
@atomantic atomantic requested a review from Copilot April 10, 2026 05:06
Copilot AI reviewed Apr 10, 2026
View reviewed changes
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@atomantic
fix: guard zero division in charts, hoist maxHour, fix workspace over…
9c44ff2 
…ride on resume

- UsagePage: clamp maxActivity to >= 1 to prevent NaN bar heights
- UsagePage: compute maxHour once outside map loop (O(n) vs O(n^2))
- RunnerPage: use functional setState for workspace selection to avoid
  overriding continuation context workspace with default
@atomantic atomantic requested a review from Copilot April 10, 2026 05:13
Copilot AI reviewed Apr 10, 2026
View reviewed changes
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@atomantic
fix: wrap app:deploy handler in consistent outer try/catch
75a99f8 
Apply the same error handling pattern as other socket handlers:
outer try/catch with safe error extraction and operation-specific
error event emission.
@atomantic atomantic requested a review from Copilot April 10, 2026 05:21
Copilot AI reviewed Apr 10, 2026
View reviewed changes
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@atomantic atomantic requested a review from Copilot April 10, 2026 05:27
Copilot AI reviewed Apr 10, 2026
View reviewed changes
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@atomantic
fix: emit recovery-specific error, remove redundant mock, use React R…
53be2d8 
…outer for state clear

- socket.js: emit error:recover:error instead of generic error:server
- memory.test.js: remove redundant cos.js mock (memory.js imports from cosEvents.js)
- RunnerPage.jsx: use navigate() with replace instead of window.history.replaceState
  to preserve React Router internal state
@atomantic atomantic requested a review from Copilot April 10, 2026 05:37
Copilot AI reviewed Apr 10, 2026
View reviewed changes
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@atomantic
fix: workspace selection race, unhandled delete rejections
c523857 
- RunnerPage: store continuation workspace in ref to survive location.state
  clearing; remove stale location.state read from apps initializer
- RunsHistoryPage: catch deleteRun failures to prevent optimistic UI update
- HistoryPage: catch clearHistory/deleteHistoryEntry failures gracefully
@atomantic atomantic requested a review from Copilot April 10, 2026 05:46
Copilot AI reviewed Apr 10, 2026
View reviewed changes
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

client/src/pages/RunsHistoryPage.jsx Outdated
Comment on lines +141 to +143
const result = await api.deleteFailedRuns().catch(() => null);
if (result) {
setRuns(prev => prev.filter(r => r.success !== false));
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

deleteFailedRuns() may also return null for HTTP 204 via request(). Because the code only updates local state when result is truthy, the list may not refresh after a successful delete that returns no content. Consider checking for result !== null (or simply awaiting the call and updating state on success).

Suggested change
const result = await api.deleteFailedRuns().catch(() => null);
if (result) {
setRuns(prev => prev.filter(r => r.success !== false));
try {
await api.deleteFailedRuns();
setRuns(prev => prev.filter(r => r.success !== false));
} catch {
// Leave local state unchanged if the delete request fails.

Copilot uses AI. Check for mistakes.
client/src/pages/RunnerPage.jsx Outdated
Comment on lines +79 to +80
setSelectedProvider(active.id);
setSelectedModel(active.defaultModel || '');
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

When navigating with continueFrom, the first effect sets selectedProvider/selectedModel from the previous run, but the initialization effect later unconditionally overwrites them with the active/default provider/model (setSelectedProvider(active.id) / setSelectedModel(active.defaultModel)). This makes “resume/continue” pick the wrong provider/model. Preserve existing selections (e.g., only set defaults when state is still empty, or gate on continueContext).

Suggested change
setSelectedProvider(active.id);
setSelectedModel(active.defaultModel || '');
setSelectedProvider(prev => prev || active.id);
setSelectedModel(prev => prev || active.defaultModel || '');

Copilot uses AI. Check for mistakes.
@atomantic
fix: handle 204 in deleteFailedRuns, preserve provider/model on resume
0ee072f 
- RunsHistoryPage: use then/catch boolean pattern for deleteFailedRuns
  to handle HTTP 204 (null result) correctly
- RunnerPage: use functional setState for provider/model to avoid
  overwriting continuation context selections with defaults
@atomantic atomantic requested a review from Copilot April 10, 2026 05:51
Copilot AI reviewed Apr 10, 2026
View reviewed changes
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@atomantic atomantic merged commit 151320e into main Apr 10, 2026
6 checks passed
@atomantic atomantic deleted the better/architecture branch April 10, 2026 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@atomantic