Skip to content

DRAFT (phase 3, after T063): T072, 13.1 — the bundled PostgreSQL server, the local install's own child (plan 034) - #69

Draft
brettheap wants to merge 14 commits into
build/034-p3i-t070-install-modefrom
build/034-p3i-t072-bundled-postgres
Draft

brettheap wants to merge 14 commits into
build/034-p3i-t070-install-modefrom
build/034-p3i-t072-bundled-postgres

Conversation

@brettheap

@brettheap brettheap commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)

Arc: neutral-product-standalone-operability

Plan 034 (specs/034-opendox-standalone-operation/tasks.md, read at openxFactory main 91e4685f), phase-3 slice P3-I, install mode and the bundle:

Ruled:

  • R1Q22 (a), 5817152735;
  • R1Q16 (i)–(iv), 5850003126;
  • the phase-3 draft-ahead widening, 5901112350.

Claimed on openxFactory#656 in 5901575394. DRAFT, authored ahead: it does not go READY before T063 lands and the holder says so.

What it does, by R1Q16's four parts

  • (i) The document server starts it as its own child, and reports it.
    • opendox generate-and-open --local (or OPENDOX_INSTALL_MODE=local) starts postgres as a direct child of the process serving the document surface. It uses subprocess.Popen and never pg_ctl, which would re-parent it.
    • It prints the socket, the pid and what it migrated.
    • runtime status reports database_bundle (data_dir, socket_dir, pid). It reads the pid from the server's own postmaster.pid and believes it only while a postgres runs there.
  • (ii) Started and migrated, and nothing more.
    • initdb runs once per data directory.
    • An idempotent bootstrap makes the database and the SERVED role. Its grants are the compose stack's (init-runtime-role.sh): CONNECT, USAGE on public, and DML on what the owner creates, by default privileges.
    • Then migrations.MigrationRunner runs as the owner, with the served role and database declared. The run narrows the ledger to SELECT for the served role and verifies its access, exactly as a hosted runtime migrate does.
    • runtime migrate under local migrates the bundle too.
  • (iii) It ships as the opendox[local] extra, which is opendox[runtime] plus pixeltable-pgserver>=0.6.0 (RULED, openxFactory#656 5916000030 item 2). The test extra joins it, so F9.1's .[test] install still runs every case.
  • (iv) It stops with the entry point.
    • SIGTERM is read as the Ctrl-C the serve loop already stops on, followed by a PostgreSQL fast shutdown (then an immediate one, then SIGKILL, each bounded).
    • The backstop is PR_SET_PDEATHSIG on Linux, so a SIGKILLed entry point still takes its server with it.
    • The server runs in its own session, so a terminal's Ctrl-C reaches the entry point, and the stop happens in order.

13.1's fixed identity

  • The data and socket directories live under OPENDOX_STATE_DIR, at <state>/postgres/data and <state>/postgres/run.
    • The setting is new. It defaults to $XDG_STATE_HOME/opendox, else ~/.local/state/opendox, and must be absolute, because the server's process and a runtime status run from elsewhere must derive the same socket.
    • A state directory too long for the kernel's sun_path is refused, naming the setting.
  • No TCP listener: listen_addresses is empty, and the socket directory is narrowed to 0700.
  • Peer authentication (RULED, openxFactory#656 5916000030 item 3, "Peer auth + accept (Recommended)"):
    • initdb runs with --auth-local=peer --auth-host=reject.
    • Before every launch the bundle rewrites pg_hba.conf and pg_ident.conf (atomically, mode 0600). pg_hba.conf holds one rule, local all all peer map=opendox, plus host … reject for IPv4 and IPv6. pg_ident.conf maps the running OS user, and nobody else, to opendox and opendox_runtime.
    • The kernel reports the connecting uid, so the DSNs carry no password because there is none. A cluster an older build left as trust is put back to peer on its next start.
  • Both DSNs are supplied: two users (owner opendox for migrations, opendox_runtime for serving) over the one socket, with port spelled so a stray PGPORT cannot redirect libpq. They pass T071's three checks for the reason those exist: one dialect, one database, and never one credential in both settings.
    • An operator DSN given beside local is refused by name. It is added to T070's HOSTED_ONLY_SETTINGS, a holder reading on openxFactory#656 that Brett may overrule.
  • A second entry point on the same state directory is refused, naming the running pid. One install's database belongs to one entry point at a time.

The migrations gap (assigned to T072 by the holder)

  • The migrations were not package data. migrations/ sits at the repository root, and only the image copies it (WORKDIR /app), so pip install "opendox[local]" run outside a checkout had nothing to apply.
  • Now pyproject.toml's [tool.setuptools.data-files] maps migrations/*.sql into the wheel's data directory (share/opendox/migrations). The root migrations/ does not move.
  • config.migrations_dir resolves an unset OPENDOX_MIGRATIONS_DIR in this order:
    1. migrations wherever the working directory has one (a checkout, or the image's /app), which is today's default, unchanged;
    2. otherwise the copy the installed distribution's RECORD lists (packaged_migrations_dir);
    3. for an editable install, which installs no data files, the source tree's own migrations/.
  • The canonical digest gate is what proves any copy found is the pinned one.
  • test_a_wheel_install_migrates_its_bundled_server_outside_a_checkout:
    • builds this package's wheel offline (--no-build-isolation, --no-index);
    • installs it under a --prefix outside the checkout;
    • runs from a directory with no migrations/, asserting that opendox is the wheel's copy and that the migrations dir is under the prefix's share/opendox;
    • migrates the bundled server there (applied == ["0001", "0002"]).

The server package (RULED, openxFactory#656 5916000030 item 2: "pixeltable-pgserver (Recommended)")

  • pixeltable-pgserver 0.6.0, the maintained fork of pgserver, uploaded 2026-07-14. Apache-2.0, as its dist-info LICENSE and OSI classifier say. It carries PostgreSQL 16.14 under the PostgreSQL License (initdb --version and postgres --version from pixeltable_pgserver/pginstall/bin). It also carries an 18.4 under pginstall18/, which this package does not use.
  • Only its binaries are used, found with importlib.util.find_spec("pixeltable_pgserver") without importing it. Its own manager is not used, because:
    • it daemonizes through pg_ctl, against (i);
    • it stops from atexit, which SIGTERM never runs, against (iv);
    • it may put the socket under the user's runtime directory, opened to 0777, against 13.1.
  • Linkage, re-verified on the installed wheel (readelf -d, ldd):
    • postgres needs libz, libpthread, librt, libdl, libm, libc;
    • initdb needs those less libz and libdl, plus the wheel's own vendored libpq. That libpq resolves through RPATH $ORIGIN/../../../pixeltable_pgserver.libs and itself needs only libc, libm and libpthread;
    • the server's loadable modules need libc, and one needs the vendored libpq.
      So the system libraries are the C library and libz only: no system PostgreSQL, no ICU.
  • Its floor and its size:
    • Wheels exist for cp310 to cp314, on Linux x86_64 and aarch64, macOS and Windows.
    • The Linux wheels are tagged manylinux_2_27 and manylinux_2_28, so they need glibc 2.27 or later. That is the tag's floor. The highest GLIBC symbol any binary or module needs is 2.25, by objdump -T.
    • Each wheel is about 24.7 MB (the cp312 x86_64 wheel is 24,704,230 bytes), because it carries two server majors.
  • It pulls in fasteners, platformdirs, psutil and typing-extensions, which nothing here imports. All four were already pinned.
  • Superseded: pgserver 0.1.4 carried PostgreSQL 16.2 and had no wheel after cp312 (Copilot r4139811507 and r4139811528, both now resolved with this ruling cited). Also rejected: postgresql-binaries, which links the system's ICU and untars at first use, and pgembed, which is PostgreSQL 17.

Outside src/ and tests/

  • pyproject.toml: the local extra, the test extra joining it, setuptools>=70.1 in test (the wheel test's offline build; 70.1 is the first release that builds a wheel with no wheel package), and the data-files map. It is a single-writer file (T057 → T072). T057, openDox's own validator and its input set (7.1, 7.1a, 7.1b, 7.2) (plan 034) #58 (T057) adds package data there, and the merge-from-main round takes it.
  • constraints-cpython312-linux.txt, in its own commit as its header asks. It was extended under its own pins in a clean 3.12.3 venv (psutil==7.2.2, platformdirs==4.12.2, fasteners==0.20 and setuptools==84.0.0 new). At 84a6c041 it was re-resolved in a clean environment under the pins less pgserver. The one line that moved is pgserver==0.1.4 → pixeltable-pgserver==0.6.0.
  • deploy/ — one line, and the task requires it. deploy/compose/.env.example gains OPENDOX_STATE_DIR=, because test_every_runtime_setting_is_documented_in_env_example requires every SETTINGS entry there. The compose stack is hosted and never reads it. docs/: untouched.
  • Not touched: serve.py (T073 adds the install block) and validate.yml. It already installs .[runtime,test], and test now carries local.
  • Existing tests changed:
    • tests/test_doxbench_entrypoint.py stands the bundle in, with a tripwire. Its cases test the model port, which reads nothing from the store (R1Q16 (ii)).
    • T070's test_install_mode.py and test_install_mode_entrypoint.py stop passing DSNs beside local.
    • test_runtime_surface.py declares opendox.runtime.bundle stdlib-only at import, because opendox.cli imports it and opendox --help runs with no extra installed.

The falsifier

F13.1's runtime status block and its TCP-listener block, verbatim in their assertions (f13-1-local.sh), against a server generate-and-open --local started in the background, with no broker and no operator database, under set -euo pipefail. Three deviations are forced, and none weakens a check:

  • Generation is stood in. The start is tests_runtime/local_entrypoint_driver.py generate-and-open --local …, which is the real cli.main with only the three openXdox generation names and serve's two binding stand-ins replaced, exactly as tests/test_doxbench_entrypoint.py replaces them. This stack's base predates T055/T056's standalone generate (T055, serve and generate standalone (5.5, 4.3 part) (plan 034) #59 and its successor).
  • The pid comes from runtime status. The TCP-listener block reads the server's pid from runtime status's database_bundle, not from caps.json. /capabilities' install block is T073's, so F13.1's caps.json block is T073's to run.
  • The corpus is a one-file stand-in, because T050's fixture is not on this base.

BEFORE is #67's head b50e3b1; AFTER is this branch:

=== BEFORE (b50e3b1)
ready=1
runtime status rc=1
AssertionError: no bundled database answered: None OPENDOX_DATABASE_URL is required and is not set: …
FAIL status-block
AssertionError: the bundle reports no server pid: None
FAIL tcp-listener-block
=== AFTER (this branch, set -euo pipefail, exit 0)
ready=1
runtime status rc=0
PASS status-block
  server pid 638810, sockets ['3692911'], TCP LISTEN rows: none
PASS tcp-listener-block
PASS stops-with-entry-point (pid 638810 gone)
--- server stdout:
  database /tmp/tmp.v0wydKm5Zm/postgres/run (bundled, pid 638810, migrations applied now: ['0001', '0002'])

T070's F13.1 refusal probes and F13.1's load_settings block still pass on this branch (F13.1 REFUSALS + T070 PAIR: ALL PASSED).

In the suite, tests_runtime/test_bundled_postgres.py runs the same two blocks on the same background launch, and adds three checks: the server's PPid is the entry point's pid (i), the socket directory is 0700, and SIGTERM ends the entry point with exit 0 and the server gone (iv). Beside that it has a SIGKILL case (the parent-death backstop), the second-server refusal, runtime migrate under local, the wheel case above, and the layout and refusal cases. Under CI it fails rather than skips if the server is missing, because validate.yml pins EXPECT_SKIPPED=11 exactly.

A mutant of each new refusal and guarantee, killed

Each mutant was applied alone, and test_bundled_postgres.py plus test_install_mode.py were run with -x, with a 240 s bound so a hang could not pass for a kill:

mutant killed by
M1 an operator DSN accepted beside local test_migrate_under_the_local_mode_uses_the_bundle_and_refuses_a_dsn
M2 a socket path too long accepted test_a_state_dir_too_long_for_a_unix_socket_is_refused_naming_it
M3 a relative state dir accepted test_a_relative_state_dir_is_refused_naming_it
M4 the server opens a TCP listener (listen_addresses=127.0.0.1) test_the_entry_point_owns_a_migrated_server_with_no_tcp_listener
M5 the server is not the entry point's child (setsid -f) same
M6 stop() a no-op test_a_second_entry_point_on_the_same_state_dir_is_refused
M7 no parent-death signal test_the_server_stops_even_when_the_entry_point_is_killed_outright
M8 SIGTERM not read as an interrupt test_the_entry_point_owns_a_migrated_server_with_no_tcp_listener
M9 started but not migrated same
M10 the served DSN is the owner's (a collapse) test_the_two_dsns_are_two_users_over_the_one_socket
M11 no packaged migrations found test_a_wheel_install_migrates_its_bundled_server_outside_a_checkout
M12 the socket directory left 0755 test_the_entry_point_owns_a_migrated_server_with_no_tcp_listener
M13 a second server on one state dir not refused test_a_second_entry_point_on_the_same_state_dir_is_refused

A defect this PR's own test found in itself. The first cut of the wheel case ran pip install --prefix without --ignore-installed. pip then read the suite's own editable opendox as the installed copy of the same project and uninstalled it, emptying the environment the suite runs in (measured: pip list lost opendox and both console scripts). The flag is now there, with a comment, and the case asserts afterwards that the suite's own opendox still resolves.

The repo's own suite

Full python -m pytest -q, LANG=C.UTF-8, CI=true, against a postgres:16 like validate.yml's:

selected passed skipped failed errors
#60's head f097fd8 2485 2474 11 0 0
#67 (T070) b50e3b1 2531 2520 11 0 0
#67 (T070) 525f61c, its fix round 2 2538 2527 11 0 0
this branch before the merge, c3a70a2 2545 2534 11 0 0
32db5d8 (merges 525f61c) 2552 2541 11 0 0
ac61596 (merges 02dadc5) 2554 2543 11 0 0
28bdccd (fix round 4, and merges 859b37b6) 2580 2569 11 0 0
96b2699f (fix round 5) 2584 2573 11 0 0
a0fb7c8d (merges 026f00ea; fix round 6) 2584 2573 11 0 0
0f77d5c1 (fix round 7) 2595 2584 11 0 0
379fbb14 (fix round 8) 2601 2590 11 0 0
84a6c041 (the carrier and peer authentication, as ruled) 2613 2602 11 0 0
this branch, f8e6e9e9 (fix round 10) 2619 2608 11 0 0

EXPECT_SKIPPED=11 holds exactly, and the floors allow the rise unchanged. The new module adds about 32 s to the run (ten cases, each server start about 1.5 s).

Merging #67's fix rounds. 95fe16f merges 32683e8, and 32db5d8 merges 525f61c: runtime migrate and reset refuse what a local install cannot be.

  • 525f61c and this PR both rewrite load_migration_settings. The conflict resolves to this PR's structure: under local, the refusal is asked first, and only then is the bundle's migration DSN read. T070's reason is carried into the comment.
  • The two merged cases set the local shape as this PR defines it, with the mode and the state dir and no operator DSN. Beside local a DSN is itself refused here, so a case that set one would have tested the DSN refusal instead of the broker or bind refusal it names.
  • A mutant that drops the refusal from the migration loader fails all 7 merged cases.
  • ac61596 merges 02dadc5, cleanly. With the runtime extra absent, status's early return now reports a local install's broker as not configured. The merged case uses this PR's local shape and also asserts that database_bundle is reported on that return: present for local, null for hosted.

Fix rounds 4 and 5: Copilot's twelve threads (5e52872, 96b2699f)

Copilot reviewed 95fe16f, 32db5d8 and ac61596a and opened twelve threads. Ten are fixed, answered and resolved. The new cases are in tests_runtime/test_local_lifecycle.py (new, hermetic), plus two in test_bundled_postgres.py.

  • Migrations (r4139811473, r4139880241).
    • An explicit OPENDOX_MIGRATIONS_DIR is used as given.
    • Unset, a local install uses only its own installation's copy, never the working directory's. That copy is the source tree __file__ came from first, then the RECORD of the distribution that holds the running module. Where there is none, it is refused.
    • A hosted install's default is unchanged (13.6).
  • The pid (r4139811555, r4139938402).
    • A pid is believed only when /proc proves it is an executable named postgres running in this data directory. A proven-stale lock is removed before the launch.
    • Where there is no /proc, nothing is believed, and PostgreSQL's own interlocks stand. The price is a status with no pid on macOS and the BSDs, recorded in the thread.
  • initdb (r4139880213): it runs into an attempt directory that is renamed into place only on success. Abandoned attempts are removed. A non-cluster data/ is refused and left alone.
  • start() (r4139880279): every phase is one guarded operation, and every failure is the one named refusal, with its phase and class.
  • Interrupts (r4139880267): SIGTERM or Ctrl-C anywhere in the local lifecycle is a clean stop, exiting 128 + the signal number. A served run still exits 0.
  • Refusal wording (r4139880298): broker settings and DSNs are two classes, each with its own reason.
  • State dir (r4139938444): an unknown ~user, or no home, refuses naming OPENDOX_STATE_DIR.
  • Test helper (r4139811584): bounded by a selector. With a silent 8 s child and a 1 s deadline, it returned after 1.0 s where the old loop took 8.0 s.

Evidence:

  • Against ac61596's source, 17 of round 4's first 18 cases fail; the one that passes is the unchanged hosted default. Round 5's cases fail against 28bdccd.
  • Mutants: 23 of round 4 and 5 of round 5, all killed (runs/mutants-t072-r4.txt, -r5.txt in the writer's workdir).

Round 6 (a0fb7c8d): Copilot's review at 28bdccd9 opened two more threads.

  • The unknown-_serves point was already fixed at 96b2699f; it is answered and resolved.
  • The pyproject note named a function that no longer exists. It now names installation_migrations_dir, and the packaging case checks every opendox.runtime.config.<name> pyproject names.
  • 4aed6278 merges T070's 026f00ea, a docstring change.

Round 7 (0f77d5c1): Copilot's review at a0fb7c8d opened three threads, all fixed and resolved. SonarCloud raised one reliability finding.

  • libpq's environment. Every PG* variable is lifted out of os.environ for the duration and put back afterwards, around generate-and-open --local's lifecycle and the runtime verbs under local. PGHOSTADDR, PGSERVICE and PGOPTIONS can no longer redirect the bundle's connections. The real entry point and status are proven with all three set.
  • The socket's path. The resolved state tree must be this user's own and writable by no one else. Every ancestor must be owned by the user or root, and sticky where others can write it; a group-writable ancestor of the user's own group is allowed. Symlinks inside the tree are refused before any chmod.
  • A relative HOME is refused for the default state directory.
  • SonarCloud S6466 (reliability): server_binaries no longer indexes a list.
  • Evidence: 8 new cases fail against a0fb7c8d, and 11 mutants are killed.

Round 8 (379fbb14): Copilot's review at 0f77d5c1 opened two threads, both fixed and resolved.

  • A group-writable ancestor is refused whatever its group, because a primary group can have other members.
  • The configured path is checked as configured, as well as resolved: both chains' ancestors, every link's owner, and no .. anywhere.
  • Evidence: 5 new cases fail against 0f77d5c1, and 5 mutants are killed.

Round 9 (84a6c041) applies Brett's two rulings, openxFactory#656 5916000030 items 2 and 3: the carrier and peer authentication, as described above.

  • The server confirms it, in its own views:
    • pg_hba_file_rules has exactly the one peer rule (with map=opendox) and the two rejects;
    • pg_ident_file_mappings has exactly the two mappings, for this OS user;
    • system_user is peer:<os user> for both roles.
  • The map decides. The same OS user asking for a role outside the map is refused (peer authentication failed). A non-root suite cannot connect as a second OS user, so for that case the map, read back from the server, stands: it names no other user.
  • Evidence: 13 new cases fail against 379fbb14. 9 mutants are killed:
    • initdb trust;
    • no map;
    • a trust rule;
    • a wildcard system user;
    • a third role;
    • no re-assertion;
    • any name admitted;
    • files 0644;
    • the old carrier.
      The auth mutants are also killed by the real-server cases alone.

Round 10 (f8e6e9e9): Copilot's review at 84a6c041 raised three points, all fixed.

  • An existing postgres/data joins the tree check, a broken link included (r4147680113, resolved).
  • Missing parent directories are created exactly 0700 whatever the umask. mkdir(parents=True) under umask 0002 made them group-writable.
  • Readiness requires the data directory's lock file to name the launched child, so a racing loser cannot adopt the winner's socket.
  • Evidence: 6 new cases fail against 84a6c041, and 4 mutants are killed.

SonarCloud S2115, ACCEPTED, as ruled.

  • Issue: AaDvyyOCiqwq-gAw53M3, python:S2115, "Add password protection to this database", on src/opendox/runtime/config.py DatabaseBundle.dsn.
  • New status: accept (SonarCloud now reports it RESOLVED). Set with the SonarQube tool on openxFactory#656 5916000030 item 3's authority.
  • Rationale: the DSN has no password because the server authenticates Unix-socket connections by PEER. The kernel verifies the connecting uid (SO_PEERCRED), and pg_ident.conf maps only this install's OS user to the two roles. The socket directory is 0700, the server has no TCP listener (listen_addresses is empty), and every host connection is rejected. The same rationale is in the DSN's docstring.
  • Gate: after the change, SonarCloud reports the PR's quality gate OK on every condition.

Downstream, for the holder

🤖 Generated with Claude Code

Summary by Sourcery

Add a self-contained local installation mode that owns, migrates, reports, and cleans up its bundled PostgreSQL server.

New Features:

  • Add a standalone local-install mode with a bundled PostgreSQL server managed by the document entry point.
  • Expose bundled database location and process information through runtime status, including separate migration and serving credentials over a Unix socket.
  • Package migrations and the PostgreSQL carrier in local-install wheels so installations outside a checkout can initialize and migrate their database.

Bug Fixes:

  • Prevent local installs from using operator-supplied database or migration DSNs and from accidentally applying migrations from another working directory.
  • Ensure stale or conflicting bundled-server state is detected safely and refuse multiple entry points sharing one state directory.

Enhancements:

  • Enforce a state-directory layout, absolute-path and Unix-socket length constraints, local-only socket access, and no TCP listener.
  • Manage bundled PostgreSQL lifecycle with child-process ownership, graceful shutdown, parent-death cleanup, and bounded startup failure handling.
  • Resolve local and hosted migration sources independently while preserving the hosted migration default.

Build:

  • Add the local and test dependency extras, pin bundled-server dependencies, and include migrations in built wheels.

Deployment:

  • Document OPENDOX_STATE_DIR in the compose environment example.

Tests:

  • Add end-to-end coverage for bundled PostgreSQL startup, migration, process ownership, lifecycle cleanup, socket security, refusal behavior, and wheel installation outside a checkout.
  • Extend install-mode and runtime-surface tests for local settings, bundled database reporting, packaging, and lifecycle interrupt handling.

brettheap and others added 3 commits September 30, 2026 00:47
…hild (plan 034)

A LOCAL install (T070's `generate-and-open --local`, or
OPENDOX_INSTALL_MODE=local) now brings its own database (#1144 13.1, as
T007 batch H's addendum reads; RULED R1Q16 (i)-(iv), 5850003126).

- (i) `generate-and-open --local` starts a PostgreSQL server as its own
  direct child (subprocess.Popen, never pg_ctl) and reports it. The
  document server a user reaches is the process that owns it.
- (ii) The server is started AND migrated: initdb once, an idempotent
  bootstrap (the database, the served role, and the compose stack's grants
  narrowed to this install's owner), then migrations.MigrationRunner as the
  owner, with the served role and database declared.
- (iii) It ships as the `opendox[local]` extra: `opendox[runtime]` plus
  `pgserver>=0.1.4`, whose bundled binaries link only libc and libz. The
  `test` extra joins it, so F9.1's `.[test]` install still runs every case.
- (iv) It stops with the entry point. SIGTERM is read as the Ctrl-C the
  serve loop already stops on, followed by a fast shutdown.
  PR_SET_PDEATHSIG is the backstop when the entry point is SIGKILLed.
- Its data and socket directories live under OPENDOX_STATE_DIR, a new
  setting that defaults per user and must be absolute. The server listens
  on a 0700 Unix socket with listen_addresses empty: no TCP listener at all.
- Both DSNs are supplied: two users over the one socket, which pass T071's
  three checks. An operator DSN beside `local` is refused by name, joining
  T070's broker settings (a holder reading on openxFactory#656).
- `runtime status` reports database_bundle (data_dir, socket_dir, pid).
  `runtime migrate` under local migrates the bundle.

THE MIGRATIONS GAP (assigned to T072 by the holder). pyproject maps
migrations/*.sql into the wheel's data directory (share/opendox/migrations),
without moving the root migrations/ that the image copies. An unset
OPENDOX_MIGRATIONS_DIR is `migrations` wherever the working directory has
one (today's default, unchanged), and otherwise the copy the installed
distribution records. A test builds the wheel, installs it outside the
checkout, runs from a directory with no migrations/, and migrates the
bundled server.

Also:
- deploy/compose/.env.example gains OPENDOX_STATE_DIR=, because every
  SETTINGS entry is named there.
- tests/test_doxbench_entrypoint.py stands the bundle in, since those cases
  test the model port.
- T070's own tests stop passing DSNs beside `local`.

Arc: neutral-product-standalone-operability
Lane: openxfactory-4 (openXfactory-4-openDox_extraction)

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…xtra's setuptools

The lock is extended under its own pins (`-c` this file), in a clean
cpython 3.12.3 venv on linux x86_64, as its header asks. Five pins are new:

- pgserver 0.1.4, with its own psutil, platformdirs and fasteners;
- setuptools, for the wheel-install test's offline build.

No earlier pin moved.

Arc: neutral-product-standalone-operability
Lane: openxfactory-4 (openXfactory-4-openDox_extraction)

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Arc: neutral-product-standalone-operability
Lane: openxfactory-4 (openXfactory-4-openDox_extraction)

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings September 30, 2026 00:50
@sourcery-ai

sourcery-ai Bot commented Sep 30, 2026

Copy link
Copy Markdown

Reviewer's Guide

This PR makes local installation mode self-contained by packaging PostgreSQL binaries and migrations, deriving a private Unix-socket database under OPENDOX_STATE_DIR, managing the server as the document process's child through startup, migration, status reporting, and shutdown, and adding integration and wheel-install tests that falsify the process, security, packaging, and lifecycle guarantees.

Sequence diagram for the local bundled PostgreSQL lifecycle

sequenceDiagram
    participant User
    participant CLI as opendox CLI
    participant Bundle as BundledServer
    participant Postgres as postgres child
    participant Runner as MigrationRunner

    User->>CLI: generate-and-open --local
    CLI->>Bundle: start()
    Bundle->>Bundle: server_binaries()
    Bundle->>Bundle: _initdb()
    Bundle->>Postgres: subprocess.Popen()
    Bundle->>Postgres: wait for readiness
    Bundle->>Bundle: _bootstrap()
    Bundle->>Runner: apply()
    Runner-->>Bundle: applied migrations
    Bundle-->>CLI: report()
    CLI-->>User: serve document surface
    User->>CLI: SIGTERM or Ctrl-C
    CLI->>Bundle: stop()
    Bundle->>Postgres: SIGINT fast shutdown
    Bundle->>Postgres: SIGQUIT if needed
    Bundle->>Postgres: SIGKILL if needed
Loading

Sequence diagram for local runtime status reporting

sequenceDiagram
    participant Operator
    participant RuntimeCLI as runtime status
    participant Config as runtime.config
    participant PID as postmaster.pid

    Operator->>RuntimeCLI: runtime status
    RuntimeCLI->>Config: load_settings()
    Config-->>RuntimeCLI: state_dir and local settings
    RuntimeCLI->>Config: database_bundle(state_dir)
    RuntimeCLI->>PID: read pid
    PID-->>RuntimeCLI: postgres pid or none
    RuntimeCLI-->>Operator: database_bundle data_dir, socket_dir, pid
Loading

File-Level Changes

Change Details Files
Add a bundled PostgreSQL lifecycle for local installs, owned directly by the document-serving process.
  • Resolve a per-install state directory and fixed Unix-socket layout.
  • Start PostgreSQL with subprocess.Popen, no TCP listener, socket permissions, parent-death handling, and bounded shutdown escalation.
  • Bootstrap roles/database, run migrations, expose bundle metadata, and reject concurrent owners.
src/opendox/runtime/bundle.py
src/opendox/cli.py
src/opendox/runtime/cli.py
src/opendox/runtime/config.py
Make local mode self-contained in configuration and packaging.
  • Derive separate owner and served DSNs while rejecting operator DSNs beside local mode.
  • Add the local extra with pinned PostgreSQL packaging dependencies and include it through the test extra.
  • Package migrations as wheel data and resolve them from the checkout, installed distribution, or editable source tree.
pyproject.toml
constraints-cpython312-linux.txt
deploy/compose/.env.example
src/opendox/runtime/config.py
Expand tests and runtime falsification around local bundled-server guarantees.
  • Add background entry-point tests for migration, process ancestry, shutdown, socket permissions, no TCP listeners, SIGKILL cleanup, and duplicate-server refusal.
  • Add packaging/wheel installation coverage outside a checkout and update local-mode/configuration tests and import-surface contracts.
  • Use a narrowly scoped generation/serve stand-in until the stacked upstream changes land.
tests_runtime/test_bundled_postgres.py
tests_runtime/local_entrypoint_driver.py
tests/test_doxbench_entrypoint.py
tests/test_install_mode.py
tests/test_install_mode_entrypoint.py
tests_runtime/test_runtime_surface.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Migration-source trust, startup races, lifecycle guarantees, compatibility, and bundled database security require correction.

Review effort: Balanced
Findings: 1 High severity · 4 Medium severity

Open (5)
What changed in this PR

Adds lifecycle-managed PostgreSQL for local openDox installations, including packaged migrations and runtime reporting.

Changes:

  • Starts, migrates, reports, and stops a bundled PostgreSQL child process.
  • Adds local state-directory and packaged-migration resolution.
  • Adds dependencies, constraints, configuration, and integration coverage.
File Description
src/​opendox/​runtime/​bundle.py Implements bundled PostgreSQL lifecycle.
src/​opendox/​runtime/​config.py Adds bundle paths, DSNs, and migrations lookup.
src/​opendox/​runtime/​cli.py Reports bundle state.
src/​opendox/​cli.py Integrates bundle with local entrypoint.
pyproject.toml Adds local extra and migration data.
constraints-cpython312-linux.txt Pins new dependencies.
deploy/​compose/​.env.example Documents the state directory.
tests_runtime/​test_bundled_postgres.py Adds end-to-end bundle tests.
tests_runtime/​local_entrypoint_driver.py Provides an integration-test driver.
tests_runtime/​test_install_mode.py Updates local-mode expectations.
tests_runtime/​test_runtime_surface.py Verifies lightweight imports.
tests/​test_install_mode_entrypoint.py Updates entrypoint shape tests.
tests/​test_doxbench_entrypoint.py Adds a bundle stand-in.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/opendox/runtime/config.py
Comment thread pyproject.toml Outdated
Comment thread pyproject.toml Outdated
Comment thread src/opendox/runtime/bundle.py Outdated
Comment thread tests_runtime/test_bundled_postgres.py Outdated
T070's 525f61c makes `load_migration_settings` refuse what a local install
cannot be (Copilot review of openDox-code#67). T072 had already restructured
the same lines: under `local`, it asks that refusal and then takes the
bundle's migration DSN. The conflict resolves to T072's structure, with
T070's reason carried into its comment. The refusal is asked once, before
the bundle's DSN is read.

The two merged cases now set the local shape as T072 defines it, with the
mode and the state dir and no operator DSN. Beside `local` a DSN is itself
refused (T072), so a merged case that set one would have tested the DSN
refusal rather than the broker or bind refusal it names.

Full suite: 2552 selected, 2541 passed, 11 skipped, 0 failed. A mutant that
drops the refusal from the migration loader fails all 7 merged cases.

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)
Arc: neutral-product-standalone-operability

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings September 30, 2026 01:06

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread src/opendox/runtime/bundle.py Outdated
Comment thread src/opendox/runtime/config.py
Comment thread src/opendox/cli.py
Comment thread src/opendox/runtime/bundle.py Outdated
Comment thread src/opendox/runtime/config.py
T070's 02dadc5 makes `runtime status`, when the runtime extra is absent,
report a local install's broker as not configured on the early return too
(Copilot review of openDox-code#67). It merges cleanly: T072's
`database_bundle` report comes before that return, in another hunk.

The merged case sets the local shape as T072 defines it, with the mode and
the state dir and no operator DSN. It also asserts that the bundle is
reported on the early return (`database_bundle` present for local, `null`
for hosted).

Full suite: 2554 selected, 2543 passed, 11 skipped, 0 failed.

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)
Arc: neutral-product-standalone-operability

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings September 30, 2026 01:16

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread src/opendox/runtime/bundle.py Outdated
Comment thread src/opendox/runtime/config.py
brettheap and others added 2 commits September 30, 2026 15:11
…errupts hardened (Copilot review)

Copilot's reviews at 95fe16f and 32db5d8 opened ten threads. Eight are fixed
here. The two about the server package (PostgreSQL 16.2, no wheel for 3.13)
wait on the holder.

- Migrations (r4139811473, r4139880241). An explicit OPENDOX_MIGRATIONS_DIR
  is used as given. Unset, a LOCAL install uses only the copy its own
  installation carries, and never the working directory's: its entry point
  runs every migration as the bundle's owner, and the canonical gate pins
  0001 alone. Where the installation carries none, it is refused, naming the
  setting. The installation's copy is the source tree the module was
  imported from (src/ beside a pyproject.toml naming opendox), then the
  RECORD of the distribution that holds the running module, and never
  another one found by name. A HOSTED install's unset default is unchanged
  (13.6).
- The pid (r4139811555). A postmaster.pid is believed only for this data
  directory's postmaster, as the kernel reports it: an executable named
  postgres whose working directory is the data directory. Another user's
  process is never believed. A lock that /proc proves stale is removed
  before the launch, so a recycled pid no longer holds the bundle.
- initdb (r4139880213). It runs into an attempt directory beside the data
  directory, which is renamed into place only on success. An attempt whose
  process is gone is removed. A non-empty data directory that holds no
  cluster is refused and left untouched.
- start() (r4139880279). Directories, initialize, launch, wait, bootstrap
  and migrate are one guarded operation, and every failure is the one named
  refusal (phase and class name), with anything started stopped.
- Interrupts (r4139880267). SIGTERM or Ctrl-C anywhere in the local
  lifecycle is a clean stop: no traceback, the bundle stopped, the handler
  restored first. Nothing was served, so the exit is 128 + the signal number.
  A served run ended by SIGTERM still exits 0.
- Refusal wording (r4139880298). Broker settings and operator DSNs are two
  classes, and each is refused with its own reason.
- The test helper (r4139811584). The launch helper is bounded by its
  deadline, through a selector. Measured with a silent 8 s child and a 1 s
  deadline: the old loop returned after 8.0 s, the new one after 1.0 s.

tests_runtime/test_local_lifecycle.py (new, hermetic) holds these cases,
plus a real-server stale-lock case and the helper's own case in
test_bundled_postgres.py. Against ac61596's source, 17 of the module's
first 18 cases fail. The one that passes is the hosted default, which is
unchanged on purpose. All 23 mutants of the fixes are killed.

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)
Arc: neutral-product-standalone-operability

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
T070's 859b37b adds a DB-backed case proving that a healthy local
`status` exits 0, and scopes RuntimeSettings' broker invariants to their
loader (Copilot review of openDox-code#67). It merges cleanly.

Here the case uses the local install's own database. Beside `local` an
operator's DSN is refused (T072), so the case starts the bundled server
on a fresh state directory and asks `status` about it. With the local
return mutated to `ok=False`, it fails and the other 42 cases in the
module pass.

Full suite, with this PR's fourth fix round (5e52872): 2580 selected,
2569 passed, 11 skipped, 0 failed.

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)
Arc: neutral-product-standalone-operability

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings September 30, 2026 15:18

Copilot AI left a comment

Copy link
Copy Markdown

Comment thread src/opendox/runtime/bundle.py Outdated
Comment thread pyproject.toml Outdated
…es by name (Copilot review)

Copilot's review at ac61596 opened two more threads, both real.

- r4139938402: the old fallback believed any pid it could not inspect. So a
  process that exited between the signal check and the /proc read, or any
  pid on a platform without /proc, counted as the server. Round 4 already
  treated a vanished process as gone on the /proc path. This round makes
  the rule total. `running_pid` believes a pid only when the kernel proves
  it is this data directory's postmaster. Where nothing can be asked (no
  /proc: macOS, the BSDs), it believes nothing, and this module does not
  refuse a start over it. PostgreSQL's own interlocks, the lock file's
  live-pid check and the shared-memory check, still refuse a second
  postmaster, so this never yields two servers, and never a refusal over a
  process that is not one. A lock that cannot be proven stale is left for
  PostgreSQL to judge. The price on such a platform is a `status` with no
  pid. That is recorded, not hidden: the standard library has no portable
  way to ask, and a third-party module here would be an undeclared runtime
  dependency (test_consumer_reach). Measured before: round 4's source with
  no /proc, and a python decoy in the data dir, reported the decoy's pid.
  ac61596's source reported a pid that had already exited.
- r4139938444: `Path.expanduser()` raises RuntimeError for an unknown
  `~user`, and `Path.home()` does the same where there is no home. Both now
  refuse by name, as ConfigurationError naming OPENDOX_STATE_DIR. A hosted
  install still never reads the setting and is not refused over it (13.6).

Five new cases fail against 28bdccd's source and pass here. Five mutants of
the fixes are killed. Full suite: 2584 selected, 2573 passed, 11 skipped,
0 failed.

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)
Arc: neutral-product-standalone-operability

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings September 30, 2026 15:48
brettheap and others added 2 commits September 30, 2026 15:52
T070's 026f00e corrects the install-mode module's docstring. It now names
the one DB-backed case instead of calling every case hermetic (Copilot
review of openDox-code#67). Here that case starts the local install's own
bundled server, because beside `local` an operator's DSN is refused, so the
merged sentence says so. Docstring only: the module runs 43 passed.

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)
Arc: neutral-product-standalone-operability

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…(Copilot review)

The data-files note pointed readers at
`opendox.runtime.config.packaged_migrations_dir`, which fix round 4 replaced
with `installation_migrations_dir`, the source tree first and then the RECORD
of the distribution that holds the running module. The note now names that
function and says what it asks.

The packaging case now also checks that every
`opendox.runtime.config.<name>` pyproject.toml names exists, so a stale
pointer cannot come back. Against 4aed627's pyproject.toml it fails, naming
`packaged_migrations_dir`. Here it passes. Full suite: 2584 selected, 2573
passed, 11 skipped, 0 failed.

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)
Arc: neutral-product-standalone-operability

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Comment thread src/opendox/runtime/config.py
Copilot AI balanced review requested due to automatic review settings September 30, 2026 16:01

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Inherited libpq settings can redirect local connections, and unsafe state-directory parents undermine the socket trust boundary.

Review effort: Balanced
Findings: 2 High severity · 3 Medium severity

Open (5)

Comment thread src/opendox/runtime/bundle.py Outdated
Comment thread src/opendox/runtime/config.py
… (Copilot and SonarCloud review)

Copilot's review at a0fb7c8 opened three threads, and SonarCloud raised a
reliability finding. All four are fixed here.

- PG* defaults (r4146787926). libpq fills every parameter a DSN leaves
  unset from the environment. PGHOSTADDR outranks the socket `host` and
  sends the connection to TCP, PGSERVICE fills parameters from a service
  file, and PGOPTIONS sets the session's parameters. No DSN can name every
  parameter, and an explicitly empty `service` is itself an error. So
  `bundle.isolated_from_libpq_environment` lifts every PG* variable out of
  os.environ for the duration and puts it back afterwards. It wraps
  `generate-and-open --local`'s whole lifecycle and the runtime CLI's verbs
  under `local`. A hosted install's libpq is untouched (13.6). With
  PGHOSTADDR=192.0.2.1, PGSERVICE=no-such-service and PGOPTIONS=-c
  search_path=nowhere set, the real entry point still starts, migrates and
  serves its own server, and `runtime status` still finds it.
- The socket's path (r4146787852). Before the socket directory is chmodded
  (a chmod follows a symlink), the resolved path is checked. The state dir,
  postgres/ and run/ must be real directories owned by this user and
  writable by no one else. Every ancestor must be owned by this user or by
  root, and must be sticky if every user can write it, or if a group other
  than this user's own can write it. Anything else is refused by name.
- A relative HOME (r4146659876). It is refused for the default state
  directory, which would otherwise depend on the working directory.
- SonarCloud S6466. server_binaries no longer indexes a list. It takes the
  first search location or none, and both refusal shapes have a case.

Against a0fb7c8's source, 8 of the new cases fail and the positive control
passes. 11 mutants are killed. Full suite: 2595 selected, 2584 passed,
11 skipped, 0 failed.

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)
Arc: neutral-product-standalone-operability

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings September 30, 2026 16:33

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Filesystem trust checks and interrupted initialization can allow socket redirection or leave child processes running.

Review effort: Balanced
Findings: 2 High severity · 2 Medium severity

Open (4)
Resolved since last review (3)
Previously missed (2)

In code that hasn't changed since last review

Medium severity Remove stale locks when an inaccessible PID is not the postmaster

src/​opendox/​runtime/​bundle.py:251

A PermissionError here means the PID is live but belongs to a process this user cannot signal, so by this module's same-user ownership invariant it cannot be this bundle's postmaster. Leaving that stale lock delegates to PostgreSQL, which treats a live PID as an active postmaster and can refuse every restart until the unrelated process exits. Distinguish this case from an unverifiable platform and remove the lock only after _serves() confirms a mismatch.

Medium severity Terminate and reap initdb on signal-driven interruption

src/​opendox/​runtime/​bundle.py:553

subprocess.run() does not terminate initdb when the SIGTERM handler raises KeyboardInterrupt; its Popen cleanup waits briefly and re-raises, while self.process is still None. The entry point can therefore return 143 with initdb still writing to an attempt named with the now-dead parent PID, and a restart may delete that live attempt. Manage initdb with an explicit child handle and terminate/kill/wait it on BaseException before removing the attempt.

Comment thread src/opendox/runtime/bundle.py Outdated
Comment thread src/opendox/runtime/bundle.py Outdated
…uch for (Copilot review)

Copilot's review at 0f77d5c opened two threads. Both were real.

- r4147004990: a user's primary group can have other members, so a 0775
  ancestor is not private. Every ancestor that anyone else can write, a
  group included, must now be sticky. The round-7 allowance for the user's
  own group is gone, and its positive control is now a refusal case. The
  sticky shape (/tmp) is still the control.
- r4147005063: resolving the configured path before checking it discarded
  the path that was actually configured. A link on that path could be
  repointed afterwards, while the bundle kept using the unresolved paths.
  Now:
  - the ancestors of BOTH the configured path and the resolved one are
    checked;
  - every symbolic link on the configured path must be owned by this user
    or by root;
  - `..` is refused in OPENDOX_STATE_DIR and XDG_STATE_HOME (and in a
    derived HOME), so the configured components are the ones the kernel
    walks.
  A user's own link to a private directory is still accepted.

Against 0f77d5c's source, 5 of the new cases fail and the 4 controls pass.
5 mutants are killed. Full suite: 2601 selected, 2590 passed, 11 skipped,
0 failed.

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)
Arc: neutral-product-standalone-operability

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings September 30, 2026 16:48

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The unresolved PostgreSQL 16.2 security age and unsupported Python 3.13+ carrier compatibility remain release blockers.

Review effort: Balanced
Findings: 2 Medium severity

Open (2)
Resolved since last review (2)

… peer (RULED 5916000030 items 2, 3)

Brett's rulings on openxFactory#656 (comment 5916000030) cover two things.

Item 2, "pixeltable-pgserver (Recommended)". The `local` extra's carrier is
now pixeltable-pgserver>=0.6.0, the maintained fork of pgserver. Only its
binaries are used, found under pixeltable_pgserver/pginstall/bin. Measured
on the installed 0.6.0 wheel:
- initdb and postgres report PostgreSQL 16.14;
- postgres links libz, libpthread, librt, libdl, libm and libc only, and
  initdb links the wheel's own vendored libpq through $ORIGIN;
- the highest GLIBC symbol any binary or server module needs is 2.25, and
  the wheels are tagged manylinux_2_27/2_28;
- the licence is Apache-2.0 (dist-info LICENSE and classifier);
- the cp312 x86_64 wheel is 24,704,230 bytes;
- wheels exist for cp310 to cp314.
The lock was re-resolved in a clean environment under the existing pins
less pgserver. The only line that moved is pgserver==0.1.4 ->
pixeltable-pgserver==0.6.0.

Item 3, "Peer auth + accept (Recommended)".
- initdb now runs with --auth-local=peer --auth-host=reject.
- Before every launch the bundle writes pg_hba.conf and pg_ident.conf
  atomically, mode 0600. pg_hba.conf holds one local rule, peer map=opendox,
  and host reject for IPv4 and IPv6. pg_ident.conf maps the running OS user
  (from the password database), and nobody else, to opendox and
  opendox_runtime.
- listen_addresses stays empty.
- An OS user name the map cannot hold plainly is refused, as is a uid with
  no password entry.
- A cluster that an older build left as trust is put back to peer on its
  next start.

The server's own reading proves it. pg_hba_file_rules has exactly those
three rules and pg_ident_file_mappings exactly those two mappings, and
system_user is peer:<os user> for both roles. The same OS user asking for a
role outside the map is refused ("peer authentication failed").

Against 379fbb1's source and packaging, 13 of the new cases fail. Nine
mutants of the carrier and the authentication are killed. The auth mutants
are also killed by the real-server cases alone. Full suite: 2613 selected,
2602 passed, 11 skipped, 0 failed.

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)
Arc: neutral-product-standalone-operability

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings September 30, 2026 17:47

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Startup can fail under common umasks, omit data-directory symlink validation, and accept another concurrently launched server as its own.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Resolved since last review (2)
Previously missed (2)

In code that hasn't changed since last review

Medium severity Create parent directories explicitly with mode 0700

src/​opendox/​runtime/​bundle.py:534

parents=True does not apply mode=0o700 to missing parents: pathlib creates those with its default 0777 masked by the process umask. With a common umask 0002, a fresh default such as ~/.local/state/opendox can therefore create .local/state as 0775, after which _refuse_an_unsafe_tree() rejects the directories this method just created. Create each missing component explicitly with mode 0700 so first startup works independently of umask.

Medium severity Tie readiness checks to the launched PostgreSQL process

src/​opendox/​runtime/​bundle.py:710

Readiness is not tied to the process just launched. If two entry points race from an idle state (or process identity cannot be verified without /proc), the losing postgres can still be alive briefly while this connection succeeds against the winner's already-listening socket; this instance then completes bootstrap/migration and serves while owning no database child. Before and after connecting, require postmaster.pid to name self.process.pid; otherwise keep waiting for this child to exit/refuse.

Comment thread src/opendox/runtime/bundle.py
…his install (Copilot review)

Copilot's review at 84a6c04 made three points, all real.

- r4147680113: the tree check left out postgres/data. An existing data
  directory, a broken link included, now joins the own-tree check: a real
  directory, owned by this user, writable by no one else, not a link. A link
  to a cluster elsewhere would otherwise have been given this install's
  authentication files and launched outside the state tree. A fresh data
  directory needs no check, because _initialize renames it into place.
- Fresh directories and the umask (overview, previously missed).
  mkdir(parents=True) creates intermediate directories with the default mode
  less the umask. Under umask 0002, a fresh ~/.local/state/opendox would
  create group-writable parents, which the tree check then refused. Each
  missing component is now created on its own and set to exactly 0700,
  whatever the umask.
- Readiness (overview, previously missed). A successful connection proves
  only that some server answered. Two entry points racing from an idle
  state both launch, and the loser's postgres lives a moment while the
  winner's socket answers. So readiness now also needs the data directory's
  lock file to name this child. Otherwise the wait goes on until this child
  exits and is refused. One check after the connection is enough, since the
  lock admits one postmaster per data directory and the socket directory
  belongs to exactly one data directory. A before-check was tried and
  dropped: no mutant distinguishes it.

Against 84a6c04's bundle.py, all 6 new cases fail. 4 mutants are killed.
Full suite: 2619 selected, 2608 passed, 11 skipped, 0 failed.

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)
Arc: neutral-product-standalone-operability

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings September 30, 2026 18:08
@sonarqubecloud

Copy link
Copy Markdown

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Authentication overrides, unsafe pre-validation filesystem mutations, stale locks, and restrictive-umask failures remain unresolved.

Review effort: Balanced
Findings: None

Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Handle EPERM stale locks with process identity checks

src/​opendox/​runtime/​bundle.py:265

PermissionError means this PID is not signalable, but returning here skips the existing identity check. If a stale lock's PID has been recycled by another user, PostgreSQL treats kill(pid, 0) == EPERM as a live process and the local install remains blocked until that unrelated process exits. Continue to _serves; remove the lock only when the available process inspection says it is not this data directory's server.

This issue also appears in the following locations of the same file:

  • line 438
  • line 558
  • line 659
  • line 699
  • line 703

@brettheap

Copy link
Copy Markdown
Contributor Author

On the overview's "previously missed" note at f8e6e9e9, "Handle EPERM stale locks with process identity checks" (bundle.py _remove_a_proven_stale_lock): the early return on PermissionError is deliberate. The note's premise about PostgreSQL does not hold for 16.

The note says PostgreSQL treats kill(pid, 0) == EPERM as a live process. PostgreSQL 16 does the opposite. src/backend/utils/init/miscinit.c (REL_16_STABLE), CreateLockFile, reads:

if (kill(other_pid, 0) == 0 ||
    (errno != ESRCH && errno != EPERM))
{
    /* lockfile belongs to a live process */
    ereport(FATAL, ... "lock file \"%s\" already exists" ...

EPERM is read as stale, and the comment above that code says why: a postmaster cannot attach to a data directory owned by another uid. So a lock whose pid was recycled to another user's process is removed by PostgreSQL itself at the next start, and nothing here needs to prove anything.

That is why _remove_a_proven_stale_lock returns on both ProcessLookupError and PermissionError ("PostgreSQL's own rule covers both"). It removes a lock itself only in the one case PostgreSQL refuses: a same-user live process that /proc proves is not this data directory's postmaster.

running_pid also returns None for EPERM, so status never reports another user's process as this server. That behaviour is pinned by test_another_users_process_is_not_this_server.

No code change.

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)

brettheap added a commit that referenced this pull request Sep 30, 2026
…ult_registry.resolve_source) (#70)

## What this is

A small follow-up to T055 (#59, landed as `fa140875`). It takes the one finding Copilot's review of #59 at `0c946f4e` raised as "previously missed", after the last push that could take it.

### The finding, quoted

Copilot review overview on #59 at `0c946f4e` (review `5368674478`), medium, "Avoid unstable second lookup during source path confinement", `src/opendox/default_registry.py:444`:

> `resolve_source()` performs a second lookup by `(repository, ref)` after the caller has already resolved an entry. A concurrent refresh can replace that key between the two operations, so the returned path can be confined under a different entry's `source_root` than the entry whose metadata/listed paths the caller is using (for example, `serve_project._resolved_listed_edit_entry`). Expose an entry-based confinement operation or make the caller pass the resolved entry so lookup, validation, and path confinement use one stable entry.

It is real. #59 already closed the same pattern in `serve.py`'s `/source` arm (Copilot r4136585695 and r4136863569): that arm resolves the entry once and confines to that entry's own root with `resolve_source_path(Path(root), rest)`. `serve_project._resolved_listed_edit_entry` was the one caller left.

## What changed

- `src/opendox/serve_project.py`: `_resolved_listed_edit_entry` resolves the entry ONCE and confines the path to THAT entry's own root through the registry seam's declared `resolve_within` (read as `projection_seams.registry.current()` inside the function). The listed-path check already read the entry in hand, and the editor is launched over `entry.source_root`, so the lookup, the validation and the confinement are now one entry. An entry with no root serves nothing, as before.
- The caller needs no method the seam does not declare. `resolve_source` is on no seam's list (`REGISTRY_CALLABLES`), so a contributed registry is never asked for one. That is why the fix confines the entry in hand rather than adding an entry-based method to openDox's own registry: a host's registry would not carry it.
- `src/opendox/default_registry.py`: `SnapshotRegistry.resolve_source` keeps its behaviour (one lookup, confined to that entry). Its docstring now says it is for a caller that holds only a pair, and why a caller that already holds an entry must not ask again by its pair.
- `tests/test_edit_action_one_entry.py` (new, 8 cases).

No module-level proxy is bound in `serve_project.py`: `tests/test_projection_seams.py::test_no_proxy_over_a_seam_is_read_at_import_time` pins the exact set of modules that bind one (`serve.py`, `serve_workbench.py`), and this PR does not edit that file.

## Every caller of the two-step path

`git grep resolve_source -- src` at `fa140875` finds the definition and exactly one production caller, `serve_project.py:111`. The other registry lookups in `src/` are one resolution each: `serve.py` `_serve_snapshot` and `_serve_source` (already one entry), `serve_workbench.py` (`resolve` then `resolve_within(entry.source_root, ...)`, three sites), and `branch_session.py` (stamping an entry after a register, no confinement). `_keyed_source`'s `registry.get(*parsed)` is a parse-time existence check whose result is a key, and `_serve_source` then resolves that key once.

The new test `test_no_module_asks_a_registry_for_a_path_by_a_pair` holds that set empty, so a future caller has to be argued for.

## Evidence

**Red at main, green after.** The new module against `fa140875`'s sources (`serve_project.py` and `default_registry.py` as on main):

```
FAILED test_the_edit_arm_asks_the_registry_once_and_never_for_a_path_by_a_pair
FAILED test_no_module_asks_a_registry_for_a_path_by_a_pair
FAILED test_a_host_registry_needs_only_what_the_seam_declares
FAILED test_a_refresh_that_replaces_the_key_cannot_take_the_file_the_route_refuses
FAILED test_a_refresh_that_replaces_the_key_cannot_take_the_file_the_route_accepts
5 failed, 3 passed
```

With this PR: `8 passed`. The three that pass at main are the control (a listed file opens with no refresh), confinement kept, and "no root serves nothing", which pin what must NOT change.

The race is tested at the ROUTE, both ways, with a real server, a real `POST /actions/edit` and the console token. A registry whose key is replaced right after the route's first `resolve` (as a refresh on another thread would) lands the replacement between the resolution and the confinement (asserted):

- Entry's root has NO file, the replacement's root has it. At main the route took the file from the replacement's root, read the first entry's listing, and started the editor over the first entry's root: `200` and an editor over a file that is not there. Now: `404 document_unavailable`, no editor.
- Entry's root has the file, the replacement's root has none. At main the route refused a file its own entry holds (`404`). Now: `200`, and the editor is started over that entry's root.

**Mutants of the fix, all killed** (the new module only, `serve_project.py` restored after each):

| mutant | killed by |
|---|---|
| M1 the second lookup again (`registry.resolve_source(repository, ref, path)`, i.e. main) | one-lookup count, the no-module-asks scan, the host registry, both race cases |
| M2 confine by `Path(root) / path`, no containment rule | `test_the_entrys_own_root_still_confines_what_the_route_accepts` |
| M3 the no-root check dropped | host-registry and no-root cases |
| M4 the listed-path check dropped | the confinement case (an unlisted file inside the root) |
| M5 the listing read through a second lookup | the one-lookup count (the race cases cannot see it, as both entries share a snapshot) |

**T056's module against this fix.** Fetched #66's head `38761c76` read-only into a scratch worktree, merged main (`fa140875`; the two add/add conflicts, `default_registry.py` and `tests/test_projection_seams.py`, resolved to main's blobs, as T056's own diff does not touch either), cherry-picked this commit on top, and ran `tests/test_standalone_generate_path.py` (its children run from that worktree's `src` via `PYTHONPATH`):

```
tests/test_standalone_generate_path.py + tests/test_edit_action_one_entry.py: 14 passed
control, this commit reverted: tests/test_standalone_generate_path.py: 6 passed
```

That module's requests are `GET /source/notes-toolshed-inventory.md` (200, byte-equal) and `GET /source/.git/config` (404). They go through `serve.py`'s `/source` arm (`resolve_source_path`), which #59 already moved off `resolve_source`, not through `serve_project`, so this PR leaves them as they were. The module passes identically with and without this commit.

**Whole suite** at `5666505b`, `LANG=C.UTF-8`, run in the foreground with a throwaway `postgres:16` and `OPENDOX_TEST_DATABASE_URL` set as CI sets it:

```
python -m pytest -q tests          2379 passed, 11 skipped
python -m pytest -q tests_runtime   607 passed
total                              2986 passed, 11 skipped, 0 failed
```

#59's CI at `0c946f4e` was `selected=2989 passed=2978 skipped=11`. This PR adds 8 cases: 2997 selected, 2986 passed, 11 skipped.

## Overlap with open PRs

None. `gh pr diff --name-only` on #60 to #69, read against each PR's own merge-base (#66 and #68 carry #59's commits, which lists `default_registry.py` spuriously): their own diffs touch neither `serve_project.py` nor `default_registry.py`. #66's own `serve.py` change is one flushed `print` near line 2216, outside the `/source` arm.

## Review rounds

- **Copilot at `0471f8c7`**: "Approval recommended", no findings, 0 threads. The SonarCloud quality gate failed there on 4.9% duplication on new code (required at most 3%): the new test module carried a copy of `test_projection_seams.py`'s autouse isolation fixture and `git` helper.
- **`5666505b`**: imports both instead of copying them (the `tests/test_doxbench_*.py` precedent), a test-only change. The autouse fixture still applies to all eight cases (eight SETUPs under `--setup-show`), the eight cases pass, and M1 to M5 are still killed.
- **Copilot at `5666505b`**: "Approval recommended", no findings, 0 threads. `validate` and SonarCloud ("Quality Gate passed") green at `5666505b`.

Arc: neutral-product-standalone-operability
Lane: openxfactory-4 (openXfactory-4-openDox_extraction)

🤖 Generated with [Claude Code](https://claude.com/claude-code)


Arc: neutral-product-standalone-operability
Lane: openxfactory-4 (openXfactory-4-openDox_extraction)
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants