toes/ISSUES.md

Issues - Versioned Deployment Implementation

Critical Issues

1. Watch Filter Breaks File Change Detection

Location: src/server/apps.ts:589-593

Problem: The watch logic ignores all changes deeper than 2 levels:

// Ignore changes deeper than 2 levels (inside timestamp dirs)
if (parts.length > 2) return

// For versioned apps, only care about changes to "current" directory
if (parts.length === 2 && parts[1] !== 'current' && parts[1] !== 'package.json') return

Files inside current/ are 3 levels deep: appname/current/somefile.ts

This ignores all file changes inside the current directory, breaking hot reload and auto-restart.

Fix:

// Ignore changes inside old timestamp dirs (but allow current/)
if (parts.length > 2 && parts[1] !== 'current') return

---

2. App Restart Race Condition

Location: src/server/api/sync.ts:145-148

Problem: Activation uses arbitrary 1 second delay without confirming stop completed:

// Restart app to use new version
const app = allApps().find(a => a.name === appName)
if (app?.state === 'running') {
  stopApp(appName)
  setTimeout(() => startApp(appName), 1000)  // ⚠️ Arbitrary 1s delay
}

Issues:

• 1 second may not be enough for app to fully stop
• No confirmation that stop completed before start
• If app crashes during startup, activation still returns success

Fix: Make activation async and wait for stop to complete, or move restart logic to after symlink succeeds and poll for stop completion.

---

Major Issues

3. No Version Cleanup

Problem: Old timestamp directories accumulate forever with no cleanup mechanism.

Impact: Disk space grows indefinitely as deployments pile up.

Recommendation: Add cleanup logic:

• Keep last N versions (e.g., 5-10)
• Delete versions older than X days
• Expose as toes clean <app> command or automatic post-activation

---

4. safePath Behavior Change

Location: src/server/api/sync.ts:14-21

Problem: Security model changed by resolving symlinks in base path:

const canonicalBase = existsSync(base) ? realpathSync(base) : base

Previously paths were checked against literal base, now against resolved base. This changes behavior if someone creates a symlink attack.

Recommendation: Document this intentional change, or keep original check and add separate symlink resolution only where needed.

---

5. Non-Atomic Deploy Copy

Location: src/server/api/sync.ts:133-141

Problem: Race condition possible during deploy:

if (existsSync(currentLink)) {
  const currentReal = realpathSync(currentLink)
  cpSync(currentReal, newVersion, { recursive: true })
}

If current changes between existsSync and cpSync, stale code might be copied.

Impact: Low probability for single-user system, but possible during concurrent deploys.

Fix: Read symlink once and reuse: const currentReal = existsSync(currentLink) ? realpathSync(currentLink) : null

---

6. Upload Error Handling Inconsistency

Location: src/cli/commands/sync.ts:113-115

Problem: Upload continues if a file fails but doesn't track failures:

if (success) {
  console.log(`  ${color.green('↑')} ${file}`)
} else {
  console.log(`  ${color.red('✗')} ${file}`)
  // Continue even if one file fails
}

If any file fails to upload, deployment still activates with incomplete files.

Fix: Either:

• Abort entire deployment on first failure
• Track failures and warn/abort before activating

---

Minor Issues

7. POST Body Check Too Loose

Location: src/cli/http.ts:42-44

Problem: Falsy values like 0, false, or "" would incorrectly skip body:

headers: body ? { 'Content-Type': 'application/json' } : undefined,
body: body ? JSON.stringify(body) : undefined,

Fix:

headers: body !== undefined ? { 'Content-Type': 'application/json' } : undefined,
body: body !== undefined ? JSON.stringify(body) : undefined,

---

8. Unconventional Timestamp Format

Location: src/server/api/sync.ts:115-123

Problem: Unusual array construction with separator as element:

const timestamp = [
  now.getFullYear(),
  String(now.getMonth() + 1).padStart(2, '0'),
  String(now.getDate()).padStart(2, '0'),
  '-',  // Unusual to put separator as array element
  String(now.getHours()).padStart(2, '0'),
  String(now.getMinutes()).padStart(2, '0'),
  String(now.getSeconds()).padStart(2, '0'),
].join('')

Recommendation: Use template literal:

const pad = (n: number) => String(n).padStart(2, '0')
const timestamp = `${now.getFullYear()}${pad(now.getMonth()+1)}${pad(now.getDate())}-${pad(now.getHours())}${pad(now.getMinutes())}${pad(now.getSeconds())}`

Or ISO format: now.toISOString().replace(/[:.]/g, '-').slice(0, -5)

---

Positive Points

✓ Atomic symlink swapping with temp file pattern is correct ✓ Clear 3-step deployment protocol (deploy, upload, activate) ✓ Proper use of realpathSync to resolve canonical paths for running apps ✓ Good separation of concerns in API routes

---

Priority

1. BLOCKER: Fix watch filter (#1) - breaks hot reload
2. HIGH: Fix restart race condition (#2) - affects deployment reliability
3. HIGH: Add version cleanup (#3) - disk space concern
4. MEDIUM: Fix upload error handling (#6) - data integrity
5. LOW: Everything else