Provider catalog, local model discovery, and plan revisions

.gitignore

@@ -1,2 +1,10 @@
 .DS_Store
 .forge/
+
+# Local agent worktrees (scratch checkouts created during development)
+.claude/worktrees/
+
+# Local environment / secrets — the installer writes secrets to ./.env
+.env
+.env.*
+!.env.example

docs/install-uninstall.md

@@ -101,8 +101,11 @@ bash scripts/uninstall.sh --remove-data
 ```
 
 This removes local build artifacts, recorded Forge-only packages, `.env`,
-Docker volumes, recorded PostgreSQL database objects, recorded Ollama models,
-and Forge's local install state.
+Docker volumes, recorded Ollama models, and Forge's local install state. It also
+**drops the Forge application database** named in `DATABASE_URL` (default
+`forge`), which clears saved logins, projects, and task history — so a fresh
+install starts from a clean login. The drop runs whenever PostgreSQL is reachable,
+even if the database existed before Forge installed.
 
 By default it does **not** delete the local project folders Forge created. When
 run interactively it asks whether to delete them, or you can opt in directly:
@@ -111,9 +114,10 @@ run interactively it asks whether to delete them, or you can opt in directly:
 bash scripts/uninstall.sh --remove-data --remove-projects
 ```
 
-This deletes every folder listed in `.forge/project-paths` (the local projects
-Forge created) along with their files. Use `--keep-projects` to skip the prompt
-and always keep them.
+This deletes every local project folder Forge created, along with their files.
+Folders are discovered both from `.forge/project-paths` and from the `projects`
+table in the database (so projects created through the web UI are removed too).
+Use `--keep-projects` to skip the prompt and always keep them.
 
 It still does not remove Homebrew, Linux package managers, Docker
 Desktop/Engine, packages that existed before Forge, or recorded packages that

docs/shipping-roadmap.md

@@ -155,6 +155,24 @@ npm run build             # pass
 5. Add permission checks around project/task access once multi-user behavior is
    productized beyond local operator usage.
 
+## GitHub Authentication
+
+GitHub authentication must happen **in the web UI, and only when the `gh` CLI is
+not already authenticated** (detected via `gh auth status`, the same probe used in
+`web/app/api/health/route.ts`). When the CLI is already logged in, Forge uses that
+token and never prompts.
+
+- **Phase 1 — Personal Access Token (PAT).** When the CLI is not authenticated, the
+  web UI prompts for a PAT, validates it against `GET https://api.github.com/user`,
+  and stores it encrypted via `web/lib/crypto.ts` (the same mechanism as provider
+  keys). Token resolution order for repo operations: stored PAT → `gh` CLI token →
+  legacy per-project `githubTokenEnvVar`. Tracked in
+  [issue #12](https://github.com/Joncallim/Forge/issues/12).
+- **Phase 2 — GitHub OAuth (device flow).** Register a GitHub OAuth app and run the
+  device-code flow in the web UI so the user authorizes Forge without creating a PAT
+  by hand; store the resulting token encrypted. This is the preferred end state once
+  an OAuth app is registered; the PAT path remains as a fallback for headless setups.
+
 ## P2 Autonomous Coding Stage
 
 1. Add a specialist subagent registry and harness model. Each harness should

scripts/uninstall.sh

@@ -114,6 +114,77 @@ project_paths() {
   fi
 }
 
+# Read a key from the first .env file that defines it. The .env files still
+# exist at this point because remove_env_files runs near the end of uninstall.
+read_env_var() {
+  local key="$1" file value
+  for file in "$REPO_ROOT/.env" "$REPO_ROOT/.env.local" "$REPO_ROOT/web/.env" "$REPO_ROOT/web/.env.local"; do
+    [ -f "$file" ] || continue
+    value="$(grep -E "^${key}=" "$file" 2>/dev/null | tail -n1)" || true
+    if [ -n "$value" ]; then
+      value="${value#"${key}="}"
+      value="${value%\"}"; value="${value#\"}"
+      value="${value%\'}"; value="${value#\'}"
+      printf '%s' "$value"
+      return 0
+    fi
+  done
+}
+
+app_database_url() {
+  read_env_var DATABASE_URL
+}
+
+# Local project folders recorded in the database. This catches projects created
+# through the web UI, which the on-disk registry (.forge/project-paths) does not
+# always record. Best-effort: silent when psql or the database is unavailable.
+db_project_paths() {
+  local url
+  url="$(app_database_url)"
+  [ -n "$url" ] || return 0
+  command -v psql >/dev/null 2>&1 || return 0
+  psql "$url" -tAc \
+    "SELECT local_path FROM projects WHERE local_path IS NOT NULL AND local_path <> ''" \
+    2>/dev/null | grep -v '^[[:space:]]*$' || true
+}
+
+# Union of folders from the on-disk registry and the database, de-duplicated
+# while preserving order.
+all_project_paths() {
+  { project_paths; db_project_paths; } | grep -v '^[[:space:]]*$' | awk '!seen[$0]++'
+}
+
+# Guard against deleting paths that are clearly not a project folder Forge
+# created. Project rows can hold arbitrary paths (set via the project update API
+# or edited by hand), so a row pointing at $HOME, the repo checkout, or a
+# top-level directory must never be passed to rm -rf. Mirrors isSafeToDelete in
+# web/app/api/projects/[id]/route.ts: must be absolute, not the filesystem root,
+# not $HOME, not the repo root, and at least two segments deep.
+is_safe_project_path() {
+  local raw="$1" p depth
+  [ -n "$raw" ] || return 1
+  case "$raw" in
+    /*) ;;
+    *) return 1 ;;
+  esac
+  p="$raw"
+  while [ "${#p}" -gt 1 ] && [ "${p%/}" != "$p" ]; do p="${p%/}"; done
+  [ "$p" = "/" ] && return 1
+  [ -n "${HOME:-}" ] && [ "$p" = "$HOME" ] && return 1
+  [ "$p" = "$REPO_ROOT" ] && return 1
+  depth="$(printf '%s\n' "${p#/}" | awk -F/ '{c=0; for (i=1;i<=NF;i++) if ($i != "") c++; print c}')"
+  [ "${depth:-0}" -ge 2 ] || return 1
+  return 0
+}
+
+# Project folders safe to delete, after applying the guard above.
+safe_project_paths() {
+  local dir
+  while IFS= read -r dir; do
+    is_safe_project_path "$dir" && printf '%s\n' "$dir"
+  done < <(all_project_paths)
+}
+
 resolve_remove_projects() {
   [ -n "$REMOVE_PROJECTS" ] && return 0
 
@@ -122,7 +193,7 @@ resolve_remove_projects() {
     return 0
   fi
 
-  if [ -z "$(project_paths)" ]; then
+  if [ -z "$(safe_project_paths)" ]; then
     REMOVE_PROJECTS=0
     return 0
   fi
@@ -131,7 +202,7 @@ resolve_remove_projects() {
   info "Forge created these local project folders:"
   while IFS= read -r project_dir; do
     [ -n "$project_dir" ] && info "  - $project_dir"
-  done < <(project_paths)
+  done < <(safe_project_paths)
   printf "    Delete all of these project folders and their files? [y/N] "
   read -r answer || answer=""
   case "$answer" in
@@ -142,9 +213,17 @@ resolve_remove_projects() {
 
 remove_project_files() {
   [ "$REMOVE_PROJECTS" = "1" ] || return 0
-  [ -n "$(project_paths)" ] || return 0
+  [ -n "$(all_project_paths)" ] || return 0
 
   step "Removing local project folders"
+
+  # Surface anything we refuse to touch so an operator can clean it up by hand.
+  while IFS= read -r project_dir; do
+    [ -n "$project_dir" ] || continue
+    is_safe_project_path "$project_dir" && continue
+    warn "Skipped unsafe project path (not deleted): $project_dir"
+  done < <(all_project_paths)
+
   while IFS= read -r project_dir; do
     [ -n "$project_dir" ] || continue
     if [ ! -e "$project_dir" ]; then
@@ -156,7 +235,7 @@ remove_project_files() {
     else
       rm -rf "$project_dir" && info "Removed $project_dir" || warn "Could not remove $project_dir"
     fi
-  done < <(project_paths)
+  done < <(safe_project_paths)
 
   if [ "$DRY_RUN" != "1" ]; then
     rm -f "$PROJECT_PATHS_FILE" 2>/dev/null || true
@@ -316,6 +395,41 @@ drop_recorded_postgres_data() {
   fi
 }
 
+# Drop the Forge application database named in DATABASE_URL (default: forge),
+# even when the installer did not record creating it (e.g. a reused database).
+# This is what actually removes saved logins, projects, and task history on
+# --remove-data. The recorded role/database cleanup still runs afterwards via
+# drop_recorded_postgres_data, which uses the local maintenance connection.
+drop_app_database() {
+  [ "$KEEP_DATA" = "0" ] || return 0
+
+  local url dbname admin_url
+  url="$(app_database_url)"
+  if [ -n "$url" ]; then
+    dbname="${url##*/}"
+    dbname="${dbname%%\?*}"
+    admin_url="${url%/*}/postgres"
+    if [ -n "$dbname" ]; then
+      step "Removing the Forge application database"
+      if [ "$DRY_RUN" = "1" ]; then
+        info "[dry-run] Drop database $dbname (removes saved logins, projects, and history)"
+      elif ! command -v psql >/dev/null 2>&1; then
+        info "psql is not installed, so database $dbname was left in place. Saved logins remain."
+      elif ! psql "$admin_url" -tAc 'SELECT 1' >/dev/null 2>&1; then
+        info "PostgreSQL is not reachable, so the database was left in place. Saved logins and projects remain."
+      elif psql "$admin_url" -c "DROP DATABASE IF EXISTS \"$dbname\" WITH (FORCE);" >/dev/null 2>&1; then
+        info "Dropped database $dbname (removed saved logins, projects, and task history)."
+      else
+        info "Database $dbname could not be dropped now, so it was left in place."
+      fi
+    fi
+  fi
+
+  # Recorded role/database cleanup via the local maintenance connection.
+  # Idempotent with the URL-based drop above.
+  drop_recorded_postgres_data
+}
+
 stop_docker_services() {
   step "Stopping Docker Compose services"
   if ! command -v docker >/dev/null 2>&1; then
@@ -644,7 +758,7 @@ fi
 
 remove_project_files
 remove_build_artifacts
-drop_recorded_postgres_data
+drop_app_database
 stop_docker_services
 remove_recorded_ollama_models
 remove_recorded_packages

web/__tests__/api.test.ts

@@ -329,6 +329,81 @@ describe('POST /api/tasks/:id/approve — 409 when status is pending', () => {
   })
 })
 
+// ---------------------------------------------------------------------------
+// Suite 3.4b — Change plan: POST /api/tasks/:id/replan
+// ---------------------------------------------------------------------------
+
+describe('POST /api/tasks/:id/replan', () => {
+  beforeEach(() => { vi.clearAllMocks() })
+
+  it('returns 409 when task is not awaiting approval', async () => {
+    mockGetSession.mockResolvedValue(FAKE_SESSION)
+    mockDbSelect.mockReturnValue(chain([{ id: 'task-9', status: 'pending', prompt: 'do x' }]))
+
+    const { POST } = await import('@/app/api/tasks/[id]/replan/route')
+    const req = authRequest('/api/tasks/task-9/replan', {
+      method: 'POST',
+      headers: { 'Content-Type': 'application/json' },
+      body: JSON.stringify({ feedback: 'tweak it' }),
+    })
+    const res = await POST(req as never, { params: Promise.resolve({ id: 'task-9' }) })
+
+    expect(res.status).toBe(409)
+    expect(mockDbUpdate).not.toHaveBeenCalled()
+  })
+
+  it('returns 400 when feedback is missing', async () => {
+    mockGetSession.mockResolvedValue(FAKE_SESSION)
+    mockDbSelect.mockReturnValue(chain([{ id: 'task-9', status: 'awaiting_approval', prompt: 'do x' }]))
+
+    const { POST } = await import('@/app/api/tasks/[id]/replan/route')
+    const req = authRequest('/api/tasks/task-9/replan', {
+      method: 'POST',
+      headers: { 'Content-Type': 'application/json' },
+      body: JSON.stringify({}),
+    })
+    const res = await POST(req as never, { params: Promise.resolve({ id: 'task-9' }) })
+
+    expect(res.status).toBe(400)
+    expect(mockDbUpdate).not.toHaveBeenCalled()
+  })
+
+  it('re-queues the task and appends feedback to the prompt on success', async () => {
+    mockGetSession.mockResolvedValue(FAKE_SESSION)
+    mockDbSelect.mockReturnValue(chain([{ id: 'task-9', status: 'awaiting_approval', prompt: 'do x' }]))
+    mockDbUpdate.mockReturnValue(chain([{ id: 'task-9', status: 'pending', updatedAt: new Date() }]))
+
+    const { POST } = await import('@/app/api/tasks/[id]/replan/route')
+    const req = authRequest('/api/tasks/task-9/replan', {
+      method: 'POST',
+      headers: { 'Content-Type': 'application/json' },
+      body: JSON.stringify({ feedback: 'use a queue instead' }),
+    })
+    const res = await POST(req as never, { params: Promise.resolve({ id: 'task-9' }) })
+
+    expect(res.status).toBe(200)
+    expect(mockDbUpdate).toHaveBeenCalled()
+    expect(mockRedisLpush).toHaveBeenCalledWith('forge:tasks', expect.stringContaining('task-9'))
+  })
+})
+
+// ---------------------------------------------------------------------------
+// Suite 3.4c — Local discovery: POST /api/providers/discover-local auth guard
+// ---------------------------------------------------------------------------
+
+describe('POST /api/providers/discover-local — auth guard', () => {
+  beforeEach(() => { vi.clearAllMocks() })
+
+  it('returns 401 when not authenticated', async () => {
+    mockGetSession.mockResolvedValue(null)
+
+    const { POST } = await import('@/app/api/providers/discover-local/route')
+    const res = await POST(authRequest('/api/providers/discover-local', { method: 'POST' }) as never)
+
+    expect(res.status).toBe(401)
+  })
+})
+
 // ---------------------------------------------------------------------------
 // Suite 3.5 — Agent type sanitisation: PUT /api/agents/../../etc/passwd returns 400
 //
@@ -362,14 +437,15 @@ describe('PUT /api/agents/[type] — path traversal blocked', () => {
 })
 
 // ---------------------------------------------------------------------------
-// Suite 3.6 — Provider validation: POST /api/providers with providerType='ollama'
-//              and no baseUrl returns 400
+// Suite 3.6 — Provider validation: baseUrl is required only for self-hosted
+//              endpoints (custom/litellm). Local runtimes like ollama default to
+//              a known localhost URL, so no baseUrl is required.
 // ---------------------------------------------------------------------------
 
-describe('POST /api/providers — baseUrl required for ollama', () => {
+describe('POST /api/providers — baseUrl requirement', () => {
   beforeEach(() => { vi.clearAllMocks() })
 
-  it('returns 400 when providerType is ollama and baseUrl is missing', async () => {
+  it('allows ollama without baseUrl (defaults to the local endpoint)', async () => {
     mockGetSession.mockResolvedValue(FAKE_SESSION)
 
     const { POST } = await import('@/app/api/providers/route')
@@ -381,15 +457,13 @@ describe('POST /api/providers — baseUrl required for ollama', () => {
         providerType: 'ollama',
         modelId: 'llama3',
         isLocal: true,
-        // no baseUrl
+        // no baseUrl — ollama defaults to http://localhost:11434
       }),
     })
 
     const res = await POST(req as never)
 
-    expect(res.status).toBe(400)
-    const body = await res.json()
-    expect(body.error).toMatch(/baseUrl/i)
+    expect(res.status).toBe(201)
   })
 
   it('returns 400 when providerType is custom and baseUrl is missing', async () => {