Skip to content

DRAFT (phase 3, after T063): T070, 13.4-13.6 — OPENDOX_INSTALL_MODE and generate-and-open --local (plan 034) - #67

Draft
brettheap wants to merge 6 commits into
build/034-p3i-t071-load-settings-dialect-and-collapsefrom
build/034-p3i-t070-install-mode
Draft

brettheap wants to merge 6 commits into
build/034-p3i-t071-load-settings-dialect-and-collapsefrom
build/034-p3i-t070-install-mode

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, after T007 batch H landed as openxFactory#1206 → f99a2097), phase-3 slice P3-I, install mode and the bundle:

Ruled:

  • R1Q22 (a), 5817152735;
  • R1Q15 (b), 5850003126 (as batch H's 13.4 addendum reads);
  • the phase-3 draft-ahead widening, 5901112350 ("Install chain + lens (Recommended)").

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

  • The selector (13.4): OPENDOX_INSTALL_MODE, values local and hosted, defaulting to hosted. It is a SETTINGS entry read in runtime/config.py, placed immediately beside OPENDOX_OIDC_ISSUER. A blank value reads as unset, which means hosted: "It is UNSET, not local, that must be safe."
  • The flag (R1Q15 (b)): generate-and-open --local selects local exactly as OPENDOX_INSTALL_MODE=local does. With neither, the install is hosted (13.5). The flag belongs to the verb and follows it (10.1).
  • A flag and a setting that disagree are refused, naming both. Example: --local beside OPENDOX_INSTALL_MODE=hosted. No explicit selection is silently overridden. This is plan 034's fail-closed reading (Principle VII), not #1144's text. No answer rules the pair, batch H does not write it into #1144, and evidence/analyze-round-2.md (U2-1, V2-6) records it for Brett.
  • Local needs no broker. RuntimeSettings.oidc_issuer and oidc_audience are empty, and oidc_jwks_url is None. jwks_url() and discovery_url() return "" rather than a path glued onto nothing.
  • Local binds loopback only, with no opt-in (13.4). A non-loopback --host on generate-and-open, or OPENDOX_BIND_HOST for the runtime's own listener, is refused, naming the loopback rule.
    • The set is serve.py's own LOOPBACK_HOSTS (127.0.0.1, ::1, localhost). 13.4 asks for "the same judgement at the mode's own boundary".
    • config cannot import serve, so it spells the set, and a test holds the two equal. 127.0.0.2 is therefore refused, because the document server does not treat it as loopback either.
  • Hosted, or unset, with no issuer refuses, naming OPENDOX_OIDC_ISSUER (13.5).
    • generate-and-open asks for the issuer first (require_the_hosted_issuer), and then loads the whole runtime configuration. The serving process is the one whose settings are the install's (13.4a; R1Q16 (i)).
    • So a hosted run with nothing configured names the issuer and --local, not the OPENDOX_DATABASE_URL that load_settings happens to ask for first (plan 034's requirement-13 scenario 2).
    • load_settings keeps its own order for every verb that relies on it.
  • Hosted is otherwise unchanged (13.6): same broker, same pinned issuer, same order of refusals, and a hosted bind may still be 0.0.0.0.
  • The shape is resolved first. generate-and-open resolves it before it scans, mints or binds anything, so each refusal exits 1 at once. That covers F13.1's test "$rc" -ne 124.

Holder readings (coordinator, 2026-09-30, on openxFactory#656; Brett may overrule)

  1. opendox-runtime runtime serve refuses under local (refusal: local-mode-has-no-broker). Every /api/v1 route verifies a broker-signed token, and a local install is served by generate-and-open --local. In release 1 its document surface reads nothing from the store (R1Q16 (ii)).
  2. runtime status under local reports broker_keys: "not configured (local mode)" and broker_discovery: null. It never builds a verifier, and its exit code is the database's verdict alone, so F13.1's set -e survives.
  3. A broker setting beside local is refused, naming each one: OPENDOX_OIDC_ISSUER, OPENDOX_OIDC_AUDIENCE and OPENDOX_OIDC_JWKS_URL. The values are never repeated. T072 adds the two DSNs to the list.
  4. An unrecognised value (Local, single-user …) is refused, naming local and hosted, and matching is case-sensitive.

Deliberately NOT here

  • T070 is the identity half; T072 is the datastore half. Under local, load_settings still takes the DSNs from the environment, and generate-and-open --local loads no database setting. T072 supplies both DSNs from the bundled server (13.1) and refuses operator DSNs beside local.
  • T073's install block is not here. serve.py and pyproject.toml are untouched.

Outside src/ and tests/

  • deploy/ — one line, and the task requires it. deploy/compose/.env.example gains OPENDOX_INSTALL_MODE=hosted with its comment. config.py's header makes .env.example the place every setting is named, and tests_runtime/test_deploy_shape.py::test_every_runtime_setting_is_documented_in_env_example is parametrized over SETTINGS, so a new setting without the line is red. Neither the compose file nor the Kubernetes base changes: the default is already hosted.
  • docs/: untouched. 10.3's README line belongs to T076, in the openDox root.
  • One existing test changes, tests/test_doxbench_entrypoint.py's fixture. It drives cmd_generate_and_open with no settings, and the unset default is now hosted, which refuses without an issuer. So it passes --local and scrubs the runtime settings first.

The falsifier

F13.1's refusal probes, verbatim from # LOCAL mode REFUSES a non-loopback bind to the end of the block, plus T070's own disagreeing pair. Two deviations are forced by the base, and neither weakens a check:

Each probe on its own. BEFORE is #60's head f097fd8; AFTER is this branch:

=== BEFORE (f097fd8)
FAIL local-refuses-non-loopback-bind (rc=1): Traceback (most recent call last):   File ".../src/opendox/consumer_reach.py" …
FAIL hosted-no-issuer-names-it (rc=1): Traceback (most recent call last): …
FAIL unset-default-refuses-identically (rc=1): Traceback (most recent call last): …
FAIL disagreeing-flag-and-setting-names-both (rc=2): usage: ideation-dashboard [-h] … (argparse: no --local)
=== AFTER (this branch)
PASS local-refuses-non-loopback-bind (rc=1)
PASS hosted-no-issuer-names-it (rc=1)
PASS unset-default-refuses-identically (rc=1)
PASS disagreeing-flag-and-setting-names-both (rc=1)

The whole sequence under set -euo pipefail (F13.1's own form), AFTER:

probe 1: local refuses a non-loopback bind
  rc=1; stderr: generate-and-open refused: --host '0.0.0.0' is not a loopback address, and a LOCAL install binds LOOPBACK ONLY (127.0.0.1, ::1, localhost). … There is no opt-in: …
probe 2: load_settings (T071's block, unchanged)
probe 3: hosted with no issuer refuses naming it
  rc=1; stderr: generate-and-open refused: OPENDOX_OIDC_ISSUER is required and is not set, and this install is HOSTED …
probe 4: the unset default refuses identically
  rc=1; stderr: generate-and-open refused: OPENDOX_OIDC_ISSUER is required and is not set, and this install is HOSTED (OPENDOX_INSTALL_MODE is unset, and unset means hosted) …
probe 5 (T070's own, not F13.1's): a disagreeing flag and setting are refused naming both
  rc=1; stderr: generate-and-open refused: --local selects the LOCAL install and OPENDOX_INSTALL_MODE=hosted selects the HOSTED one. …
F13.1 REFUSALS + T070 PAIR: ALL PASSED

In the suite, the same probes run as bounded child processes of python -m opendox.cli (timeout=30, and a TimeoutExpired fails as "a server that STARTED") in tests/test_install_mode_entrypoint.py. The loader-level cases are in tests_runtime/test_install_mode.py.

A mutant of each new refusal, killed

Each mutant was applied alone and the three T070 test files run with -x, with a 240 s bound so a hang could not pass for a kill:

mutant killed by
M1 the disagreeing pair accepted (flag wins) test_a_flag_and_a_setting_that_disagree_are_refused_naming_both
M2 an unknown value read as hosted test_an_unrecognised_value_is_refused_naming_the_two[Local]
M3 unknown values matched case-insensitively same, [Local]
M4 the default flipped to local (the safety) test_the_selector_has_two_values_and_its_default_is_hosted
M5 the hosted issuer-first check a no-op test_a_hosted_install_with_no_issuer_refuses_naming_it[None]
M6 the local loopback refusal a no-op test_a_local_install_refuses_a_non_loopback_bind_naming_the_rule[0.0.0.0]
M7 broker settings beside local accepted test_a_broker_setting_beside_the_local_mode_is_refused_by_name[OPENDOX_OIDC_ISSUER]
M8 the local bind set widened (127.0.0.2) test_a_local_install_refuses_a_non_loopback_bind_naming_the_rule[127.0.0.2]
M9 local still requires the issuer test_a_local_install_needs_no_broker[setting]
M10 runtime serve serves under local test_runtime_serve_refuses_under_the_local_mode
M11 runtime status probes a broker under local test_runtime_status_under_the_local_mode_probes_no_broker
M12 generate-and-open ignores --local test_the_flag_refuses_a_non_loopback_bind_exactly_as_the_setting_does
M13 generate-and-open skips the install shape test_local_mode_refuses_a_non_loopback_bind_naming_the_rule

M10 at first HUNG rather than failed: the un-stubbed case started a real uvicorn listener. That case now stubs uvicorn and the app, so the mutant fails at once. The table above is the re-run.

Fix round: the fixture's repository ignores the user's git config (32683e8)

Copilot's review at b50e3b1 (finding) was real. tests/test_install_mode_entrypoint.py's corpus fixture ran git commit under the caller's global git configuration, so a global commit.gpgsign=true failed the setup before any probe ran.

It now sets GIT_CONFIG_GLOBAL=/dev/null and GIT_CONFIG_NOSYSTEM=1, as tests/test_checkout_head.py does. Measured with a hostile global config (commit.gpgsign = true, gpg.program = /bin/false):

  • at b50e3b1: 7 passed, 7 errors;
  • at 32683e8: 14 passed.

Replied. The probes are unchanged.

Fix round 2: runtime migrate and reset refuse what a local install cannot be (525f61c)

Copilot's second review, at 32683e8, raised two points in its overview, with no inline thread. Both are answered on the PR.

  1. Real, and fixed. load_migration_settings recorded local but never asked refuse_what_a_local_install_cannot_be. So runtime migrate and a confirmed runtime reset accepted a broker setting, or a non-loopback OPENDOX_BIND_HOST, beside OPENDOX_INSTALL_MODE=local. They now refuse at configuration, before any database is reached.
    • Seven new cases in tests_runtime/test_install_mode.py: the three broker settings × {migrate, reset --confirm …}, plus the bind.
    • Against 32683e8's config they give 7 failed; here they pass.
  2. Pre-existing, and not changed here: ::1 passes the loopback rule, but the server cannot bind it. serve.build_server is IPv4-only (ThreadingHTTPServer), while serve.LOOPBACK_HOSTS lists ::1.

Fix round 3: status reports a local install's broker on its early return too (02dadc5)

Copilot's review at 525f61c raised one point in its overview, with no inline thread. It is real, and it is answered on the PR.

  • With the runtime extra absent, runtime status returns early, and that return said broker_keys: "not probed" for every install.
  • Local now gets the full report's answer there: "not configured (local mode)", with broker_discovery: null. Both returns write it through one helper.
  • Hosted still says "not probed" (13.6).
  • The new case runs with opendox.runtime.db absent from sys.modules. [local] fails against 525f61c's runtime/cli.py and passes here; [hosted] passes in both.
  • Four mutants of the fix are killed.

Fix round 4: a healthy local status is proven to exit 0; RuntimeSettings' invariants are scoped (859b37b6)

Copilot's review at 02dadc55 opened two threads, both real. Both are answered and resolved.

  • r4139922962: both local status cases forced a database fault, so nothing proved exit 0.
    • A DB-backed case now runs local status against a migrated schema on the suite's server and asserts ok, exit 0, and a broker that is not configured and never probed.
    • With the local return mutated to ok=False, the new case fails and the module's other 40 pass. Before this commit, that mutant survived.
  • r4139922999: the docstring's broker statements hold for load_settings only, so they are now scoped to their loader. load_migration_settings carries the migration sentinels in either shape. This is documentation only.

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
b50e3b1 (T070) 2531 2520 11 0 0
525f61c (fix round 2) 2538 2527 11 0 0
02dadc5 (fix round 3) 2540 2529 11 0 0
this branch, 859b37b6 (fix round 4) 2541 2530 11 0 0

026f00ea (fix round 5) is a docstring change. The install-mode module now names its one DB-backed case (r4146171212, resolved), and the module runs 41 passed.

That is +56 cases: this PR's two new files, plus the .env.example census's new parameter (+46 at b50e3b1), fix round 2's seven, fix round 3's two, and fix round 4's one. EXPECT_SKIPPED=11 holds exactly, and the floors (MIN_SELECTED=2476, MIN_PASSED=2465) allow the rise unchanged.

Downstream, for the holder

🤖 Generated with Claude Code

Summary by Sourcery

Add fail-closed hosted and local install-mode selection across the document-generation and runtime commands, preserving hosted behavior while enabling broker-free loopback operation.

New Features:

  • Add the OPENDOX_INSTALL_MODE setting and generate-and-open --local flag for explicitly selecting hosted or local operation.
  • Support local single-user operation without broker configuration while enforcing loopback-only binding.

Bug Fixes:

  • Prevent conflicting or invalid install-mode selections and broker settings from being silently accepted.
  • Ensure hosted runs without an issuer fail with a clear issuer-specific refusal before other configuration checks.
  • Prevent runtime API serving and broker probing in local mode, while reporting local status correctly.

Enhancements:

  • Expose the resolved install mode in runtime settings and status output, with empty broker endpoints for local installations.
  • Apply local-mode validation consistently to migration and reset commands.

Deployment:

  • Document the new install-mode setting in the deployment environment example.

Tests:

  • Add entrypoint and runtime coverage for install-mode selection, refusal behavior, local status and serve behavior, and hosted-mode compatibility.
  • Make repository fixtures independent of users' global and system Git configuration.

…n --local (plan 034)

OPENDOX_INSTALL_MODE (`local` | `hosted`, default `hosted`) is read in
runtime/config.py beside OPENDOX_OIDC_ISSUER and decides the install shape
(#1144 13.4). `generate-and-open --local` makes the same selection
(R1Q15 (b), as T007 batch H's 13.4 addendum reads); with neither the install
is hosted (13.5).

- A flag and a setting that disagree (`--local` beside
  OPENDOX_INSTALL_MODE=hosted) are refused, naming both. This is plan 034's
  fail-closed reading (Principle VII); no answer rules it and batch H does
  not write it into #1144.
- LOCAL needs no broker: issuer, audience and key-set URL are empty.
- LOCAL binds loopback only, with no opt-in. A non-loopback `--host` or
  OPENDOX_BIND_HOST is refused, naming the rule. The set is serve.py's own
  LOOPBACK_HOSTS, and a test holds the two equal.
- HOSTED, set or by default, with no issuer refuses, naming
  OPENDOX_OIDC_ISSUER. generate-and-open asks the issuer first, so a run with
  nothing configured names it and `--local`. The hosted mode is otherwise
  unchanged (13.6).

Holder readings on openxFactory#656 (Brett may overrule):
- `runtime serve` refuses under local, because the API's identity is the
  broker's.
- `runtime status` under local reports broker_keys "not configured (local
  mode)" and does not count it as a fault.
- A broker setting beside local is refused by name.
- An unrecognised mode value is refused, case-sensitively.

The document server's generate-and-open resolves the shape before it scans,
mints or binds anything. The hosted path loads the whole runtime
configuration (R1Q16 (i); 13.4a).

Also:
- deploy/compose/.env.example gains OPENDOX_INSTALL_MODE=hosted, which
  test_every_runtime_setting_is_documented_in_env_example requires of every
  SETTINGS entry.
- tests/test_doxbench_entrypoint.py's fixture now selects `--local` and
  scrubs the runtime settings, since the unset default is hosted and refuses
  with no issuer.

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:22
@sourcery-ai

sourcery-ai Bot commented Sep 30, 2026

Copy link
Copy Markdown

Reviewer's Guide

Implements phase-3 standalone install-mode selection: hosted remains the safe default, while explicit local mode is selected by OPENDOX_INSTALL_MODE=local or generate-and-open --local, enforced with fail-closed conflict handling, loopback-only binding, no broker requirements, and early refusals. Runtime commands and status reporting are adapted accordingly, with extensive bounded entrypoint and configuration tests plus deployment documentation.

Sequence diagram for install-mode resolution in generate-and-open

sequenceDiagram
    participant User
    participant CLI as generate-and-open
    participant Config as runtime_config
    participant Runtime as Runtime configuration
    User->>CLI: generate-and-open [--local]
    CLI->>Config: install_mode(env, local_flag)
    alt conflicting explicit selections
        Config-->>CLI: ConfigurationError
        CLI-->>User: refusal and exit 1
    else local mode
        Config->>Config: refuse_a_non_loopback_local_bind(--host, host)
        Config->>Config: refuse_what_a_local_install_cannot_be(env)
        Config-->>CLI: LOCAL
    else hosted or unset
        Config->>Config: require_the_hosted_issuer(env)
        Config->>Runtime: load_settings(env)
        Runtime-->>CLI: HOSTED settings
    end
    CLI->>CLI: generate and serve only after resolution
Loading

File-Level Changes

Change Details Files
Add install-mode resolution and fail-closed selection semantics to runtime configuration and the document-generation CLI.
  • Introduce OPENDOX_INSTALL_MODE with exact local/hosted values and a hosted default.
  • Add generate-and-open --local, reject disagreement with the environment selector, and resolve the mode before scanning, generating, or serving.
  • Require the hosted issuer explicitly while allowing local mode to omit broker settings.
  • Reject broker-only settings and non-loopback binds for local installs, while preserving hosted behavior.
src/opendox/runtime/config.py
src/opendox/cli.py
Make runtime service and status behavior aware of local installs without attempting broker operations.
  • Refuse runtime serve in local mode with a structured no-broker explanation.
  • Report broker configuration as unavailable in local-mode status and avoid verifier construction.
  • Expose the resolved install mode in redacted settings and make empty local discovery/JWKS URLs explicit.
src/opendox/runtime/cli.py
src/opendox/runtime/config.py
Add comprehensive selector, boundary, CLI-entrypoint, and runtime behavior coverage.
  • Test hosted defaults, local selection, invalid values, conflicting selectors, broker-setting refusals, and loopback enforcement.
  • Run bounded subprocess probes to verify refusals occur before a server starts and preserve expected exit behavior.
  • Cover local serve refusal, broker-free status, hosted compatibility, and migration-loader selector validation.
tests/test_install_mode_entrypoint.py
tests_runtime/test_install_mode.py
Update an existing single-user fixture and deployment configuration documentation for the new setting.
  • Make the doxbench entrypoint explicitly select local mode and clear inherited runtime settings.
  • Document the hosted default in the compose environment example.
tests/test_doxbench_entrypoint.py
deploy/compose/.env.example

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

The new Git-backed fixture inherits global Git configuration and can fail before exercising the behavior under test.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Adds fail-closed hosted/local install selection and broker-free, loopback-only local operation.

Changes:

  • Adds OPENDOX_INSTALL_MODE and generate-and-open --local.
  • Adapts runtime serve/status behavior for local mode.
  • Documents and tests install-mode validation and refusals.
File Description
src/​opendox/​runtime/​config.py Defines and validates install modes.
src/​opendox/​runtime/​cli.py Adjusts runtime serve and status behavior.
src/​opendox/​cli.py Adds and resolves --local.
deploy/​compose/​.env.example Documents hosted install mode.
tests/​test_install_mode_entrypoint.py Adds bounded entrypoint tests.
tests_runtime/​test_install_mode.py Adds runtime configuration coverage.
tests/​test_doxbench_entrypoint.py Selects local mode in the fixture.

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

Comment thread tests/test_install_mode_entrypoint.py
…it config (Copilot review)

`tests/test_install_mode_entrypoint.py`'s `corpus` fixture ran `git commit`
under the caller's global and system git configuration. A global
`commit.gpgsign=true` therefore failed the setup before any install-mode
probe ran. Measured with a hostile global config (`commit.gpgsign = true`,
`gpg.program = /bin/false`): 7 errors at b50e3b1, 14 passed here. The
fixture now sets GIT_CONFIG_GLOBAL=/dev/null and GIT_CONFIG_NOSYSTEM=1, as
tests/test_checkout_head.py does.

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 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

IPv6 loopback serving fails, and migration commands bypass local-mode environment validation.

Review effort: Balanced
Findings: None

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

In code that hasn't changed since last review

Medium severity Support IPv6 localhost or reject ::1 host

src/​opendox/​runtime/​config.py:1549

::1 is accepted here (and the new tests assert that it is valid), but generate-and-open passes it to serve.build_server, which constructs the standard ThreadingHTTPServer with its default IPv4 address family. Binding --host ::1 therefore raises gaierror instead of starting the local server; serve.server_url would also need IPv6 bracket handling. Please make the server/URL path IPv6-aware, or avoid advertising ::1 as supported.

Medium severity Validate local environment before migration settings construction

src/​opendox/​runtime/​config.py:1825

This records local but skips the local-shape validation, so runtime migrate and confirmed runtime reset silently accept OPENDOX_OIDC_ISSUER, OPENDOX_OIDC_AUDIENCE, or OPENDOX_OIDC_JWKS_URL beside OPENDOX_INSTALL_MODE=local. That contradicts the new fail-closed rule applied by load_settings and generate-and-open. Validate the local environment here too before constructing the migration settings.

…ot be (Copilot review)

load_migration_settings recorded OPENDOX_INSTALL_MODE=local but never asked
refuse_what_a_local_install_cannot_be. So `runtime migrate` and a
confirmed `runtime reset` accepted OPENDOX_OIDC_ISSUER, OPENDOX_OIDC_AUDIENCE,
OPENDOX_OIDC_JWKS_URL or a non-loopback OPENDOX_BIND_HOST beside `local`,
which load_settings and generate-and-open both refuse. They now refuse them
at configuration, before any database is reached.

Seven new cases:
- the three broker settings x {migrate, reset};
- the bind.

All seven fail at 32683e8 and pass here. Full suite: 2538 selected, 2527
passed, 11 skipped, 0 failed.

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:58
@brettheap

Copy link
Copy Markdown
Contributor Author

Copilot's review at 32683e8 raised two "previously missed" points in its overview, with no inline thread. Both are answered here.

1. Migration commands bypass the local-mode validation. Real, and fixed at 525f61c.

  • load_migration_settings recorded local but never asked refuse_what_a_local_install_cannot_be. So runtime migrate and a confirmed runtime reset accepted a broker setting, or a non-loopback OPENDOX_BIND_HOST, beside OPENDOX_INSTALL_MODE=local.
  • It now refuses them at configuration, before any database is reached.
  • Seven new cases (the three broker settings × {migrate, reset --confirm …}, plus the bind): 7 failed against 32683e8's config, and all pass here.
  • Full suite: 2538 selected, 2527 passed, 11 skipped, 0 failed.

2. ::1 is accepted, but the server cannot bind it. That is pre-existing in serve.py, and it is not changed here.

  • serve.build_server has never bound ::1. Measured at DRAFT (phase 3, after T063): T071, 13.2 and 13.3 — load_settings refuses a non-PostgreSQL DSN and a collapsed DSN pair (plan 034) #60's head f097fd8, before this PR, with the stand-in snapshot source: 127.0.0.1 bound ('127.0.0.1', 39311) and ::1 FAILS … gaierror [Errno -9] Address family for hostname not supported.
  • That is because ThreadingHTTPServer is IPv4, while serve.LOOPBACK_HOSTS lists ::1.
  • This PR's local rule is deliberately serve.LOOPBACK_HOSTS itself. 13.4 asks for "the same judgement at the mode's own boundary", and a test holds the two sets equal. So ::1 passes the loopback rule, exactly as the document server already classifies it, and then fails at the bind exactly as it does in hosted mode today.
  • Making the serve path IPv6-aware (address family, and server_url's brackets) is a serve.py change, and serve.py is not in T070's file set or single-writer slot. It is recorded for the holder rather than done around the plan.

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

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

Local status reports the wrong broker state when the runtime extra is unavailable.

Review effort: Balanced
Findings: None

Previously missed (1)

In code that hasn't changed since last review

Medium severity Populate local broker status before runtime dependency early return

src/​opendox/​runtime/​cli.py:783

The local-mode branch runs after the existing runtime-extra import at lines 694–700. If psycopg is absent, cmd_status returns there with broker_keys: "not probed" and no broker_discovery, so local status does not consistently report the promised “not configured (local mode)”/null broker state. Populate the local broker fields before that early return (while still skipping build_verifier later).

…n too (Copilot review)

When the runtime extra is absent, `runtime status` returns early, and that
return said `broker_keys: "not probed"` for every install. A local install's
broker is not configured whether or not the extra is present. That answer
comes from the configuration, not from a probe, so the early return now gives
the local install the answer the full report gives: `"not configured (local
mode)"`, with `broker_discovery: null`. Both returns write it through one
helper, so the two cannot drift. A hosted install's early return still reads
"not probed", as before (13.6).

The branch is covered now, so its `pragma: no cover` goes. A new case runs
both shapes with `opendox.runtime.db` absent from `sys.modules`. Before
(`525f61c`'s runtime/cli.py): local 1 failed and hosted passed. After: both
pass. Four mutants of the fix are killed. Full suite: 2540 selected, 2529
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:12

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

The healthy local-status exit path lacks regression coverage proving that an absent broker does not cause failure.

Review effort: Balanced
Findings: None

Previously missed (1)

In code that hasn't changed since last review

Medium severity Add healthy-local status test to verify exit code 0

tests_runtime/​test_install_mode.py:321

This case cannot verify the promised healthy-local exit code: port 1 guarantees the database path has already set ok = False, so a regression that also treats the intentionally absent broker as unhealthy would still satisfy code == 1. Add a local-status case with a successful/stubbed database and migration probe and assert exit code 0; that is the path needed to prove status survives set -e when only the broker is absent.

@brettheap

Copy link
Copy Markdown
Contributor Author

Copilot's review at 525f61c raised one "previously missed" point in its overview, with no inline thread: "Populate local broker status before runtime dependency early return", at src/opendox/runtime/cli.py:783.

It is real, and it is fixed at 02dadc5.

  • The runtime extra's ImportError return said broker_keys: "not probed" for every install.
  • A local install's broker is not configured whether or not the extra is present. That answer comes from its configuration, not from a probe, so that return now gives local the full report's answer: "not configured (local mode)", with broker_discovery: null.
  • Both returns write it through one helper (_report_the_local_broker). A hosted install's early return still says "not probed" (13.6). ok is still false there, because the database was not probed.
  • The branch is covered now, so its pragma: no cover goes.

Measured

  • New case test_runtime_status_without_the_runtime_extra_reports_the_broker_by_mode[local|hosted], with opendox.runtime.db set to None in sys.modules.
    • Against 525f61c's runtime/cli.py: [local] fails and [hosted] passes.
    • Here: both pass.
  • Four mutants are killed: local reads "not probed"; hosted gets the local answer; the helper omits broker_discovery; the early return counts as ok.
  • Full suite: 2540 selected, 2529 passed, 11 skipped, 0 failed.

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

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 local status success path lacks coverage, and the RuntimeSettings documentation overstates its invariant.

Review effort: Balanced
Findings: 1 Medium severity · 1 Low severity

Open (2)

Comment thread src/opendox/runtime/cli.py
Comment thread src/opendox/runtime/config.py Outdated
…tings' broker invariants are scoped (Copilot review)

A local `status` returns `ok` on the database's verdict alone. Both earlier
local cases forced a database fault and asserted exit 1, so a regression
that also counted the absent broker as a fault would still have passed. A
DB-backed case now runs `status` for a local install against a migrated
schema on the suite's own server (`database` and `postgres_dsn`, with the
schema selected in the DSN). It asserts `ok` true, exit 0, the database
reachable with nothing pending and no drift, and the broker reported as not
configured and never probed. Measured: with the local return mutated to
`ok=False`, this case fails and the other 40 in the module pass.

`RuntimeSettings`' docstring said that a local install's issuer and audience
are empty and that a hosted one always carries a real issuer. That is true of
`load_settings` alone. `load_migration_settings` carries the migration
sentinels in either shape. The docstring now scopes each statement to its
loader.

Full suite: 2541 selected, 2530 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:04

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

It remains an explicitly stacked draft awaiting T063, and one documentation inconsistency remains.

Review effort: Balanced
Findings: 1 Low severity

Open (1)
Resolved since last review (2)

Comment thread tests_runtime/test_install_mode.py Outdated
…ed (Copilot review)

The module docstring called every case hermetic. Since the fourth fix round,
one is not: `test_runtime_status_of_a_healthy_local_install_exits_zero` takes
the suite's `postgres_dsn` and `database` fixtures, because a healthy local
`status` exits 0 only against a database that answers. The docstring now
names that case and says it is skipped without Postgres and fails under CI,
like every DB-backed case. It says the rest stay hermetic. Docstring only:
the module runs 41 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 AI balanced review requested due to automatic review settings September 30, 2026 15:51
@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

It remains an intentionally stacked draft awaiting T063, holder approval, and completion of the current validation run.

Review effort: Balanced
Findings: None

Resolved since last review (1)

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