Address 4 PR review comments + add learnings to copilot instructions

Agent-Logs-Url: https://github.com/InterestingSoftware/SproutGit/sessions/741dc53d-08c0-4423-961d-568eca87c217

Author: Copilot

.github/copilot-instructions.md

@@ -301,6 +301,7 @@ static WORKSPACE_MIGRATIONS: LazyLock<Migrations<'static>> = LazyLock::new(|| {
 - Append the new `M::up(include_str!("../migrations/<db>/<NNN>_….sql"))` entry to the correct `LazyLock` vec.
 - Indexes that reference a column added in the same migration must appear after that `ALTER TABLE` in the same file, or in a later migration file.
 - Do not create bespoke `PRAGMA table_info` checks — the framework handles idempotency.
+- **Never add `ALTER TABLE … ADD COLUMN` statements outside migration files**, even as "compatibility shims" that try to swallow "duplicate column name" errors. SQLite error strings can vary across versions, making string-matching fragile. If a column is needed, add a numbered migration file and register it in the `LazyLock` vec.
 
 ### `db.rs` public API
 
@@ -436,7 +437,7 @@ When adding or changing **any** git or system interaction, follow all rules belo
 - **Path handling**: Use `Path`/`PathBuf` and platform-aware path/env separators. Do not hardcode `:` as PATH separator.
 - **Path canonicalization on Windows**: `std::fs::canonicalize()` returns `\\?\`-prefixed extended-length paths on Windows (e.g. `\\?\D:\...`). Always chain `strip_win_prefix()` from `crate::git::helpers` immediately after any `canonicalize()` call. This applies to paths stored in SQLite, set as subprocess env vars, or passed to shell scripts — not just paths serialised to the frontend. Git for Windows tolerates extended-length paths silently, but PowerShell cmdlets (e.g. `Join-Path`) reject them with cryptic errors.
 - **Single-source normalization rule**: Path normalization logic must live in shared helpers, not duplicated at call sites. When introducing a new canonicalization/normalization path, reuse existing helper functions or add one shared helper first.
-- **Path serialization — frontend-bound values**: Any `Path`/`PathBuf` value serialised in a Tauri command response (struct fields, `Ok(...)` return values) **must** be converted with `path_to_frontend()` from `crate::git::helpers`, not with `to_string_lossy()`. Git always outputs forward slashes on every OS; using the same convention prevents frontend path-comparison mismatches on Windows. Paths passed as arguments to git/system commands should stay as native OS paths via `to_string_lossy()` directly.
+- **Path serialization — frontend-bound values**: Any `Path`/`PathBuf` value serialised in a Tauri command response (struct fields, `Ok(...)` return values) **must** be converted with `path_to_frontend()` from `crate::git::helpers`, not with `to_string_lossy()`. Git always outputs forward slashes on every OS; using the same convention prevents frontend path-comparison mismatches on Windows. Paths passed as arguments to git/system commands should stay as native OS paths via `to_string_lossy()` directly. **This also applies to path strings parsed from git command output** (e.g. the `worktree <path>` lines from `git worktree list --porcelain`) — call `path_to_frontend(Path::new(parsed_str))` rather than inlining `str.replace('\\', "/")` at the call site.
 - **Least privilege and clear errors**: Fail closed on invalid input and return clear, user-safe error messages.
 
 ## Security Testing Requirements
@@ -492,6 +493,7 @@ When editing or relying on documentation in this repository:
 - If a command, script, route, or API wrapper changes in code, update the corresponding docs in the same change.
 - Prefer representative command/API summaries plus explicit source-of-truth links over exhaustive static lists that quickly drift.
 - Remove placeholders and stale claims (for example, outdated URLs or deleted files) immediately when discovered.
+- **When editing component markup or styles, check nearby code comments for implementation-detail references** (e.g. hardcoded class names, colour literals) that may no longer be accurate. Stale comments about specifics like "hardcoded `bg-[#1e1e2e]`" that have since been replaced with CSS-variable equivalents cause confusion and must be updated or removed in the same change.
 
 Quick verification command before commit:
 
@@ -780,3 +782,4 @@ The `Spinner.svelte` component supports:
 - Svelte 5 `class:` directive with Tailwind arbitrary values (e.g., `class:bg-[var(--x)]/10={cond}`) works but looks odd
 - Window overflow: parent containers must be `flex flex-col overflow-hidden` for child `flex-1 overflow-auto` to scroll properly
 - **Windows `\\?\` paths in PowerShell**: If a path produced by `canonicalize()` is set as a subprocess env var and consumed by PowerShell, `Join-Path` will fail with "Cannot process argument because the value of argument 'drive' is null". The env var is non-null so a `if (-not $var)` guard won't catch it — the crash happens on the next line that uses the path. Always apply `strip_win_prefix` before passing any path into a hook or child process environment.
+- **`TerminalContainer.svelte` auto-spawn**: The component spawns an initial terminal session on mount via a `$effect` guarded by a plain (non-reactive) `_autoSpawned` flag. Do not remove this — E2E flows assert that a `[data-pty-id]` element exists immediately after the Terminal tab is revealed. If the auto-spawn is absent, the terminal tab is stuck on "No terminal sessions" until the user manually clicks `+`. The flag guard also prevents a reactive loop that would occur if `sessions.length` were read directly inside the effect (which Svelte would track as a dependency and re-fire on every session change).

e2e/helpers/screenshots.ts

@@ -116,8 +116,8 @@ const DARK_XTERM_THEME = {
 async function forceTheme(tauriPage: TauriPage | BrowserPageAdapter, theme: 'light' | 'dark') {
   const termBg = theme === 'dark' ? DARK_XTERM_THEME.background : LIGHT_XTERM_THEME.background;
   const cssVars = theme === 'dark' ? DARK_CSS_VARS : LIGHT_CSS_VARS;
-  // Include a rule that overrides the terminal wrapper background so the dark
-  // hardcoded bg-[#1e1e2e] class is replaced for light-mode screenshots.
+  // Include a rule that forces the terminal wrapper background to match the
+  // selected screenshot theme so the wrapper stays in sync with the xterm canvas.
   const css = JSON.stringify(
     `:root{${cssVars}} [data-sg-terminal]{background-color:${termBg}!important}`
   );

src-tauri/src/db.rs

@@ -48,30 +48,6 @@ static CONFIG_MIGRATIONS: LazyLock<Migrations<'static>> = LazyLock::new(|| {
     ))])
 });
 
-fn ensure_workspace_schema_compatibility(conn: &rusqlite::Connection) -> Result<(), String> {
-    let compatibility_statements = [
-        "ALTER TABLE hook_definitions ADD COLUMN keep_open_on_completion INTEGER NOT NULL DEFAULT 0",
-        "ALTER TABLE hook_definitions ADD COLUMN execution_target TEXT NOT NULL DEFAULT 'trigger_worktree'",
-        "ALTER TABLE hook_definitions ADD COLUMN execution_mode TEXT NOT NULL DEFAULT 'headless'",
-    ];
-
-    for statement in compatibility_statements {
-        match conn.execute(statement, []) {
-            Ok(_) => {},
-            Err(err) => {
-                let message = err.to_string();
-                if !message.contains("duplicate column name") {
-                    return Err(format!(
-                        "Failed to ensure workspace schema compatibility: {message}"
-                    ));
-                }
-            },
-        }
-    }
-
-    Ok(())
-}
-
 fn run_workspace_migrations(db_path: &Path) -> Result<(), String> {
     if let Some(parent) = db_path.parent() {
         ensure_directory(parent)?;
@@ -81,8 +57,7 @@ fn run_workspace_migrations(db_path: &Path) -> Result<(), String> {
     apply_connection_pragmas(&conn)?;
     WORKSPACE_MIGRATIONS
         .to_latest(&mut conn)
-        .map_err(|e| format!("Workspace database migration failed: {e}"))?;
-    ensure_workspace_schema_compatibility(&conn)
+        .map_err(|e| format!("Workspace database migration failed: {e}"))
 }
 
 fn run_config_migrations(db_path: &Path) -> Result<(), String> {

src-tauri/src/git/operations.rs

@@ -202,10 +202,8 @@ pub async fn list_worktrees(repo_path: String) -> Result<WorktreeListResult, Str
             if let Some(item) = current.take() {
                 worktrees.push(item);
             }
-            // Normalize path to forward slashes for consistent frontend comparison
-            let normalized_path = rest.replace('\\', "/");
             current = Some(WorktreeInfo {
-                path: normalized_path,
+                path: path_to_frontend(Path::new(rest)),
                 head: None,
                 branch: None,
                 detached: false,

src/lib/components/TerminalContainer.svelte

@@ -37,6 +37,18 @@
   let counter = 0;
   let lastLaunchRequestId = $state<string | null>(null);
 
+  // Auto-spawn the first session when the component mounts and a default shell is available.
+  // The plain (non-reactive) `_autoSpawned` flag ensures this runs only once even if
+  // `defaultShell` later changes, and prevents a reactive loop since `sessions` is never
+  // read inside this effect.
+  let _autoSpawned = false;
+  $effect(() => {
+    if (!_autoSpawned && defaultShell) {
+      _autoSpawned = true;
+      addSession(defaultShell);
+    }
+  });
+
   // Panel component refs — used to forward focus() and refit().
   // Plain object (not $state) because we don't need reactivity on the refs themselves.
   const panelInstances: Partial<Record<string, { focus: () => void; refit: () => void } | null>> =