feat(chassis): SDK-native chassis behind L9_CHASSIS + W4-04 audit pool wiring - #150
Merged
Conversation
… pool Two changes that cannot be split: the SDK chassis consumes the same engine.handlers surface and the same GraphLifecycle that W4-04 modifies. SDK-native chassis (dormant by default) - chassis/entrypoint.py: single uvicorn target dispatching on L9_CHASSIS (legacy -> chassis_app.create_app, sdk -> node_app.create_app). Every launch site (Makefile, Dockerfile.prod, scripts/entrypoint.sh, compose) now points here, so switching chassis is config, not a command change. - chassis/node_app.py: builds the app via constellation-node-sdk create_node_app. SdkLifecycleAdapter bridges engine.boot.GraphLifecycle (legacy LifecycleHook) onto the SDK's unrelated LifecycleHook ABC, which leaves engine/boot.py free of SDK imports during dual-run. - chassis/handler_registration.py: registers ACTION_HANDLERS with the SDK registry, wrapping each with the PacketEnvelope audit side effect that chassis/actions.py applies on the legacy path. - engine/handlers.py now exports ACTION_HANDLERS as the single action registry; chassis/actions.py consumes it instead of a hand-maintained dict, so the two chassis cannot drift. test_chassis_parity.py locks that. W4-04 Postgres audit pool - The ComplianceEngine consumer side is already on main; this adds the missing wiring. GraphLifecycle.startup creates an asyncpg pool when POSTGRES_DSN is set and passes it through init_dependencies to EngineState.db_pool; shutdown closes it. Soft dependency: an unset or unreachable DSN leaves db_pool None and logs a warning rather than blocking startup, matching the existing Neo4j degrade path. - docker-compose gains a postgres service seeded from packet_store.sql. Chassis consolidation - Deletes chassis/app.py and chassis/auth/app.py, which duplicated chassis/chassis_app.py. All imports and launch sites repointed. Settings - Under pytest, env_file is None so a developer's local .env cannot leak opt-in flags into the suite. Verified: 1667 passed / 15 skipped / 56 xfailed, ruff clean, contract scanner clean. mypy reports one pre-existing syntax error on main. L9_META headers are left at main's schema v1 so this does not collide with the schema v2 stamp in #147. Co-authored-by: Cursor <cursoragent@cursor.com>
|
📋 Best Practices for Large Changes
✅ This PR passes the blocking limit but is larger than recommended. |
3 tasks
ACTION_HANDLERS is now the single action->handler map for both chassis; CONTRACT-02 and the compliance workflow comment still described the pre-rename chassis/app.py layout.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| - name: Set up Python | ||
| uses: actions/setup-python@v7 | ||
| uses: actions/setup-python@v5 |
…nit tests Remediation-Cycle: #150/cycle-1 Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Pull request overview
Adds an opt-in SDK-native chassis implementation (selected via L9_CHASSIS) while keeping the existing legacy chassis as the default, and wires an optional Postgres asyncpg pool into engine state for ComplianceEngine audit flushing. The PR also consolidates chassis entrypoints, unifies action registration via a single engine.handlers.ACTION_HANDLERS registry, and updates tests/docs/dev tooling to match.
Changes:
- Introduce
chassis.entrypoint:create_appas the single uvicorn factory, dispatching between legacy and SDK chassis viaL9_CHASSIS. - Add optional Postgres audit pool wiring (
POSTGRES_DSN) throughGraphLifecycle → init_dependencies → EngineState.db_pool → ComplianceEngine. - Consolidate/remap chassis imports, and add parity/SDK-chassis unit + contract tests to prevent action-set drift.
Reviewed changes
Copilot reviewed 27 out of 29 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_node_app.py | New unit coverage for SDK chassis app construction, handler registration, packet audit wrapper, and gate-only ingress middleware. |
| tests/unit/test_chassis_app.py | Updates imports/docs to reference chassis/chassis_app.py after consolidation. |
| tests/unit/test_api_error_handling.py | Updates imports to the consolidated legacy chassis module. |
| tests/contracts/test_env_contract.py | Updates contract source reference from removed chassis/app.py to chassis/chassis_app.py. |
| tests/contracts/test_chassis_parity.py | New contract test ensuring legacy + SDK chassis share the same action registry/dispatch targets. |
| scripts/entrypoint.sh | Switches uvicorn target to chassis.entrypoint:create_app and logs selected chassis. |
| Makefile | Switches local run target to new entrypoint and adds a local-api-sdk target for SDK chassis. |
| engine/state.py | Adds optional db_pool to EngineState lifecycle and health reporting. |
| engine/security/P2_9_llm_schemas.py | Reorders missing-key vs missing-package errors for clearer failure mode in CI/dev. |
| engine/handlers.py | Extends dependency injection to include optional db_pool; introduces ACTION_HANDLERS as single action registry and refactors register_all() to use it. |
| engine/config/settings.py | Makes settings hermetic under pytest by disabling .env loading; adds postgres_dsn setting. |
| engine/boot.py | Creates optional asyncpg pool on startup and injects it into engine dependencies; avoids double-close on shutdown. |
| docs/FEATURE_GATES.md | Documents new chassis selection and Postgres audit pool; expands feature gate table and operational guidance. |
| Dockerfile.prod | Switches container CMD to chassis.entrypoint:create_app. |
| docker-compose.yml | Adds Postgres service and env wiring; switches app selection to L9_CHASSIS entrypoint; updates healthcheck logic. |
| contracts/contract_02.yaml | Updates CONTRACT-02 to formalize ACTION_HANDLERS as the single action→handler source for both chassis paths. |
| chassis/node_app.py | New SDK-native chassis app factory built via constellation-node-sdk, including gate-only ingress middleware and lifecycle adapter. |
| chassis/middleware.py | Updates usage comment to reference chassis/chassis_app.py. |
| chassis/handler_registration.py | New SDK handler registration bridging ACTION_HANDLERS into SDK registry with PacketEnvelope audit persistence. |
| chassis/entrypoint.py | New unified uvicorn factory dispatching between legacy and SDK chassis implementations. |
| chassis/chassis_app.py | Renames/updates module text to reflect consolidation from chassis/app.py. |
| chassis/auth/app.py | Removed legacy duplicate chassis/auth app factory implementation. |
| chassis/app.py | Removed deprecated alias module (now fully consolidated into chassis/chassis_app.py). |
| chassis/actions.py | Legacy action execution now consumes engine.handlers.ACTION_HANDLERS to avoid drift. |
| chassis/init.py | Re-exports updated to import from chassis.chassis_app. |
| artifacts/spec_checklist.json | Formatting/termination fix (trailing bracket alignment). |
| artifacts/coverage_matrix.json | Formatting/termination fix (trailing brace alignment). |
| .github/workflows/compliance.yml | Pins setup-python action version and updates comment referencing consolidated chassis module. |
| .env.template | Adds SDK chassis configuration examples and Postgres DSN wiring guidance. |
Suppressed comments (1)
docs/FEATURE_GATES.md:253
- Rollback guidance says LLM-backed calls raise FeatureNotEnabled when OPENAI_API_KEY is unset, but the current implementation raises RuntimeError. Please align this wording with the actual exception type/message.
### Rollback
- Unset `OPENAI_API_KEY`. Any LLM-backed call raises `FeatureNotEnabled`
cleanly rather than failing at the network layer.
Comment on lines
+62
to
+67
| origin_kind = str(provenance.get("origin_kind", "")).strip().lower() | ||
| if origin_kind != "gate": | ||
| return f"provenance.origin_kind must be 'gate', got {origin_kind!r}" | ||
|
|
||
| if not provenance.get("resolved_by_gate", False): | ||
| return "provenance.resolved_by_gate must be true" |
Comment on lines
+54
to
+60
| request_packet = inflate_ingress( | ||
| action=action, | ||
| payload=payload, | ||
| tenant=tenant, | ||
| trace_id=trace_id, | ||
| source_node="gate", | ||
| ) |
Comment on lines
+240
to
+244
| Input sanitization and output schema validation wrap the actual `_call()` | ||
| integration, which calls the OpenAI SDK using `OPENAI_API_KEY`. Raises | ||
| `FeatureNotEnabled("LLM SDK", flag="OPENAI_API_KEY")` only when the key is | ||
| missing — the provider selection itself (`LLM_PROVIDER=openai` | | ||
| `openai-compatible`) does not gate the feature. |
…de-sdk Remediation-Cycle: #150/cycle-2 Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 29 changed files in this pull request and generated no new comments.
Suppressed comments (2)
chassis/handler_registration.py:55
inflate_ingress(..., source_node="gate")hard-codes the Gate node name.chassis/node_app.pyvalidates incoming packets againstL9_GATE_NODE_NAME, so when that env var is not "gate" the persisted/audited PacketEnvelope will record the wrongsource_nodevalue.
trace_id = str(uuid.uuid4())
start = time.time()
request_packet = inflate_ingress(
action=action,
contracts/contract_02.yaml:34
verification.testis currently a pytest node id (contains::), but tooling in this repo (e.g.tools/contract_report.py) treatsverification.testas a filesystem path. Keepingverification.testas a path maintains consistency with the othercontracts/contract_*.yamlfiles and avoids contract-report undercounting coverage; you can still keep the granular node ids inverification.tests.
scanner_rules:
- DI-001
test: tests/contracts/test_contracts.py::TestContract02HandlerInterface
tests:
- tests/contracts/test_contracts.py::TestContract02HandlerInterface
- tests/contracts/test_chassis_parity.py
- tests/unit/test_node_app.py
Co-authored-by: Cursor <cursoragent@cursor.com>
L9 Audit Harness Report
Step Results
Architecture Audit Findings
See Spec Coverage
See Next StepsAll checks passed. Safe to merge. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated no new comments.
Suppressed comments (3)
docs/FEATURE_GATES.md:8
- docs/FEATURE_GATES.md now uses an L9_META schema v2 header, but the PR description states this PR is intentionally staying on schema v1 to avoid colliding with the schema v2 stamp work. This also diverges from other docs in this repo (most are l9_schema: 1 and include an owner field). Aligning this header to schema v1 (or updating the PR description) would avoid confusion and merge conflicts with #147.
l9_schema: 2
origin: engine-specific
engine: graph
docs/FEATURE_GATES.md:40
- The Quick Reference explanation says the “Default” column comes from engine/config/settings.py, but at least one flag listed in the table (LLM_PROVIDER) is read directly from environment in engine/security/P2_9_llm_schemas.py rather than Settings. This makes the guidance misleading for operators trying to reason about defaults.
The **Default** column is the code-level default in `engine/config/settings.py`
(what ships if no environment variable is set — the seL4 "dormant mechanism"
baseline). The **Local `.env`** column reflects this repo's checked-in-locally,
gitignored `.env` (see `.env.template`), which the operator may activate
independently of the code default.
engine/handlers.py:90
- init_dependencies() sets EngineState fields directly (including _initialized later in this function), but it never sets EngineState._initialize_time like EngineState.initialize() does. That leaves EngineState.health_check()['initialize_time'] at 0.0, making health snapshots misleading.
state = get_state()
state._graph_driver = graph_driver
state._domain_loader = domain_loader
state._db_pool = db_pool
Count PR size block threshold on additions only so deletion-heavy migrations are not double-penalized, and allowlist the Makefile l9-dev-password local-dev fixture for gitleaks. Co-authored-by: Cursor <cursoragent@cursor.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why one PR
These two bodies of work share
engine/handlers.pyandengine/boot.py. The SDK chassis wraps the sameGraphLifecyclethat W4-04 modifies, and both depend on the newACTION_HANDLERSregistry. Splitting them would mean shipping a broken intermediate state or resolving the same conflict twice.1. SDK-native chassis — dormant by default
L9_CHASSIS=legacy(the default) preserves today's behavior exactly.L9_CHASSIS=sdkselects the new path.chassis/entrypoint.pyL9_CHASSISchassis/node_app.pyconstellation-node-sdkcreate_node_appchassis/handler_registration.pyACTION_HANDLERSwith the SDK registry + packet audit wrapperSdkLifecycleAdapterbridgesengine.boot.GraphLifecycle(which subclasses the legacyLifecycleHook) onto the SDK's unrelatedLifecycleHookABC. Keeping the adapter inchassis/meansengine/boot.pystays free of SDK imports while both chassis run in parallel.Every launch site now points at
chassis.entrypoint:create_app— Makefile,Dockerfile.prod,scripts/entrypoint.sh,docker-compose.yml— so switching chassis is a config change, not a command change.Single action registry
engine/handlers.pynow exportsACTION_HANDLERS.chassis/actions.pyconsumes it instead of maintaining its own dict, so the legacy and SDK chassis cannot expose different action sets.tests/contracts/test_chassis_parity.pyfails if a future edit reintroduces a hand-maintained list in either one.2. W4-04 Postgres audit pool
The
ComplianceEngineconsumer side is already onmain; only the wiring was missing.GraphLifecycle.startup()creates anasyncpgpool whenPOSTGRES_DSNis set and passes it throughinit_dependencies()toEngineState.db_pool.shutdown()closes it.This is a soft dependency: an unset or unreachable DSN leaves
db_poolasNoneand logs a warning rather than blocking startup — the same graceful-degrade pattern as the existing Neo4j connection.EngineState.health_check()reportsdb_pool_present.docker-compose.ymlgains apostgres:16-alpineservice seeded fromengine/packet/packet_store.sql.3. Chassis consolidation
Deletes
chassis/app.py(371 lines) andchassis/auth/app.py(208 lines), which duplicatedchassis/chassis_app.py. All imports and launch sites repointed. This clears thechassis/auth/app.pyitem inmemory-bank/tech-debt.md.4. Hermetic settings under pytest
engine/config/settings.pysetsenv_file=Nonewhenpytestis insys.modules, so a developer's local.envcannot leak opt-in feature flags into the test suite.Verification
mypy engine/reports one error (engine/inference_rule_registry.py:462syntax) that reproduces on cleanorigin/main— pre-existing, not introduced here.Deliberately out of scope
main's schema v1 so this does not collide with the schema v2 stamp in feat(meta): config-driven L9_META pipeline, schema v2, full-repo stamp #147.chassis/app.py(docs/contracts/api/openapi.yaml,DEPENDENCY_INJECTION.md,config/env-contract.yaml) are owned by docs(contracts): align the YAML registry and the prose docs into one registry #146 — left untouched to avoid conflicts.tools/l9_meta_injector.pystill lists the two deleted files in its registry; feat(meta): config-driven L9_META pipeline, schema v2, full-repo stamp #147 replaces that module wholesale.gitinstall fromfix/docker-git-deps-and-proprietary-licenseare not included.Made with Cursor