Files
vectorize-io__hindsight/scripts/dev/start-oracle.sh
Nicolò Boschi d39a2ca618 feat(oracle): unify migrations under Alembic with dialect dispatcher (#1330)
* feat(oracle): unify migrations under Alembic with dialect dispatcher

Oracle DDL was a 636-line idempotent file (`migrations_oracle.py`) outside
Alembic, which meant no version tracking, no per-tenant version table, and
schema drift every time a PG migration was added without a corresponding
Oracle change. This unifies both backends behind a single Alembic tree.

- New `alembic/_dialect.py::run_for_dialect(pg=, oracle=)` helper. Each
  migration declares `_pg_upgrade` / `_oracle_upgrade` and dispatches based
  on the live connection's dialect.
- `alembic/env.py` is dialect-aware: PG keeps the existing search_path /
  read-write session setup; Oracle uses `ALTER SESSION SET CURRENT_SCHEMA`
  and `DDL_LOCK_TIMEOUT`.
- `alembic/script.py.mako` scaffolds the new pattern by default.
- All 59 existing PG migrations refactored mechanically — bodies moved into
  `_pg_upgrade` / `_pg_downgrade`, top-level dispatchers added.
- New `o1a2b3c4d5e6_oracle_baseline` migration brings a fresh Oracle 23ai
  database to the current schema in one step (PG = no-op). Drops the legacy
  partition-conversion / dedup / `observation_sources` backfill since those
  only existed for pre-baseline Oracle installs we explicitly are not
  supporting.
- `OracleBackend.run_migrations()` now goes through the unified Alembic
  pipeline; `migrations.py` skips the PG-specific advisory lock + pgvector
  setup when the URL is Oracle.
- `migrations_oracle.py` deleted; tests updated to use `run_migrations()`.
- New `tests/test_migration_shape.py` lint fails CI if any migration omits
  `run_for_dialect` — keeps drift from re-emerging.
- CLAUDE.md updated with the new template and dialect-asymmetry guidance.

* ci: run client integration tests against Oracle on oracle-tests label

Adds test-python-client-oracle and test-typescript-client-oracle. These
mirror the existing test-python-client / test-typescript-client jobs but
spin up Oracle 23ai as a service container and point the API server at it
via HINDSIGHT_API_DATABASE_BACKEND=oracle + DATABASE_URL.

Why a new job instead of matrixing the existing one: Oracle Free's image
takes ~2min to start and is network-heavy, so we don't want to pay that
cost on every PR — only when oracle-tests is opted in via the PR label,
matching the existing test-api-oracle gate.

Why client tests, not unit tests: the unit suite already runs against
both backends via the abstraction layer. Only the client tests exercise
full HTTP round-trips with real serialized payloads, so they catch API
changes that work on PG but break on Oracle (or vice versa) in ways the
abstraction can't see.

* refactor(oracle): tighten feature requirements and dedup is_oracle_url

- Move is_oracle_url to db_url.py and import from there in env.py and
  migrations.py — was duplicated in both.
- Type-annotate _configure_pg_session / _configure_oracle_session params
  (Engine, Connection); ty checks pass.
- Update the Oracle baseline comment around vector + text index creation
  to make the hard requirement explicit: VECTOR + CTXSYS must be
  available, the migration fails hard if either is missing. The
  swallow-only-ORA-00955 behavior was already correct; the previous
  comment misleadingly called it "best-effort".

* chore(openclaw): apply pending prettier reformat to keep verify-generated-files green

Three formatting-only changes prettier wants to make. They've been stale
on main; CI's verify-generated-files runs lint with LINT_ALL=1 (vs the
"only changed integrations" local default), which surfaces them on every
unrelated PR. Folding them in here so this PR can land.

* fix(retain): plumb ops through handle_document_tracking

Line 312 of fact_storage.py references ``ops`` without ``handle_document_tracking``
declaring it as a parameter — straight NameError on every retain that walks
the upsert path. Bug landed on main in d8ec2d7f (#1325) when
``delete_stale_observations_for_memories`` started taking a backend-aware
``ops`` to choose between the PG array operator and the Oracle junction
table; the call site was added but the parameter wasn't threaded into the
enclosing function.

Fix: add ``ops=None`` to ``handle_document_tracking`` and pass ``pool.ops``
from each of the three call sites in orchestrator.py.

This is unrelated to the Alembic dialect-dispatcher refactor in this PR but
is what's blocking it — the NameError caused 17 retain tests to fail (and
left a pytest-xdist worker in a state that hung the whole job at 99%).

* test(observation): pass ops to handle_document_tracking in upsert test

The test calls fact_storage.handle_document_tracking directly, which
delegates to delete_stale_observations_for_memories(ops=ops). With ops=None
the helper falls back to the Oracle junction-table query and fails on PG
with "relation public.observation_sources does not exist". Real callers
(orchestrator, _delete_stale_observations_for_memories wrapper) all pass
self._backend.ops; the test just needs to do the same.

* ci: run client-against-oracle on every API change, drop label gate

Reserve the "oracle-tests" label for the heavy test-api-oracle (full unit
suite). The two client integration jobs against Oracle should run on every
API/client change just like their PG counterparts — the whole point is to
catch PG/Oracle drift before merge, which doesn't work if you have to
remember to label every PR. test-api-oracle keeps its label gate because
the full suite is too slow to run on every push.

* fix(oracle): rewrite path-style service to ?service_name= for SQLAlchemy

Oracle Free / Autonomous DB only register a service name with the listener,
but SQLAlchemy's oracle+oracledb dialect interprets the URL path as a SID.
That mismatch crashes alembic migrations on first connect:
  DPY-6003: SID "FREEPDB1" is not registered with the listener

Rewrite ``oracle://user:pass@host:port/SERVICE`` to
``oracle+oracledb://user:pass@host:port/?service_name=SERVICE`` so the
dialect uses the correct connect descriptor. ``?sid=`` and ``?service_name=``
already in the URL are passed through untouched.

Also adds scripts/dev/start-oracle.sh / stop-oracle.sh that spin up the same
Oracle 23ai Free image CI uses (``container-registry.oracle.com/database/free``)
and bootstrap the HINDSIGHT_TEST user, so we can repro this kind of issue
locally without round-tripping through GitHub Actions.

* fix(oracle): commit after migrations so alembic_version persists

On Oracle, alembic runs each migration with transactional_ddl=False
("Will assume non-transactional DDL"). Each CREATE TABLE auto-commits, but
the trailing ``UPDATE alembic_version SET version_num = ...`` is plain DML
that needs an explicit COMMIT. Without it the connection close rolls the
update back, leaving the schema fully created but the version row one
revision behind — so ``run_migrations`` reports success while the head row
sits at the previous revision.

Caught locally with the new scripts/dev/start-oracle.sh harness running the
same Oracle 23ai Free image CI uses; alembic_version was stuck at
``k6l7m8n9o0p1`` even though every table from the ``o1a2b3c4d5e6`` baseline
existed. After the fix it correctly advances to ``o1a2b3c4d5e6``, and a
second run is a no-op as expected.

PG already needs the same commit (Supabase RW-mode SET), so just drop the
``if not is_oracle`` guard.

* ci(oracle): run python client tests sequentially to avoid ORA-00060

The python client pyproject.toml defaults to -n auto (pytest-xdist).
Against Oracle that hits row-level deadlocks during retain cleanup —
ORA-00060 is logged repeatedly in the API server output and most tests
fail with "Internal Server Error" at fixture teardown. Same shape as the
existing test-api-oracle issue, which is already pinned to -n0.

Override to -n0 in the Oracle client job (only). The PG client job stays
parallel since pgvector + advisory locks handle concurrent retain fine.
TS client tests are unaffected — they run via vitest, not pytest.
2026-04-30 13:58:03 +02:00

124 lines
3.9 KiB
Bash
Executable File

#!/usr/bin/env bash
# Start a local Oracle 23ai Free container that mirrors what CI uses
# (`test-api-oracle`, `test-{python,typescript}-client-oracle`), bootstrap
# the HINDSIGHT_TEST user, and print the connection URL.
#
# Usage:
# ./scripts/dev/start-oracle.sh # start (idempotent) and print URL
# ./scripts/dev/start-oracle.sh --reset # drop and recreate the test user
#
# Stop with: ./scripts/dev/stop-oracle.sh
set -euo pipefail
CONTAINER_NAME="hindsight-oracle"
IMAGE="container-registry.oracle.com/database/free:latest"
PORT=1521
ORACLE_PWD="oracle"
TEST_USER="hindsight_test"
TEST_PASS="hindsight_test"
want_reset=0
for arg in "$@"; do
case "$arg" in
--reset) want_reset=1 ;;
*) echo "unknown arg: $arg" >&2; exit 2 ;;
esac
done
# 1) Ensure the container is running.
if [ -z "$(docker ps -q -f name="^${CONTAINER_NAME}$")" ]; then
if [ -n "$(docker ps -aq -f name="^${CONTAINER_NAME}$")" ]; then
echo "→ Removing stopped container ${CONTAINER_NAME}"
docker rm -f "${CONTAINER_NAME}" >/dev/null
fi
echo "→ Starting ${CONTAINER_NAME} (${IMAGE})"
docker run -d \
--name "${CONTAINER_NAME}" \
-p ${PORT}:1521 \
-e ORACLE_PWD="${ORACLE_PWD}" \
"${IMAGE}" >/dev/null
fi
# 2) Wait until SQL*Plus inside the container can SELECT 1 FROM DUAL.
# Same readiness probe CI uses for the service container's health-cmd.
echo "→ Waiting for FREEPDB1 to accept connections (~60-120s on a cold start) ..."
for i in $(seq 1 120); do
if docker exec "${CONTAINER_NAME}" \
bash -c "echo 'SELECT 1 FROM DUAL;' | sqlplus -s system/${ORACLE_PWD}@localhost:1521/FREEPDB1" \
>/dev/null 2>&1; then
echo " ready after ${i}s"
break
fi
if [ "$i" -eq 120 ]; then
echo "FREEPDB1 not ready after 120s — recent container logs:" >&2
docker logs --tail 60 "${CONTAINER_NAME}" >&2 || true
exit 1
fi
sleep 1
done
# 3) Bootstrap (or reset) the HINDSIGHT_TEST user. Mirrors the CI step.
if [ "$want_reset" = "1" ]; then
echo "→ --reset: dropping existing ${TEST_USER}"
docker exec -i "${CONTAINER_NAME}" \
sqlplus -s system/${ORACLE_PWD}@localhost:1521/FREEPDB1 <<SQL >/dev/null
DROP USER ${TEST_USER} CASCADE;
EXIT;
SQL
fi
echo "→ Ensuring ${TEST_USER} exists with ASSM tablespace (idempotent)"
# CREATE TABLESPACE / CREATE USER / GRANT statements are mirrored from the
# CI workflow steps (test-api-oracle, test-python-client-oracle, etc.).
# We tolerate "already exists" so the script can be re-run safely.
docker exec -i "${CONTAINER_NAME}" \
sqlplus -s system/${ORACLE_PWD}@localhost:1521/FREEPDB1 <<SQL >/dev/null
WHENEVER SQLERROR EXIT FAILURE
SET ECHO OFF
SET FEEDBACK OFF
DECLARE
e_already_exists EXCEPTION;
PRAGMA EXCEPTION_INIT(e_already_exists, -1543);
BEGIN
EXECUTE IMMEDIATE 'CREATE TABLESPACE hindsight_ts
DATAFILE ''hindsight_ts.dbf'' SIZE 200M AUTOEXTEND ON NEXT 50M
EXTENT MANAGEMENT LOCAL
SEGMENT SPACE MANAGEMENT AUTO';
EXCEPTION WHEN e_already_exists THEN NULL;
END;
/
DECLARE
e_user_exists EXCEPTION;
PRAGMA EXCEPTION_INIT(e_user_exists, -1920);
BEGIN
EXECUTE IMMEDIATE 'CREATE USER ${TEST_USER} IDENTIFIED BY ${TEST_PASS}
DEFAULT TABLESPACE hindsight_ts
TEMPORARY TABLESPACE temp
QUOTA UNLIMITED ON hindsight_ts';
EXCEPTION WHEN e_user_exists THEN NULL;
END;
/
GRANT CONNECT, RESOURCE, CREATE TABLE, CREATE SEQUENCE, CREATE VIEW, CREATE PROCEDURE TO ${TEST_USER};
GRANT CTXAPP TO ${TEST_USER};
EXIT;
SQL
URL="oracle+oracledb://${TEST_USER}:${TEST_PASS}@localhost:${PORT}/FREEPDB1"
cat <<EOF
✓ Oracle is ready.
Container : ${CONTAINER_NAME}
URL : ${URL}
export HINDSIGHT_API_DATABASE_BACKEND=oracle
export HINDSIGHT_API_DATABASE_URL='${URL}'
Migrations : uv run --directory hindsight-api-slim hindsight-admin run-db-migration
Stop : ./scripts/dev/stop-oracle.sh
EOF