fix(terminal): support PostgreSQL task terminals (#3371) (#3378)

apps/backend/internal/terminal/repository/postgres_test.go

@@ -0,0 +1,146 @@
+package repository
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/kandev/kandev/internal/terminal/models"
+	"github.com/kandev/kandev/internal/testutil"
+)
+
+// @covers AC-TASKS-TASK-TERMINALS-001.1, AC-TASKS-TASK-TERMINALS-001.2,
+// AC-TASKS-TASK-TERMINALS-001.3
+func TestPostgresTerminalRepositoryLifecycle(t *testing.T) {
+	t.Run("create get and rename", testPostgresTerminalCreateGetRename)
+	t.Run("list and state", testPostgresTerminalListAndState)
+	t.Run("delete", testPostgresTerminalDelete)
+}
+
+func newPostgresTerminalTestRepo(t *testing.T) *Repository {
+	t.Helper()
+	db := testutil.OpenIsolatedPostgres(t, testutil.PostgresDSNFromEnv(t))
+	repo, err := NewWithDB(db, db, nil)
+	if err != nil {
+		t.Fatalf("new postgres repository: %v", err)
+	}
+	return repo
+}
+
+func testPostgresTerminalCreateGetRename(t *testing.T) {
+	repo := newPostgresTerminalTestRepo(t)
+	ctx := context.Background()
+
+	first, err := repo.Create(ctx, "task-1", "env-1", "terminal-1", "make dev")
+	if err != nil {
+		t.Fatalf("create first terminal: %v", err)
+	}
+	second, err := repo.Create(ctx, "task-1", "env-1", "terminal-2", "")
+	if err != nil {
+		t.Fatalf("create second terminal: %v", err)
+	}
+	if first.Seq != 1 || first.State != models.StateOpen || first.InitialCommand != "make dev" {
+		t.Fatalf("first terminal = %#v, want seq 1, open state, and initial command", first)
+	}
+	if second.Seq != 2 {
+		t.Fatalf("second terminal seq = %d, want 2", second.Seq)
+	}
+
+	name := "build watcher"
+	if err := repo.Rename(ctx, first.ID, &name); err != nil {
+		t.Fatalf("rename terminal: %v", err)
+	}
+	got, err := repo.Get(ctx, first.ID)
+	if err != nil {
+		t.Fatalf("get terminal: %v", err)
+	}
+	if got.CustomName == nil || *got.CustomName != name {
+		t.Fatalf("custom name = %v, want %q", got.CustomName, name)
+	}
+	if err := repo.Rename(ctx, first.ID, nil); err != nil {
+		t.Fatalf("clear terminal name: %v", err)
+	}
+	got, err = repo.Get(ctx, first.ID)
+	if err != nil {
+		t.Fatalf("get terminal after clear: %v", err)
+	}
+	if got.CustomName != nil {
+		t.Fatalf("custom name after clear = %v, want nil", got.CustomName)
+	}
+}
+
+func testPostgresTerminalListAndState(t *testing.T) {
+	repo := newPostgresTerminalTestRepo(t)
+	ctx := context.Background()
+	first, err := repo.Create(ctx, "task-1", "env-1", "terminal-1", "")
+	if err != nil {
+		t.Fatalf("create first terminal: %v", err)
+	}
+	second, err := repo.Create(ctx, "task-1", "env-1", "terminal-2", "")
+	if err != nil {
+		t.Fatalf("create second terminal: %v", err)
+	}
+	if err := repo.SetState(ctx, second.ID, models.StateParked); err != nil {
+		t.Fatalf("park terminal: %v", err)
+	}
+	open, err := repo.ListByTask(ctx, "task-1", false)
+	if err != nil {
+		t.Fatalf("list open terminals: %v", err)
+	}
+	if len(open) != 1 || open[0].ID != first.ID {
+		t.Fatalf("open terminals = %#v, want only %q", open, first.ID)
+	}
+	all, err := repo.ListByTask(ctx, "task-1", true)
+	if err != nil {
+		t.Fatalf("list all terminals: %v", err)
+	}
+	if len(all) != 2 || all[0].ID != first.ID || all[1].ID != second.ID {
+		t.Fatalf("all terminals = %#v, want %q then %q", all, first.ID, second.ID)
+	}
+
+	if err := repo.SetState(ctx, second.ID, models.StateOpen); err != nil {
+		t.Fatalf("resume terminal: %v", err)
+	}
+	open, err = repo.ListByTask(ctx, "task-1", false)
+	if err != nil {
+		t.Fatalf("list resumed terminals: %v", err)
+	}
+	if len(open) != 2 {
+		t.Fatalf("resumed terminal count = %d, want 2", len(open))
+	}
+}
+
+func testPostgresTerminalDelete(t *testing.T) {
+	repo := newPostgresTerminalTestRepo(t)
+	ctx := context.Background()
+	first, err := repo.Create(ctx, "task-1", "env-1", "terminal-1", "")
+	if err != nil {
+		t.Fatalf("create first terminal: %v", err)
+	}
+	if _, err := repo.Create(ctx, "task-1", "env-1", "terminal-2", ""); err != nil {
+		t.Fatalf("create second terminal: %v", err)
+	}
+	if _, err := repo.Create(ctx, "task-2", "env-2", "terminal-3", ""); err != nil {
+		t.Fatalf("create other task terminal: %v", err)
+	}
+	if err := repo.Delete(ctx, first.ID); err != nil {
+		t.Fatalf("delete terminal: %v", err)
+	}
+	if _, err := repo.Get(ctx, first.ID); !errors.Is(err, ErrNotFound) {
+		t.Fatalf("get deleted terminal error = %v, want ErrNotFound", err)
+	}
+	deleted, err := repo.DeleteByTask(ctx, "task-1")
+	if err != nil {
+		t.Fatalf("delete task terminals: %v", err)
+	}
+	if deleted != 1 {
+		t.Fatalf("deleted task terminal count = %d, want 1", deleted)
+	}
+	other, err := repo.ListByTask(ctx, "task-2", true)
+	if err != nil {
+		t.Fatalf("list other task terminals: %v", err)
+	}
+	if len(other) != 1 || other[0].ID != "terminal-3" {
+		t.Fatalf("other task terminals = %#v, want terminal-3", other)
+	}
+}

apps/backend/internal/terminal/repository/sqlite.go

@@ -74,9 +74,9 @@ func (r *Repository) initSchema() error {
 // agentctl PTY id). initialCommand is "" for plain shells.
 func (r *Repository) Create(ctx context.Context, taskID, envID, id, initialCommand string) (*models.Terminal, error) {
 	if _, err := r.db.ExecContext(ctx,
-		`INSERT INTO user_terminals (id, task_id, environment_id, seq, custom_name, state, initial_command)
+		r.db.Rebind(`INSERT INTO user_terminals (id, task_id, environment_id, seq, custom_name, state, initial_command)
 		 SELECT ?, ?, ?, COALESCE(MAX(seq), 0) + 1, NULL, 'open', ?
-		 FROM user_terminals WHERE task_id = ?`,
+		 FROM user_terminals WHERE task_id = ?`),
 		id, taskID, envID, initialCommand, taskID,
 	); err != nil {
 		return nil, fmt.Errorf("insert terminal: %w", err)
@@ -89,8 +89,8 @@ func (r *Repository) Create(ctx context.Context, taskID, envID, id, initialComma
 func (r *Repository) Get(ctx context.Context, id string) (*models.Terminal, error) {
 	var t models.Terminal
 	err := r.ro.GetContext(ctx, &t,
-		`SELECT id, task_id, environment_id, seq, custom_name, state, initial_command, created_at
-		 FROM user_terminals WHERE id = ?`,
+		r.ro.Rebind(`SELECT id, task_id, environment_id, seq, custom_name, state, initial_command, created_at
+		 FROM user_terminals WHERE id = ?`),
 		id,
 	)
 	if errors.Is(err, sql.ErrNoRows) {
@@ -112,6 +112,7 @@ func (r *Repository) ListByTask(ctx context.Context, taskID string, includeParke
 		q += ` AND state = 'open'`
 	}
 	q += ` ORDER BY seq ASC`
+	q = r.ro.Rebind(q)
 
 	var rows []*models.Terminal
 	if err := r.ro.SelectContext(ctx, &rows, q, args...); err != nil {
@@ -130,7 +131,7 @@ func (r *Repository) Rename(ctx context.Context, id string, name *string) error
 		nameArg = *name
 	}
 	res, err := r.db.ExecContext(ctx,
-		`UPDATE user_terminals SET custom_name = ? WHERE id = ?`,
+		r.db.Rebind(`UPDATE user_terminals SET custom_name = ? WHERE id = ?`),
 		nameArg, id,
 	)
 	if err != nil {
@@ -147,7 +148,7 @@ func (r *Repository) Rename(ctx context.Context, id string, name *string) error
 // the service's job.
 func (r *Repository) SetState(ctx context.Context, id string, state models.TerminalState) error {
 	res, err := r.db.ExecContext(ctx,
-		`UPDATE user_terminals SET state = ? WHERE id = ?`,
+		r.db.Rebind(`UPDATE user_terminals SET state = ? WHERE id = ?`),
 		string(state), id,
 	)
 	if err != nil {
@@ -162,7 +163,7 @@ func (r *Repository) SetState(ctx context.Context, id string, state models.Termi
 
 // Delete removes a single terminal row.
 func (r *Repository) Delete(ctx context.Context, id string) error {
-	_, err := r.db.ExecContext(ctx, `DELETE FROM user_terminals WHERE id = ?`, id)
+	_, err := r.db.ExecContext(ctx, r.db.Rebind(`DELETE FROM user_terminals WHERE id = ?`), id)
 	if err != nil {
 		return fmt.Errorf("delete terminal: %w", err)
 	}
@@ -172,7 +173,7 @@ func (r *Repository) Delete(ctx context.Context, id string) error {
 // DeleteByTask removes every row for a task and returns the count. Used by
 // the task.deleted cascade subscriber.
 func (r *Repository) DeleteByTask(ctx context.Context, taskID string) (int, error) {
-	res, err := r.db.ExecContext(ctx, `DELETE FROM user_terminals WHERE task_id = ?`, taskID)
+	res, err := r.db.ExecContext(ctx, r.db.Rebind(`DELETE FROM user_terminals WHERE task_id = ?`), taskID)
 	if err != nil {
 		return 0, fmt.Errorf("delete by task: %w", err)
 	}

docs/plans/postgres-task-terminal-queries/plan.md

@@ -0,0 +1,78 @@
+---
+created: 2026-09-04
+status: done
+requirements:
+  - REQ-TASKS-TASK-TERMINALS-001
+system_design:
+  - ../../specs/tasks/system-design/task-terminal-persistence.md
+legacy_specs: []
+---
+
+# Implementation Plan: PostgreSQL Task Terminal Queries
+
+## Overview
+
+Make every parameterized ordinary task-terminal query use placeholder syntax
+for the selected database driver. One work order adds PostgreSQL lifecycle
+coverage first, applies the mechanical repository correction, and runs the
+targeted backend checks.
+
+## Scope
+
+### In scope
+
+- Add an environment-gated PostgreSQL regression for the ordinary terminal
+  repository lifecycle.
+- Rebind every parameterized read and write in the ordinary terminal
+  repository.
+- Preserve existing SQLite behavior and error wrapping.
+
+### Out of scope
+
+- Changes to terminal service, WebSocket handlers, or web components.
+- Changes to terminal sequence allocation or concurrency semantics.
+- New database schema or migrations.
+- New terminal error presentation.
+
+## Technical approach
+
+Add `postgres_test.go` beside the existing SQLite repository tests. The test
+constructs the real repository over an isolated PostgreSQL schema and exercises
+all parameterized lifecycle methods. It must first fail with PostgreSQL syntax
+error `42601` on the current implementation.
+
+Update `apps/backend/internal/terminal/repository/sqlite.go` so reads call
+`r.ro.Rebind` and writes call `r.db.Rebind` before passing statements to sqlx.
+Do not change schema statements because they have no placeholders.
+
+## Tests
+
+- `AC-TASKS-TASK-TERMINALS-001.1` maps to PostgreSQL create and get assertions
+  plus the existing SQLite sequence tests.
+- `AC-TASKS-TASK-TERMINALS-001.2` maps to PostgreSQL open/all list assertions
+  and ordered sequence assertions.
+- `AC-TASKS-TASK-TERMINALS-001.3` maps to PostgreSQL rename, state, single
+  delete, and task-wide delete assertions.
+
+## E2E tests
+
+The existing browser terminal suites cover the unchanged user, WebSocket, and
+service flow. The new real-PostgreSQL lifecycle test covers the driver boundary
+that caused the end-to-end failure. No frontend or protocol behavior changes.
+
+## Work orders
+
+- [x] [Task 01: Rebind Task Terminal Queries](task-01-rebind-task-terminal-queries.md)
+
+## Verification results
+
+- The PostgreSQL lifecycle regression passed three subtests on PostgreSQL 16.
+- The terminal package suite passed 29 tests in three packages.
+- The changed-file backend linter reported zero issues.
+
+## Risks
+
+- Omitting one parameterized method can leave a later lifecycle action broken
+  on PostgreSQL even if creation starts working.
+- PostgreSQL integration coverage skips when `KANDEV_TEST_POSTGRES_DSN` is not
+  configured, so the implementation run must provide an isolated database.

docs/plans/postgres-task-terminal-queries/task-01-rebind-task-terminal-queries.md

@@ -0,0 +1,89 @@
+---
+id: "01-rebind-task-terminal-queries"
+title: "Rebind Task Terminal Queries"
+status: done
+wave: 1
+depends_on: []
+plan: "plan.md"
+requirements:
+  - REQ-TASKS-TASK-TERMINALS-001
+acceptance_criteria:
+  - AC-TASKS-TASK-TERMINALS-001.1
+  - AC-TASKS-TASK-TERMINALS-001.2
+  - AC-TASKS-TASK-TERMINALS-001.3
+system_design:
+  - ../../specs/tasks/system-design/task-terminal-persistence.md
+---
+
+# Task 01: Rebind Task Terminal Queries
+
+## Summary
+
+Add a real PostgreSQL regression for the ordinary terminal descriptor
+lifecycle. Then make each parameterized statement use placeholder syntax for
+the selected driver. Preserve the current SQLite and terminal-service contracts.
+
+## In scope
+
+- Add PostgreSQL create, get, list, rename, state, removal, and task-wide removal
+  lifecycle assertions.
+- Rebind all parameterized reads through the reader pool.
+- Rebind all parameterized writes through the writer pool.
+
+## Out of scope
+
+- Schema, API, service, PTY, WebSocket, frontend, and localization changes.
+- Sequence-allocation redesign or new concurrency behavior.
+- Broad repository verification beyond the task-defined checks.
+
+## Acceptance
+
+- The PostgreSQL lifecycle regression fails before the production correction
+  with SQLSTATE `42601` and passes after it.
+- Every parameterized ordinary terminal repository method works against both
+  SQLite and PostgreSQL with unchanged results.
+- The targeted terminal packages and changed-file backend lint pass.
+
+## Verification
+
+```bash
+go test ./internal/terminal/... -count=1
+KANDEV_TEST_POSTGRES_DSN=<isolated-dsn> go test ./internal/terminal/repository -run TestPostgresTerminalRepositoryLifecycle -count=1 -v
+golangci-lint run ./... --new-from-rev="$(git merge-base HEAD origin/main)" --timeout=5m
+```
+
+## Files likely touched
+
+- `apps/backend/internal/terminal/repository/sqlite.go`
+- `apps/backend/internal/terminal/repository/postgres_test.go`
+
+## Dependencies
+
+None.
+
+## Risks
+
+- The PostgreSQL test must use a real isolated schema so a SQLite-only or mock
+  test cannot hide placeholder syntax errors.
+
+## Parallelism
+
+`sequential`
+
+## Inputs
+
+- `REQ-TASKS-TASK-TERMINALS-001` and its acceptance criteria.
+- `docs/specs/tasks/system-design/task-terminal-persistence.md`.
+- GitHub issue #3371 and the isolated PostgreSQL reproduction evidence.
+- Existing terminal repository tests and `internal/testutil` PostgreSQL helpers.
+
+## Results
+
+- Added `postgres_test.go` with create, get, list, rename, state, single-removal,
+  and task-wide removal coverage.
+- Added driver rebinding to `Create`, `Get`, `ListByTask`, `Rename`, `SetState`,
+  `Delete`, and `DeleteByTask`.
+- `TestPostgresTerminalRepositoryLifecycle` failed before the correction with
+  SQLSTATE `42601`. It passed all three subtests after the correction.
+- `go test ./internal/terminal/... -count=1` passed 29 tests in three packages.
+- The changed-file backend linter reported zero issues.

docs/specs/tasks/README.md

@@ -76,6 +76,7 @@ signals, and task-scoped scheduling contracts.
 - [Rich task title previews](requirements/rich-task-title-previews.md)
 - [Queued run scheduling](requirements/run-scheduling.md)
 - [Task Runtime Cleanup](requirements/runtime-cleanup.md)
+- [Task Terminal Persistence](requirements/task-terminal-persistence.md)
 - [Runtime Task-State Publication Order](requirements/runtime-state-publication-order.md)
 - [Session Delete Preserves Task Workspaces](requirements/session-delete-resource-cleanup.md)
 - [Sidebar Task Editing](requirements/sidebar-task-edit.md)
@@ -142,6 +143,7 @@ signals, and task-scoped scheduling contracts.
 - [Task plan write consistency](system-design/plan-write-consistency.md)
 - [Task plan write lifecycle](system-design/plan-write-lifecycle.md)
 - [Task Runtime Cleanup](system-design/runtime-cleanup.md)
+- [Task Terminal Persistence](system-design/task-terminal-persistence.md)
 - [Runtime Task-State Publication Order](system-design/runtime-state-publication-order.md)
 - [Queued Run Scheduling](system-design/run-scheduling.md)
 - [Session Delete Preserves Task Workspaces](system-design/session-delete-resource-cleanup.md)
@@ -158,7 +160,7 @@ signals, and task-scoped scheduling contracts.
 
 ## Migration record
 
-Migration remains in progress. The six requirements above now have
+Migration remains in progress. The seven requirements above now have
 authoritative, wrapper-free requirement/design pairs. Other migrated files still
 need the same extraction before this system can return to a complete migration
 state.

docs/specs/tasks/requirements/task-terminal-persistence.md

@@ -0,0 +1,45 @@
+---
+status: active
+system: tasks
+created: 2026-09-04
+owners:
+  - kandev
+---
+
+# Task Terminal Persistence Requirements
+
+## Overview
+
+Ordinary task terminals are durable task-scoped resources. The task system owns
+their identity and lifecycle. Each record belongs to one task and its execution
+environment. These operations must remain available on every supported
+persistence backend.
+
+## Requirements
+
+### REQ-TASKS-TASK-TERMINALS-001: Persistence backend parity
+
+**Intent:** Users can create and manage ordinary task terminals without the
+configured supported persistence backend changing the terminal lifecycle.
+
+**User story:** As a Kandev user, I want task terminals to work on each supported
+database.
+
+#### Acceptance criteria
+
+- **AC-TASKS-TASK-TERMINALS-001.1:** On SQLite and PostgreSQL, the system shall
+  persist and return each new ordinary task terminal with its sequence and open
+  state.
+- **AC-TASKS-TASK-TERMINALS-001.2:** On SQLite and PostgreSQL, the system shall
+  list matching task terminals in sequence order and apply the requested parked
+  terminal filter.
+- **AC-TASKS-TASK-TERMINALS-001.3:** On SQLite and PostgreSQL, each read, rename,
+  state change, single removal, and task-wide removal shall produce the same
+  persisted result.
+
+## Out of scope
+
+- Changing terminal numbering or concurrent sequence-allocation semantics.
+- Changing PTY process lifetime, output buffering, or WebSocket transport.
+- Changing how the web application presents a terminal persistence failure.
+- Changing Quick Terminal or agent-login terminal behavior.

docs/specs/tasks/system-design/task-terminal-persistence.md

@@ -0,0 +1,74 @@
+---
+status: current
+system: tasks
+requirements:
+  - REQ-TASKS-TASK-TERMINALS-001
+---
+
+# Task Terminal Persistence System Design
+
+## Purpose and boundaries
+
+This design defines persistence-backend parity for ordinary task-terminal
+records. The existing terminal service and WebSocket handlers continue to own
+validation, PTY lifecycle, authorization, and client responses. The repository
+continues to own the `user_terminals` table and its task-scoped lifecycle.
+
+## Requirement mapping
+
+| Requirement | Design section |
+| --- | --- |
+| `REQ-TASKS-TASK-TERMINALS-001` | [Portable query execution](#portable-query-execution), [Test contract](#test-contract) |
+
+## Components and responsibilities
+
+- `apps/backend/internal/terminal/repository` persists ordinary terminal
+  descriptors and initializes `user_terminals`.
+- `apps/backend/internal/terminal/service` preserves the existing ordinary,
+  fixed, and script-terminal boundaries and delegates descriptor persistence to
+  the repository.
+- `apps/backend/internal/agent/handlers` exposes the existing `user_shell.*`
+  WebSocket actions and propagates repository failures.
+
+## Portable query execution
+
+The terminal repository keeps `?` as the source placeholder syntax. Before a
+parameterized statement reaches the database driver, the repository passes it
+through the `sqlx.DB` associated with that operation:
+
+- Writer operations use `r.db.Rebind`.
+- Read operations use `r.ro.Rebind`.
+- Schema statements contain no parameters and execute unchanged.
+
+This follows the established repository convention. SQLite retains `?`
+placeholders, while PostgreSQL receives numbered placeholders. Query text,
+arguments, ordering, filters, and lifecycle semantics otherwise remain
+unchanged.
+
+## Failure and recovery
+
+Database execution errors continue to carry the current repository operation
+context to the terminal service and WebSocket caller. The repair does not add a
+retry or convert a failed persistence operation into an in-memory terminal.
+
+## Persistence
+
+The `user_terminals` schema, unique task-sequence constraint, state values, and
+delete behavior remain unchanged. This repair changes only how parameterized
+statements are prepared for the selected driver and requires no migration.
+
+## Test contract
+
+An environment-gated PostgreSQL repository test uses
+`internal/testutil.OpenIsolatedPostgres` and exercises the complete descriptor
+lifecycle: create, get, filtered and unfiltered list, rename, state changes,
+single delete, and task-wide delete. The existing SQLite tests continue to pin
+sequence allocation and lifecycle behavior. Existing browser terminal tests
+cover the unchanged WebSocket and web flow. The PostgreSQL test covers the only
+driver-specific boundary changed by this repair.
+
+## Related decisions
+
+No architecture decision record is required. Query rebinding is an established
+repository convention, and this repair introduces no new ownership or
+persistence boundary.