Files
opensquilla--opensquilla/docs/features/gateway-startup-locking-spec.md
2026-07-13 13:12:33 +08:00

220 lines
8.3 KiB
Markdown

# Gateway Startup Locking Spec
Date: 2026-06-27
Status: Draft
## Problem
Desktop first-run startup can leave the bundled gateway unable to boot when the
initial process is interrupted or a second gateway process is started while the
first process is still in schema migration.
The observed failure pattern is:
- A fresh desktop state starts successfully enough to seed the agent workspace.
- The first bundled gateway process does not reach `gateway.started`.
- Later gateway processes fail in `apply_pending()` with yoyo `LockTimeout`.
- The yoyo error reports the same earlier process id for every retry.
The direct failure is a stale or active row in the yoyo migration lock table for
the desktop `sessions.db`. Once this state exists, repeated desktop retries do
not recover by themselves.
## Goals
- Prevent the Electron shell from launching duplicate gateway processes for the
same desktop profile.
- Keep the gateway pid lock alive for the full server lifetime and release it
on graceful shutdown.
- Recover from a stale yoyo migration lock only when the recorded pid is proven
dead.
- Preserve yoyo's safety guarantee when another process may still be migrating.
- Surface actionable startup errors instead of an opaque PyInstaller traceback.
## Non-Goals
- Do not disable yoyo migration locking.
- Do not blindly run `break-lock` on every migration lock timeout.
- Do not change the session database schema as part of this fix.
- Do not make the desktop gateway share a process with Electron.
- Do not broaden gateway binding or auth behavior.
## Existing Surfaces
- Electron launches the bundled gateway in `desktop/electron/src/main.ts` via
`spawn(...)`.
- Electron waits up to 45 seconds for `/healthz` before reporting that the
gateway did not become healthy.
- The gateway acquires `GatewayPidLock` before `build_services()` and before
session database migration.
- `build_services()` runs `apply_pending(session_db_path, migrations_dir)` before
opening `SessionStorage`.
- `apply_pending()` delegates locking to `with backend.lock():`.
- yoyo implements the migration lock as a row in `yoyo_lock`, keyed by pid, and
removes it in a `finally` block. A hard process exit can leave the row behind.
## Design
Implement three defensive layers. Each layer addresses a different failure mode
and should be independently testable.
### Layer 1: Electron Single Instance Guard
Add `app.requestSingleInstanceLock()` near Electron app startup, before any
gateway startup work can run.
Expected behavior:
- If the lock cannot be acquired, the new Electron process exits immediately.
- On `second-instance`, the existing main window is restored and focused.
- `bootDesktopApp()` must not run in the second Electron process.
- The existing process remains the only owner allowed to spawn a gateway.
This reduces duplicate gateway starts caused by double-clicking the app,
reopening from Finder, or launching from a DMG while a first run is still in
progress.
### Layer 2: Gateway PID Lock Lifetime
Store the acquired `GatewayPidLock` on the returned `GatewayServer` object and
release it in `GatewayServer.close()`.
Expected behavior:
- `start_gateway_server()` acquires the lock before database migration, as it
does today.
- The lock object remains strongly referenced for the whole server lifetime.
- `GatewayServer.close()` calls `release()` exactly once, after shutdown work is
complete enough that a new gateway may safely start.
- `release()` remains idempotent.
- Existing `atexit` and signal cleanup remain as best-effort fallback paths.
This makes normal gateway shutdown explicit instead of relying only on process
exit behavior.
### Layer 3: Conservative Yoyo Stale Lock Recovery
Wrap yoyo `LockTimeout` handling inside `apply_pending()`.
When yoyo reports a lock timeout:
1. Inspect the lock table for recorded pids.
2. If any recorded pid is alive, fail without clearing the lock.
3. If every recorded pid is dead or invalid, delete the yoyo lock rows.
4. Retry migration once.
5. If the retry fails, surface the second failure without another recovery loop.
The liveness check should use platform-appropriate process probing:
- POSIX: `os.kill(pid, 0)`.
- Windows: `OpenProcess` or equivalent existing helper behavior.
The lock recovery path must log structured events:
- `migrator.lock_timeout`
- `migrator.lock_held_by_live_process`
- `migrator.stale_lock_cleared`
- `migrator.stale_lock_retry_failed`
The operator-facing error should explain whether the gateway is still starting,
another gateway is running, or a stale migration lock could not be recovered.
## Safety Rules
- Never clear a yoyo lock held by a live pid.
- Never clear the lock if the database cannot be inspected safely.
- Never retry migration more than once after clearing a stale lock.
- Keep migration failure loud if schema state is uncertain.
- Prefer false negatives over false positives: failing startup is safer than
corrupting a migration.
## Implementation Notes
Electron:
- Add single-instance handling in `desktop/electron/src/main.ts`.
- Keep the current `startupInProgress` guard for the existing process.
- On a second instance, restore and focus `mainWindow` if it exists.
Gateway:
- Extend `GatewayServer` with an optional pid lock field.
- Assign the acquired lock after `GatewayPidLock.acquire()` succeeds.
- Release the lock in `GatewayServer.close()` in a `finally`-safe path.
Migrator:
- Import yoyo `LockTimeout` explicitly.
- Add a small helper to query `yoyo_lock` rows through the yoyo backend or a
separate SQLite connection to the same local database.
- Add a small helper to clear the yoyo lock table.
- Keep `:memory:` behavior unchanged.
- Keep normal no-pending migration behavior unchanged.
## Test Plan
### Unit Tests
- `GatewayPidLock` rejects a second lock acquisition for the same state dir while
the first lock is held.
- `GatewayServer.close()` releases the stored gateway pid lock.
- `apply_pending()` does not clear a yoyo lock for a live pid.
- `apply_pending()` clears a yoyo lock for a dead pid and retries once.
- `apply_pending()` does not enter an unbounded retry loop after stale-lock
recovery fails.
- `apply_pending()` preserves normal migration success and no-op behavior.
### Electron Tests
- A second Electron instance does not call `bootDesktopApp()`.
- A second Electron instance focuses the existing window.
- Desktop retry does not spawn a new gateway while startup is already in
progress.
### Integration Tests
- Create a temporary desktop state with `sessions.db` containing a stale
`yoyo_lock` row. Gateway startup clears it and completes migration.
- Create a temporary desktop state with a live helper process recorded in
`yoyo_lock`. Gateway startup fails with a clear, non-destructive error.
- Start two gateway processes against the same state dir. The second process
fails before database migration.
### Manual DMG Smoke
- Install the DMG into `/Applications`.
- Start from a clean desktop user data directory.
- Double-click the app repeatedly during first-run startup.
- Confirm only one gateway process is launched.
- Confirm the app becomes ready or surfaces a clear single startup error.
- Force quit during first-run migration, relaunch, and confirm stale-lock
recovery works when the recorded pid is no longer alive.
## Acceptance Criteria
- Fresh DMG first run cannot spawn two owned gateway processes from two Electron
instances.
- A stale yoyo migration lock whose pid is dead is cleared automatically and the
gateway starts.
- A yoyo migration lock whose pid is alive is never cleared automatically.
- A second gateway using the same desktop state fails before migration work.
- Graceful gateway shutdown removes `gateway.pid` and releases
`gateway.pid.lock`.
- Failure messages distinguish active startup, already-running gateway, and
unrecoverable migration lock states.
- CI covers stale-lock recovery and live-lock non-recovery.
## Rollout
- Ship behind normal startup behavior, with no user-facing setting.
- Keep structured logs in the desktop gateway log for post-incident diagnosis.
- Add a short troubleshooting entry once the behavior is implemented.
## Open Questions
- Whether yoyo lock inspection should use yoyo backend APIs only or direct
SQLite for local file databases.
- Whether desktop startup should extend the first-run health timeout after it
detects schema migration is active.
- Whether the boot splash should show a distinct "Preparing database" phase.