refactor: deduplicate execGit, UUID validation, pagination, and remove compat aliases by atomantic · Pull Request #134 · atomantic/PortOS

atomantic · 2026-04-10T04:47:25Z

Better Audit — DRY & YAGNI

Summary

6 findings addressed across 22 files.

Changes

[HIGH] Extract shared execGit to server/lib/execGit.js — delete duplicates in taskConflict.js and worktreeManager.js
[HIGH] Remove redundant existsSync guards in 5 services (ensureDir is already idempotent)
[HIGH] Replace inline UUID validation with shared UUID_RE in messages.js and calendar.js (27 occurrences)
[HIGH] Add parsePagination utility to validation.js; use in messages.js, calendar.js, feeds.js
[MEDIUM] Remove 11 getSoul* backward-compat aliases from apiDigitalTwin.js; update all 7 callers
[MEDIUM] Delete 4 deprecated schedule route aliases in cosScheduleRoutes.js

Files Modified

server/lib/{execGit.js,validation.js}, server/services/{git,taskConflict,worktreeManager,decisionLog,brainStorage,taskTemplates,taskLearning,brainSyncLog}.js, server/routes/{messages,calendar,feeds,cosScheduleRoutes}.js, client/src/services/apiDigitalTwin.js, client/src/components/digital-twin/*, client/src/pages/DigitalTwin.jsx

Merge Order

Independent — can be merged in any order

Copilot

Pull request overview

This PR refactors shared utilities and removes deprecated compatibility layers across server and client, aiming to reduce duplication (git execution, pagination parsing, UUID validation) and retire legacy “Soul” naming in the Digital Twin UI/API.

Changes:

Introduces a shared execGit utility and updates services to consume its { stdout, stderr, exitCode } result shape.
Adds parsePagination and adopts it in routes; standardizes UUID validation via UUID_RE.
Removes deprecated route aliases and client-side getSoul* backward-compat API aliases, updating several call sites.

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
server/lib/execGit.js	New shared git command runner to deduplicate implementations and avoid circular deps.
server/lib/validation.js	Adds `parsePagination` helper for consistent limit/offset parsing.
server/services/worktreeManager.js	Switches to shared `execGit` and updates callers to use `.stdout`.
server/services/taskConflict.js	Switches to shared `execGit` and updates stdout handling.
server/services/git.js	Re-exports and consumes shared `execGit` implementation.
server/services/taskTemplates.js	Removes redundant `existsSync` guard in favor of idempotent `ensureDir`.
server/services/taskLearning.js	Removes redundant `existsSync` guard in favor of idempotent `ensureDir`.
server/services/decisionLog.js	Removes redundant `existsSync` guard in favor of idempotent `ensureDir`.
server/services/brainSyncLog.js	Removes redundant `existsSync` directory guard in favor of idempotent `ensureDir`.
server/services/brainStorage.js	Removes redundant `existsSync` guard in `ensureBrainDir` in favor of `ensureDir`.
server/routes/messages.js	Uses `UUID_RE` for UUID checks; adopts `parsePagination` for inbox pagination.
server/routes/calendar.js	Uses `UUID_RE` for UUID checks; adopts `parsePagination` for events pagination.
server/routes/feeds.js	Adds limit/offset pagination via `parsePagination` (currently applied by slicing in route).
server/routes/cosScheduleRoutes.js	Removes deprecated schedule alias routes.
server/routes/cosScheduleRoutes.test.js	Removes tests for deprecated alias routes.
client/src/services/apiDigitalTwin.js	Removes `getSoul*` compatibility aliases; keeps Digital Twin API names only.
client/src/pages/DigitalTwin.jsx	Updates to `getDigitalTwin*` API calls.
client/src/components/digital-twin/tabs/TestTab.jsx	Updates to `getDigitalTwin*` API calls.
client/src/components/digital-twin/tabs/OverviewTab.jsx	Updates to `getDigitalTwin*` API calls.
client/src/components/digital-twin/tabs/ExportTab.jsx	Updates to `getDigitalTwin*` API calls.
client/src/components/digital-twin/tabs/DocumentsTab.jsx	Updates to `getDigitalTwin*` API calls.
client/src/components/digital-twin/NextActionBanner.jsx	Updates to `getDigitalTwin*` API calls.

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

server/routes/messages.js

server/routes/calendar.js

server/lib/validation.js

server/routes/feeds.js

client/src/services/apiDigitalTwin.js

Copilot

Pull request overview

Copilot reviewed 22 out of 22 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/services/apiDigitalTwin.js

server/lib/validation.js

Copilot

Pull request overview

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

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

Copilot · 2026-04-10T14:08:12Z

server/routes/messages.js

+  if (!UUID_RE.test(req.params.id)) {
    return res.status(400).json({ error: 'Invalid account ID format' });
  }


UUID_RE is less strict than the previous z.string().uuid() validation (it matches any 8-4-4-4-12 hex groups and does not enforce RFC 4122 version/variant bits). If these IDs are expected to be real UUIDs (and especially v4), this change broadens accepted inputs and may allow values that were previously rejected. Consider either switching back to z.string().uuid() here, or tightening UUID_RE (and/or its “UUID v4” doc in server/lib/fileUtils.js:376) to match the intended UUID format.

Copilot · 2026-04-10T14:08:12Z

server/routes/calendar.js

+  if (!UUID_RE.test(req.params.id)) {
    throw new ServerError('Invalid account ID format', { status: 400, code: 'VALIDATION_ERROR' });
  }


UUID_RE is less strict than the previous z.string().uuid() validation (it matches any 8-4-4-4-12 hex groups and does not enforce RFC 4122 version/variant bits). If account IDs are expected to be real UUIDs, this broadens accepted inputs. Consider using z.string().uuid() here, or tightening the shared UUID_RE (and/or correcting its “UUID v4” doc in server/lib/fileUtils.js:376) to match the intended format.

Copilot · 2026-04-10T14:08:13Z

server/lib/validation.js

+export function parsePagination(query, { defaultLimit = 50, maxLimit = 200 } = {}) {
+  const rawLimit = parseInt(query?.limit, 10);
+  const rawOffset = parseInt(query?.offset, 10);
+  const limit = Number.isFinite(rawLimit) && rawLimit > 0 ? Math.min(rawLimit, maxLimit) : defaultLimit;
+  const offset = Number.isFinite(rawOffset) && rawOffset >= 0 ? rawOffset : 0;
+  return { limit, offset };
+}


parsePagination is newly introduced but server/lib/validation.test.js has extensive coverage for other helpers in this module; adding targeted tests for defaulting, clamping to maxLimit, negative/NaN inputs, and string/array query param shapes would help prevent regressions across all routes that now depend on it.

Copilot · 2026-04-10T14:08:13Z

server/lib/execGit.js

+/**
+ * Execute a git command safely using spawn (prevents shell injection).
+ * @param {string[]} args - Git command arguments
+ * @param {string} cwd - Working directory
+ * @param {object} options - Additional options
+ * @param {number} [options.maxBuffer] - Max output buffer size in bytes (default 10 MB)
+ * @param {number} [options.timeout] - Timeout in ms (default 30s)
+ * @param {boolean} [options.ignoreExitCode] - Resolve instead of reject on non-zero exit
+ * @returns {Promise<{stdout: string, stderr: string, exitCode: number}>}
+ */
+export function execGit(args, cwd, options = {}) {
+  return new Promise((resolve, reject) => {
+    const maxBuffer = options.maxBuffer || 10 * 1024 * 1024;
+    const timeout = options.timeout || 30000;
+    const child = spawn('git', args, {
+      cwd,
+      shell: process.platform === 'win32',
+      windowsHide: true
+    });


The JSDoc says this “prevents shell injection”, but on Windows this runs spawn with shell: true. Now that this shared helper is used by more services (e.g. worktrees/conflict checks), please either avoid shell: true (if feasible) or explicitly document/guard that all interpolated args (branch names, paths, etc.) are trusted/sanitized for Windows shell execution.

dry: better audit remediation

1731128

Copilot AI review requested due to automatic review settings April 10, 2026 04:47

Copilot started reviewing on behalf of atomantic April 10, 2026 04:48 View session

Copilot AI reviewed Apr 10, 2026

View reviewed changes

server/routes/messages.js Show resolved Hide resolved

server/routes/calendar.js Show resolved Hide resolved

server/lib/validation.js Show resolved Hide resolved

server/routes/feeds.js Show resolved Hide resolved

client/src/services/apiDigitalTwin.js Show resolved Hide resolved

atomantic requested a review from Copilot April 10, 2026 04:53

Copilot started reviewing on behalf of atomantic April 10, 2026 04:54 View session

Copilot AI reviewed Apr 10, 2026

View reviewed changes

client/src/services/apiDigitalTwin.js Show resolved Hide resolved

server/lib/validation.js Show resolved Hide resolved

atomantic added 2 commits April 10, 2026 06:58

fix: address review: update remaining getSoul call sites in EnrichTab

de9b8af

fix: resolve merge conflict in EnrichTab, use canonical API names

25b48ea

Copilot AI review requested due to automatic review settings April 10, 2026 13:58

atomantic merged commit 082f21b into main Apr 10, 2026
2 checks passed

Copilot started reviewing on behalf of atomantic April 10, 2026 13:59 View session

atomantic deleted the better/dry branch April 10, 2026 13:59

Copilot AI reviewed Apr 10, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: deduplicate execGit, UUID validation, pagination, and remove compat aliases#134

refactor: deduplicate execGit, UUID validation, pagination, and remove compat aliases#134
atomantic merged 3 commits intomainfrom
better/dry

atomantic commented Apr 10, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

atomantic commented Apr 10, 2026

Better Audit — DRY & YAGNI

Summary

Changes

Files Modified

Merge Order

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants