feat(agent-chat): reply_to param, installer isolation, pg-env precedence (#548, #569, #403)

CHANGELOG.md

@@ -1,5 +1,36 @@
 # Changelog
 
+### Batch: agent-chat-reply-to-548 (Issues #548, #569, #403)
+
+#### Added
+- **`send_agent_message()` gains a 5th positional parameter `p_reply_to integer DEFAULT NULL`** (nova-mind#548) — `database/agent-chat/migrations/001-send-agent-message-reply-to.sql` drops all historical overload signatures (3-arg, 4-arg, 5-arg) before recreating the final 5-arg function, so exactly one `public.send_agent_message` overload exists after applying. Both canonical schema files (`database/agent-chat/schema.sql`, and the deprecated-but-kept-in-sync `cognition/focus/agent_chat/schema.sql`) were synced to the same 5-arg definition, including the same defensive `DROP FUNCTION IF EXISTS` guard for all three historical signatures immediately before `CREATE OR REPLACE` — this prevents a transient function-overload-ambiguity window when `agent-install.sh` re-applies `schema.sql` against a live database that still has the pre-#548 4-arg signature. `agent_chat.expires_at timestamptz` (with a partial index `WHERE expires_at IS NOT NULL`) was also added, computed from the existing but previously-unused `p_ttl` parameter.
+- **`send_agent_message()` now validates `LOWER(p_sender) = session_user`** (nova-mind#548) — Uses `session_user` (the actual connected role), not `current_user` (which the function's own `SECURITY DEFINER` context sets to the function owner, `postgres`), so a caller cannot spoof another agent's identity by passing a false `p_sender` argument.
+- **`send_agent_message()` rejects self-addressed messages** (nova-mind#548) — Raises an exception if the (lowercased) sender appears in its own (lowercased) recipients array; there is no legitimate use case, this always indicates a typo.
+- **`enforce_agent_chat_function_use()` trigger rewritten to check `current_user = 'postgres'`** (nova-mind#548) — Replaces the prior `SET LOCAL agent_chat.bypass_gate = 'on'` session-variable gate. `send_agent_message()` is `SECURITY DEFINER` and owned by `postgres` (see #569 below), so `current_user = 'postgres'` inside its body; the trigger now authorizes DML on that basis, which cannot be spoofed by a session variable a caller might set directly. The trigger also now blocks direct `UPDATE`/`DELETE` in addition to `INSERT` (messages are immutable), still allowing the logical-replication apply worker to pass through.
+- **`cognition/focus/agent_chat/src/channel.ts`: reply insertion is now a single atomic `send_agent_message(...)` call** (nova-mind#548) — `insertOutboundMessage()` passes `replyTo` as `p_reply_to` via named-argument SQL (`p_ttl => $4::interval, p_reply_to => $5::integer`) in the same `SECURITY DEFINER` call that performs the insert, instead of a separate `UPDATE agent_chat SET reply_to = ...` after the insert. The old two-step pattern's `UPDATE` was rejected by the DML lockdown trigger in some call paths; the new atomic insert has no such gap. `insertOutboundMessage` and `processAgentChatMessage` are now exported for testability.
+- **FK-violation (SQLSTATE 23503) on `reply_to` handled as a distinct error class** (nova-mind#548) — An invalid/deleted parent message id now logs `Reply for message <id> rejected: invalid reply_to (foreign key violation)` instead of the generic failure message, so operators can distinguish this class from other reply failures.
+- **`markMessageFailed` now reachable from inside `deliver()`'s catch block** (nova-mind#548) — The real OpenClaw runtime's reply dispatcher wraps `deliver()` in a `.then().catch(onError)` promise chain, which swallows a thrown error before it reaches `processAgentChatMessage`'s outer `catch`. `channel.ts` now calls `markMessageFailed()` directly inside `deliver()`'s catch, before re-throwing, so `agent_chat_processed.status` is reliably set to `'failed'` regardless of whether the dispatcher's own catch ever inspects the rejection.
+- **`markMessageRouted()` guarded against clobbering a terminal status** (nova-mind#548) — The `UPDATE` now includes `AND status NOT IN ('failed', 'responded')`, preventing a downstream "routed" transition from overwriting a terminal status (e.g. `failed`, written by the `markMessageFailed` fix above) already recorded earlier in the same reply cycle.
+- **`agent-install.sh` `agentChatDatabase` config key + refusal guard** (nova-mind#569) — The installer resolves the `agent_chat` bus target database via `agentChatDatabase` (top-level string key in `~/.openclaw/postgres.json`) → `AGENT_CHAT_DB_NAME` env var → default `agent_chat`, and persists the resolved value back to `postgres.json`. If the resolved name is the literal production database name `agent_chat` and the installer is not running as the `nova` unix account (checked via `whoami`, not `$PGUSER` — not spoofable via env), it hard-refuses and exits non-zero, preventing a staging/dev install from mutating the shared production bus.
+- **`agent-install.sh` applies `database/agent-chat/schema.sql` unconditionally before running migrations** (nova-mind#548) — Guarantees a fresh install has the tables/triggers/functions that migration files assume already exist, since the schema file is fully idempotent (`CREATE IF NOT EXISTS`/`CREATE OR REPLACE`).
+- **Schema files explicitly assign `send_agent_message()` ownership to `postgres`** (nova-mind#569) — `ALTER FUNCTION ... OWNER TO postgres`, wrapped in a `DO` block that catches `insufficient_privilege` and logs a `RAISE NOTICE` instead of failing, so a non-superuser devtest apply still succeeds. Without this, a non-postgres superuser applying the schema/migration becomes the function owner, and the `current_user = 'postgres'` trigger check (above) then denies every agent's `send_agent_message()` call with permission-denied.
+
+#### Fixed
+- **Per-field section-over-ENV precedence ported to all three TypeScript `loadPgEnv()` copies** (nova-mind#403) — `lib/pg-env.ts`, `memory/lib/pg-env.ts`, and `cognition/focus/agent_chat/lib/pg-env.ts` now apply the same per-field contract Python's `load_pg_env()` gained in #405: a field explicitly defined (non-null, non-empty) inside a requested `section` wins over a pre-exported ambient ENV var for that field only; fields the section omits are unaffected and still fall through to ENV → flat-config → default. Closes the gap where a gateway shell already exporting `PGDATABASE=nova_memory` could override `cognition/focus/agent_chat/src/channel.ts`'s `loadPgEnv(undefined, "agent_chat")` call — a confirmed affected caller prior to this fix. Ports the Python TC-30–TC-43 test coverage to the TS test suites, including a regression test for the staging failure mode described above.
+
+#### Migrations
+- `database/agent-chat/migrations/001-send-agent-message-reply-to.sql` (nova-mind#548) — Drops all historical `send_agent_message` overloads, recreates the 5-arg function with `p_reply_to`, re-applies `ALTER FUNCTION ... OWNER TO postgres` (wrapped for non-superuser applies), and re-grants `EXECUTE` to `victoria`/`nova-staging`.
+
+#### Tests
+- `cognition/focus/agent_chat/tests/agent-chat-function.test.mjs`, `cognition/focus/agent_chat/tests/channel-insert.test.mjs` (nova-mind#548) — Node built-in test suites: migration mechanics, overload-ambiguity avoidance, `send_agent_message()` behavior (reply_to, named args, backward compatibility, TTL, FK violations, validation guards, atomicity, ownership/ownership-guard skip-on-non-superuser), `insertOutboundMessage`'s single-query/no-UPDATE contract, and `processAgentChatMessage`'s FK-violation logging + `markMessageFailed`/`markMessageRouted` status-guard behavior.
+- `tests/install/test_agent_chat_installer.bats` (nova-mind#569) — `postgres.json` `agentChatDatabase` assertions and refusal-guard simulation for a non-nova user targeting `agent_chat`.
+- `lib/pg-env.test.ts`, `memory/lib/pg-env.test.ts`, `cognition/focus/agent_chat/lib/pg-env.test.ts`, `memory/tests/test-pg-env.ts` (nova-mind#403) — Ported Python TC-30–TC-43 coverage; includes the `PGDATABASE`-shadowing regression test for the `agent_chat` section.
+
+#### Issues Closed
+- #548 — `send_agent_message()` reply_to param + atomic insert (replaces insert-then-UPDATE that hit the DML lockdown trigger)
+- #569 — Installer agent_chat provisioning: parameterized database target + production-mutation refusal guard
+- #403 — TypeScript `loadPgEnv()` per-field section precedence over ENV (parity with Python #405)
+
 ### Batch: completion-log-reconcile-561 (Issue #561)
 
 #### Added

agent-install.sh

@@ -72,10 +72,25 @@ else
     exit 1
 fi
 
+# Resolve agent_chat database target.
+# Prefer the top-level key agentChatDatabase in ~/.openclaw/postgres.json,
+# then the AGENT_CHAT_DB_NAME environment override, then the production
+# default 'agent_chat'. Staging installs set agentChatDatabase to an isolated
+# name (e.g. 'agent_chat_staging') so they do not mutate the shared production
+# agent_chat bus (nova-mind#569).
+_resolve_agent_chat_db_name() {
+    local pg_config="${HOME}/.openclaw/postgres.json"
+    local configured=""
+    if [ -f "$pg_config" ] && command -v jq &>/dev/null; then
+        configured=$(jq -r '.agentChatDatabase // ""' "$pg_config" 2>/dev/null || true)
+    fi
+    printf '%s' "${configured:-${AGENT_CHAT_DB_NAME:-agent_chat}}"
+}
+
 # Derived variables
 DB_USER="${PGUSER:-$(whoami)}"
 DB_NAME="${PGDATABASE:-${DB_USER//-/_}_memory}"
-AGENT_CHAT_DB_NAME="agent_chat"
+AGENT_CHAT_DB_NAME="$(_resolve_agent_chat_db_name)"
 
 # PostgreSQL password file path
 PGPASS_FILE="${HOME}/.pgpass"
@@ -201,15 +216,16 @@ _ensure_agent_chat_postgres_json() {
 
     local new_json
     new_json=$(jq --arg db "$database" --arg user "$user" --arg pass "$password" \
-        'if (.agent_chat // null) | type == "object" then
-            .agent_chat |= . + {
-                database: (.database // $db),
-                user: (.user // $user),
-                password: (.password // $pass)
-            }
-         else
-            .agent_chat = {"database": $db, "user": $user, "password": $pass}
-         end' "$pg_config" 2>/dev/null) || return 1
+        'if (.agentChatDatabase // null) | type == "string" then . else .agentChatDatabase = $db end
+         | if (.agent_chat // null) | type == "object" then
+             .agent_chat |= . + {
+                 database: (.database // $db),
+                 user: (.user // $user),
+                 password: (.password // $pass)
+             }
+           else
+             .agent_chat = {"database": $db, "user": $user, "password": $pass}
+           end' "$pg_config" 2>/dev/null) || return 1
 
     # Only write if something changed.
     if [ "$(printf '%s\n' "$new_json" | jq -Sc .)" = "$(jq -Sc . < "$pg_config")" ]; then
@@ -486,6 +502,75 @@ _install_pg_notify_listener() {
     fi
 }
 
+# Apply sorted .sql migrations from database/agent-chat/migrations/ against the
+# dedicated agent_chat database. Hard failure on any migration error so the DB
+# cannot be left in a half-migrated state.
+_apply_agent_chat_migrations() {
+    local db_name="${1:-$AGENT_CHAT_DB_NAME}"
+    local migrations_dir="$SCRIPT_DIR/database/agent-chat/migrations"
+
+    # Belt-and-braces guard: the literal production database name must only be
+    # touched by the production unix account. This prevents a staging install
+    # from mutating the shared production agent_chat bus (nova-mind#569).
+    if [ "$db_name" = "agent_chat" ] && [ "$(whoami)" != "nova" ]; then
+        echo -e "  ${CROSS_MARK} Refusing to target production agent_chat database as user '$(whoami)'"
+        echo "      This install is running against the shared production Postgres cluster."
+        echo "      Set AGENT_CHAT_DB_NAME or configure agentChatDatabase in ~/.openclaw/postgres.json"
+        echo "      to point at an isolated staging database (e.g. 'agent_chat_staging')."
+        exit 1
+    fi
+
+    if [ ! -d "$migrations_dir" ]; then
+        return 0
+    fi
+
+    local mig_files=()
+    while IFS= read -r -d '' f; do
+        mig_files+=("$f")
+    done < <(find "$migrations_dir" -maxdepth 1 -name "*.sql" -print0 | sort -z)
+
+    if [ ${#mig_files[@]} -eq 0 ]; then
+        return 0
+    fi
+
+    # Ensure the dedicated agent_chat database exists before applying migrations.
+    if ! psql -U "$DB_USER" -lqt | cut -d \| -f 1 | grep -qw "$db_name"; then
+        echo "  Creating agent_chat database '$db_name'..."
+        _superuser_createdb "$db_name"
+        echo -e "  ${CHECK_MARK} Created database '$db_name'"
+    fi
+
+    # Apply the canonical base schema before migrations. The schema file is
+    # idempotent (CREATE IF NOT EXISTS / CREATE OR REPLACE), so applying it on
+    # every install run is safe and guarantees a fresh install gets the tables,
+    # triggers, and functions that migrations assume already exist.
+    local schema_file="$SCRIPT_DIR/database/agent-chat/schema.sql"
+    if [ ! -f "$schema_file" ]; then
+        echo -e "  ${CROSS_MARK} agent_chat schema file not found: $schema_file"
+        exit 1
+    fi
+
+    echo "  Applying agent_chat base schema..."
+    if _superuser_psql "$db_name" -v ON_ERROR_STOP=1 -f "$schema_file" >/dev/null 2>&1; then
+        echo -e "  ${CHECK_MARK} Base schema applied"
+    else
+        echo -e "  ${CROSS_MARK} Base schema apply failed"
+        exit 1
+    fi
+
+    echo "  Applying agent_chat migrations..."
+    for sql_file in "${mig_files[@]}"; do
+        local mig_name
+        mig_name=$(basename "$sql_file")
+        if _superuser_psql "$db_name" -v ON_ERROR_STOP=1 -f "$sql_file" >/dev/null 2>&1; then
+            echo -e "    ${CHECK_MARK} Migration: $mig_name"
+        else
+            echo -e "    ${CROSS_MARK} Migration failed: $mig_name"
+            exit 1
+        fi
+    done
+}
+
 echo "  Agent DB user: $DB_USER"
 if [ "$PG_SUPERUSER" != "$DB_USER" ]; then
     echo "  Superuser:     $PG_SUPERUSER (for DDL operations)"
@@ -1023,7 +1108,7 @@ verify_cognition() {
 
     # agent_chat tables live in the dedicated agent_chat DB, not the memory DB.
     local agent_chat_db
-    agent_chat_db=$(jq -r '.agent_chat.database // "agent_chat"' "$PG_CONFIG" 2>/dev/null || echo "agent_chat")
+    agent_chat_db=$(jq -r '.agentChatDatabase // "agent_chat"' "$PG_CONFIG" 2>/dev/null || echo "agent_chat")
     if psql -U "$DB_USER" -d "$agent_chat_db" -c '\q' >/dev/null 2>&1; then
         local required_tables=("agent_chat" "agent_chat_processed")
         for table in "${required_tables[@]}"; do
@@ -1908,7 +1993,8 @@ fi
 # --- agent_chat runtime configuration ---
 # The agent_chat messaging bus now lives in a dedicated `agent_chat` database.
 # Schema/objects for that database are managed by database/agent-chat/schema.sql
-# and applied by scripts/agent-chat-migration/migrate.sh, not by this installer.
+# and migrations under database/agent-chat/migrations/, which this installer
+# applies automatically after the extension is built.
 # Logical replication for agent_chat (#64/#67) was superseded by the shared-DB
 # design and is no longer configured here.
 echo ""
@@ -1934,6 +2020,10 @@ else
     echo -e "  ${WARNING} PGPASSWORD not set — skipping ~/.pgpass provisioning"
 fi
 
+# Preserve an explicit agent_chat database name (agentChatDatabase) in
+# postgres.json so the runtime and future installer runs resolve the same
+# target. Production default is 'agent_chat'; staging should set this to an
+# isolated name (e.g. 'agent_chat_staging') before running the installer.
 if _ensure_agent_chat_postgres_json "$PG_CONFIG" "$AGENT_CHAT_DB_NAME" "$DB_USER" "${PGPASSWORD:-}"; then
     echo -e "  ${CHECK_MARK} Wrote nested agent_chat section to $PG_CONFIG"
 else
@@ -2054,6 +2144,9 @@ else
     echo -e "  ${WARNING} cognition/focus/agent_chat not found (skipping extension)"
 fi
 
+# Apply agent_chat database migrations after the extension source is in place.
+_apply_agent_chat_migrations "$AGENT_CHAT_DB_NAME"
+
 # --- Cognition focus skills (managed tier — all sessions) ---
 echo ""
 echo "Cognition focus skills installation..."

cognition/CHANGELOG.md

@@ -2,6 +2,21 @@
 
 ## Unreleased
 
+### Fixed (#548/#569/#403 — agent_chat reply_to param, atomic insert, installer provisioning, pg-env TS parity)
+
+See root `CHANGELOG.md` (batch `agent-chat-reply-to-548`) for full detail. Summary of the `cognition/` pieces:
+
+- **`send_agent_message()` gains a 5th positional parameter `p_reply_to integer DEFAULT NULL`** (#548) — `cognition/focus/agent_chat/schema.sql` (marked deprecated post-#320, kept in sync with the canonical `database/agent-chat/schema.sql` to prevent drift) was updated identically: session_user sender validation, self-address guard, `expires_at` column/index, and the `postgres`-owned `SECURITY DEFINER` function with a defensive `DROP FUNCTION IF EXISTS` for all three historical signatures before `CREATE OR REPLACE`.
+- **`enforce_agent_chat_function_use()` rewritten to gate on `current_user = 'postgres'`** (#548) — replaces the old `agent_chat.bypass_gate` session-variable check; also now blocks direct `UPDATE`/`DELETE`, not just `INSERT`.
+- **`cognition/focus/agent_chat/src/channel.ts`: `insertOutboundMessage()` now passes `p_reply_to` in the same atomic `send_agent_message()` call** (#548) — removes the separate post-insert `UPDATE agent_chat SET reply_to = ...` that the DML lockdown trigger could reject. FK violations (SQLSTATE 23503) on an invalid `reply_to` are logged as a distinct error class. `markMessageFailed()` is now called directly inside `deliver()`'s catch (the real dispatcher's `.then().catch(onError)` chain swallows a plain throw before it reaches the outer catch), and `markMessageRouted()`'s `UPDATE` now guards `AND status NOT IN ('failed', 'responded')` so it cannot clobber a terminal status written earlier in the same cycle. `insertOutboundMessage`/`processAgentChatMessage` are exported for testability.
+- **`cognition/focus/agent_chat/lib/pg-env.ts` per-field section precedence over ENV** (#403) — Same fix as the canonical `lib/pg-env.ts`/`memory/lib/pg-env.ts` (see root `CHANGELOG.md`): a field explicitly defined in the `agent_chat` section now wins over a pre-exported ambient ENV var for that field only. Closes a confirmed gap in `channel.ts`'s `loadPgEnv(undefined, "agent_chat")` call.
+- **Installer `agentChatDatabase` provisioning + refusal guard** (#569) — `agent-install.sh`'s agent_chat DB target now resolves via `agentChatDatabase` (`postgres.json`) → `AGENT_CHAT_DB_NAME` env → `agent_chat` default, persisted back to `postgres.json`. Refuses to target the literal production `agent_chat` name unless running as the `nova` unix account. Base schema is now applied unconditionally before the migrations loop.
+
+#### Tests (#548/#569/#403)
+- `cognition/focus/agent_chat/tests/agent-chat-function.test.mjs`, `cognition/focus/agent_chat/tests/channel-insert.test.mjs` — SQL and channel-level coverage for the reply_to param, atomicity, ownership, and status-guard behavior.
+- `cognition/focus/agent_chat/lib/pg-env.test.ts` — ported Python TC-30–TC-43 per-field precedence coverage.
+- `tests/install/test_agent_chat_installer.bats` — `agentChatDatabase` assertions and refusal-guard simulation.
+
 ### Fixed (#508 — pg-notify-listener alerts use PGUSER sender and self-safe recipients)
 
 - **`pg-notify-listener.py` alerts (`_send_push_alert` and `_send_branch_alert`) now use connecting PGUSER as sender** ([#508](https://github.com/NOVA-Openclaw/nova-mind/issues/508)) — Replaced the hardcoded `'schema-sync'` sender string with dynamic `_agent_chat_env.get('PGUSER')` in both alert paths. Since `send_agent_message()` enforces `LOWER(p_sender) == session_user` and no `'schema-sync'` database role exists, every listener alert had silently failed to deliver in production since 2026-07-12.