feat(rooms): open-room lane — P7 runtime + schema extensions + catalog stage + A2UI consumer - #2173
Conversation
… stage + checklist consolidation Mavis open-room lane (mvs_09c9b116c675418b9d8b1a48b10867dc, 2026-07-20). - schema: add 'exchange' to room_type.enum, 'browser' to panels[].kind, free-form 'meta' object at root (all provisional, pending operator signoff per AGNOTE4482 §1348) - manifest: tokenism.room.exchange.skill_bindings now declare all required fields (skill_id, room_id, context.sources, outputs, guardrails) - catalog: schema_version 1.1.0 → 1.2.0, all 9 rooms now have current_stage (3 live / 5 rehearsal / 1 archive) + stage_source + stage_verified_at - docs: canonical CHIT activation checklist consolidated to ROOM_MANIFEST_CONTRACT.md; ROOMS_ON_A_STAGE.md + AGNOTE4482.md now link out - docs: 'suits' → 'overlays' in room context to break collision with pmoves/configs/model-suits/ - spec: pmoves/docs/specs/room-manifest-schema-extensions-2026-07-20.md (proposal for operator review) - spec: pmoves/docs/specs/p7-service-spec-2026-07-20.md (P7 FastAPI runtime design — code in feat/p7-runtime-slice worktree, blocked on schema signoff) validate_room_manifests.py: 7 OK / 2 FAILED → 9 OK / 0 FAILED. AGNOTE4482PHI.t1.md: open-room lane CLAIM 2026-07-20T17:59:39Z.
… APPROVED; proceeding to code
…chestrator to approved spec
Mavis open-room lane, Option A (mvs_09c9b116c675418b9d8b1a48b10867dc, 2026-07-20).
Aligns the pre-existing p7-room-orchestrator (planned/active/paused/ended/archived,
CHIT on chit.capability/handler/integration/trigger, NATS p7.room.session.*.v1,
port 8092, no compose entry) to the operator-approved spec at
pmoves/docs/specs/p7-service-spec-2026-07-20.md (rehearsal/live/review/archive,
CHIT on meta.chit.card_id, NATS p7.nats.* + room.session.updated.v1, port 8120,
atomic catalog writeback, schema-validated manifest loads).
- Split single-file app.py into:
- config.py Pydantic settings (P7_*-prefixed env vars)
- catalog.py CatalogLoader: load/cache/reload, atomic writeback of current_stage,
manifest validation against room.manifest.v1.schema.json
with skill.binding.v1.schema.json \ resolution (via referencing.Registry)
- nats_pub.py NATSPublisher: HMAC-SHA256 sign + publish to p7.nats.launch,
p7.nats.session, room.session.updated.v1, pmoves.config.rooms.reloaded.v1;
log-only fallback when NATS unreachable; unsigned-local status when
P7_SERVICE_CARD_ID/P7_SIGNING_KEY are unset
- transition.py TransitionEngine: state machine + 7-item CHIT activation checklist
(canonical from ROOM_MANIFEST_CONTRACT.md)
- main.py FastAPI app: /healthz, /api/p7/rooms, /api/p7/rooms/{id},
/api/p7/rooms/{id}/transition, /api/p7/reload; exception handlers
for ManifestError/InvalidTransitionError/ChecklistError/CatalogError
- README.md operator + developer quickstart
- tests/ 46 pytest cases across test_catalog, test_transition, test_nats_pub,
test_api; hermetic conftest.py builds a temp pmoves/ subtree
- Dockerfile: port 8120, healthcheck, multi-file COPY
- requirements.txt: + jsonschema, + PyYAML, + pydantic-settings
- docker-compose.yml: p7 service on profile=p7 (port 8120, NATS dep,
rooms+schemas+sigcards+agent_registry volumes)
- Makefile: up-p7, p7-health, smoke-p7 targets
- .claude/commands/p7/room.md: rewritten for new state machine, port 8120,
transition endpoint, NATS subjects
Validation:
pytest tests/ 46/46 passing
validate_room_manifests.py 9/9 OK
python -c 'import main' OK (FastAPI app imports clean)
AGNOTE4482PHI.t1.md: open-room lane CLAIM 2026-07-20T17:59:39Z +
signoff-received 2026-07-20T19:45:00Z + align-plan 2026-07-20T19:50:00Z.
Mavis open-room lane, follow-on (mvs_09c9b116c675418b9d8b1a48b10867dc, 2026-07-20).
Two follow-ons from the P7 runtime slice:
- pmoves/services/a2ui-nats-bridge/bridge.py: per-client subscriptions to
room.session.updated.v1 + pmoves.config.rooms.reloaded.v1 inside the
/ws/client WebSocket handler. Payloads wrapped in {room: p7-rooms, subject,
data} envelopes, mirroring the existing geometry pattern. New Prometheus
counters: a2ui_room_session_events_forwarded_total,
a2ui_room_config_reloaded_events_forwarded_total. All subscriptions get a
clean unsubscribe in the finally block. Subject + room names are
env-overridable (P7_ROOM_SESSION_UPDATED_SUBJECT, P7_CONFIG_RELOADED_SUBJECT,
P7_WS_ROOM).
- .claude/skills/p7-stage/SKILL.md: new operator + agent skill with three
operations - claim (read current_stage via GET /api/p7/rooms[/id]),
transition (POST with body {target_stage, reason, requester}, gates on
the 7-item CHIT checklist for rehearsal→live), observe (subscribe to
room.session.updated.v1 / pmoves.config.rooms.reloaded.v1 via nats sub
or via the A2UI WebSocket). Includes a common-pitfalls table and full
cross-references.
Validation: ast.parse on bridge.py clean; P7 pytest suite still 46/46
(no regression). Code-only changes; no schema/catalog mutations.
AGNOTE4482PHI.t1.md: open-room lane CLAIM 2026-07-20T17:59:39Z +
signoff-received 2026-07-20T19:45:00Z + align-plan 2026-07-20T19:50:00Z +
RELEASE 2026-07-20T20:00:00Z + consumer+skill 2026-07-20T20:10:00Z.
…sh command Mavis open-room lane, follow-on (mvs_09c9b116c675418b9d8b1a48b10867dc, 2026-07-20). Refactor + new tests for A2UI bridge P7 envelope handlers + new operator slash command for the P7 lifecycle. - pmoves/services/a2ui-nats-bridge/bridge.py: extracted the two new P7 handlers (room.session.updated.v1 + pmoves.config.rooms.reloaded.v1) from inside the client_websocket closure to module-level forward_room_session_event / forward_config_reloaded_event functions. The per-client closure versions delegate to the module-level so behavior is unchanged. Reason: the existing test pattern (importlib.util) re-registers prometheus_client Counter objects on each import; module-level handlers let us share one bridge instance across test files. - pmoves/services/a2ui-nats-bridge/tests/test_signature_gate.py: 7 new test cases (room_session_handler_wraps_in_p7_rooms_envelope, room_session_handler_respects_env_override, room_session_handler_handles_garbage_payload, config_reloaded_handler_wraps_in_p7_rooms_envelope, config_reloaded_handler_handles_garbage_payload, test_subject_constants_match_p7_service_spec, test_p7_ws_room_default). Co-located in this file to share the bridge instance. - .claude/commands/p7/stage.md: new operator slash command, sibling to /p7:room. claim (read), transition (mutate, gated), observe (subscribe). Backed by .claude/skills/p7-stage/SKILL.md; thin wrapper over the P7 service HTTP API + NATS subjects. State-machine reference + common-pitfalls table. Validation: A2UI bridge suite 15/15 passing; P7 service suite 46/46 passing; bridge.py parses clean. AGNOTE4482PHI.t1.md: open-room lane CLAIM 2026-07-20T17:59:39Z + signoff-received + align-plan + RELEASE + consumer+skill + COMPLETE.
Two small fixes to the cherry-picked open-room lane branch: - pmoves/contracts/schemas/room/room.manifest.v1.schema.json: 'stage' was a required root field (inherited from origin/main). The open-room lane spec (and the catalog schema v1.2.0 split) makes current_stage live in the catalog row, not the per-room manifest. The manifest's 'stage' field is removed from required to match the spec. - pmoves/services/p7-room-orchestrator/tests/test_api.py: add importlib.reload(main) to the client fixture so the SETTINGS singleton picks up P7_PMOVES_ROOT etc. set by hermetic_settings. Without the reload, SETTINGS is frozen at module import time and the catalog isn't found at the hermetic temp path. - Remove the pre-Option-A test_app.py (tested the deleted single-file app.py). The new test_api.py covers the FastAPI surface. Validation: validate_room_manifests.py 9/9 OK, P7 suite 46/46, A2UI bridge suite 16/16.
Docker Hardening ValidationHardening Validation ReportValidated: Tue Jul 21 00:59:59 UTC 2026Services CheckedPMOVES.AI Docker Hardening Validation[INFO] Checking: pmoves/docker-compose.hardened.yml [INFO] Validating: hi-rag-gateway-v2 [INFO] Validating: extract-worker [INFO] Validating: langextract [INFO] Validating: presign [INFO] Validating: render-webhook [INFO] Validating: retrieval-eval [INFO] Validating: pdf-ingest [INFO] Validating: jellyfin-bridge [INFO] Validating: invidious-companion-proxy [INFO] Validating: ffmpeg-whisper [INFO] Validating: media-video [INFO] Validating: media-audio [INFO] Validating: hi-rag-gateway-v2-gpu [INFO] Validating: hi-rag-gateway-gpu [INFO] Validating: deepresearch [INFO] Validating: supaserch [INFO] Validating: publisher-discord [INFO] Validating: mesh-agent [INFO] Validating: nats-echo-req [INFO] Validating: nats-echo-res [INFO] Validating: comfy-watcher [INFO] Validating: grayjay-plugin-host [INFO] Validating: agent-zero [INFO] Validating: p7-room-orchestrator [INFO] Validating: archon [INFO] Validating: channel-monitor [INFO] Validating: pmoves-yt [INFO] Validating: notebook-sync [INFO] Validating: supabase_service_role_key [INFO] Validating: supabase_jwt_secret [INFO] Validating: p7_control_token ====================================== |
🔏 CHIT-aware change — control-body routingThis PR touches a CHIT-aware service (ports 8086/8087/8103/8106/8113/9224 surface). Before a Control-Body ACK:
Advisory routing only — the blocking contract check is CHIT Contract. |
|
Warning Review limit reached
Next review available in: 54 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (17)
📝 WalkthroughWalkthroughIntroduces a P7 room-aware stage manager on port 8120 with lifecycle transitions, CHIT gating, catalog persistence, NATS events, A2UI forwarding, updated room contracts, operator commands, Compose/Make integration, and hermetic test coverage. ChangesP7 Room Lifecycle
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant P7FastAPI
participant TransitionEngine
participant CatalogLoader
participant NATSPublisher
participant A2UIBridge
participant WebSocketClient
Operator->>P7FastAPI: Request room transition
P7FastAPI->>TransitionEngine: Validate and execute transition
TransitionEngine->>CatalogLoader: Read manifest and current stage
TransitionEngine->>CatalogLoader: Persist updated current_stage
TransitionEngine->>NATSPublisher: Publish room.session.updated.v1
A2UIBridge->>WebSocketClient: Forward room envelope
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96b4604276
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pmoves/docs/AGENTS/AGNOTE4482.md (1)
1256-1265: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRemove the stale duplicate CHIT checklist.
The historical note says
ROOM_MANIFEST_CONTRACT.mdis the single source of truth, but this file still contains a full copy—and it has 8 items while the P7 docs state 7. Keep only the historical note and canonical link, or clearly archive the checklist so operators cannot follow conflicting gates.As per path instructions, the CHIT activation checklist must have a single source of truth in
ROOM_MANIFEST_CONTRACT.md.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pmoves/docs/AGENTS/AGNOTE4482.md` around lines 1256 - 1265, Remove or clearly archive the duplicated CHIT activation checklist beneath the historical note in AGNOTE4482.md, leaving only the note and canonical reference to ROOM_MANIFEST_CONTRACT.md. Ensure operators cannot mistake this file for an actionable checklist and preserve ROOM_MANIFEST_CONTRACT.md as the sole source of truth.Source: Path instructions
🧹 Nitpick comments (3)
pmoves/services/p7-room-orchestrator/main.py (1)
129-134: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFragile substring match to pick HTTP status for
ManifestError.
404 if "not in catalog" in str(exc) else 422couples the HTTP contract tocatalog.py's exact error-message wording. Any future rewording of those messages silently changes the API's status codes. Prefer a structured attribute (e.g.ManifestError(msg, status_code=404), mirroringTransitionError's pattern) instead of string matching.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pmoves/services/p7-room-orchestrator/main.py` around lines 129 - 134, Update ManifestError and _manifest_error_handler to use a structured status_code attribute instead of checking for the “not in catalog” substring. Assign the appropriate status code at each ManifestError construction site, defaulting to 422 where applicable, and return exc.status_code while preserving the existing error response content.pmoves/services/a2ui-nats-bridge/bridge.py (1)
162-165: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStale docstring reference to a nonexistent test file.
Docstring says tests live in
tests/test_room_envelope.py, but per this same layer they actually live intests/test_signature_gate.py(as explicitly noted in that file's own comment). Update the reference to avoid sending future readers to a file that doesn't exist.📝 Proposed fix
- Module-level (not a closure) so unit tests in - ``tests/test_room_envelope.py`` can call it directly. Named + Module-level (not a closure) so unit tests in + ``tests/test_signature_gate.py`` can call it directly. Named ``forward_*`` (not ``*_handler``) to avoid shadowing the per-client closure in ``client_websocket``.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pmoves/services/a2ui-nats-bridge/bridge.py` around lines 162 - 165, Update the module-level helper docstring near the forward_* functions to reference tests/test_signature_gate.py instead of the nonexistent tests/test_room_envelope.py, leaving the surrounding explanation unchanged.pmoves/services/p7-room-orchestrator/tests/test_nats_pub.py (1)
65-74: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssertion-free test provides no real coverage.
test_publish_room_updated_shapeinvokespublish_room_updatedbut asserts nothing — it only proves the call doesn't raise.test_high_level_helpers_use_correct_subjectsright below already verifies the same method's subject/payload shape via monkeypatchingp.publish. Consider dropping this test (or merging its intent into the one below) so the name doesn't imply verification it doesn't perform.🧪 Suggested cleanup
-@pytest.mark.asyncio -async def test_publish_room_updated_shape(): - p = NATSPublisher(nats_url="nats://127.0.0.1:1", service_card_id="", signing_key="") - await p.publish_room_updated( - room_id="x.room", previous_stage="rehearsal", new_stage="live", - reason="promotion", requester="DARKXSIDE", - ) - # We can't inspect the publisher's internal call list without monkey-patching - # but the function should be side-effect-free in log-only mode.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pmoves/services/p7-room-orchestrator/tests/test_nats_pub.py` around lines 65 - 74, Remove the assertion-free test_publish_room_updated_shape, since test_high_level_helpers_use_correct_subjects already verifies publish_room_updated’s subject and payload through the mocked p.publish call. Do not add redundant coverage; retain the existing meaningful subject/payload assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/commands/p7/stage.md:
- Line 133: Fix the malformed troubleshooting table rows so each has exactly
three pipe-delimited cells: symptom, cause, and fix. In
.claude/commands/p7/stage.md lines 133-133, keep the NATS subject symptom and
remediation together in the fix cell; apply the same three-column correction to
the affected rows in .claude/skills/p7-stage/SKILL.md lines 149-156.
In @.claude/skills/p7-stage/SKILL.md:
- Around line 89-99: Update the transition procedure’s publish outcome in the
POST transition step to state that NATS publishes use either a signed or
unsigned-local envelope, depending on signing configuration, instead of claiming
every transition is signed.
In `@pmoves/docker-compose.yml`:
- Around line 2874-2887: Update the P7 service configuration to ensure its
FastAPI server binds to 0.0.0.0 by adding the host setting expected by the
application, such as P7_HOST. Add support for P7_SIGNING_KEY_FILE using the
repository’s established secret-file loading convention, while preserving
P7_SIGNING_KEY as the existing fallback.
In `@pmoves/docs/ROOMS_ON_A_STAGE.md`:
- Around line 62-69: Update the adjacent P7 deployment contract in the
documentation to reference service port 8120 and the current p7.nats.*
control-plane subjects, removing legacy port 8122 and p7.room.*.v1 references.
Keep the operational claims aligned with the current runbooks and smoke
evidence.
In `@pmoves/docs/specs/p7-service-spec-2026-07-20.md`:
- Around line 86-102: Update the P7 service layout, Docker Compose snippet, and
Makefile targets in the spec to match the implemented p7-room-orchestrator
service and evidence from runbooks/smokes. Replace p7 paths with
services/p7-room-orchestrator, place smoke tests under
services/p7-room-orchestrator/tests, and use the P7_-prefixed environment
variables, including P7_NATS_URL, P7_ROOM_CATALOG_PATH, P7_ROOMS_DIR, and
P7_CHIT_REQUIRE_SIGNATURE, consistently throughout the affected sections.
In `@pmoves/docs/specs/room-manifest-schema-extensions-2026-07-20.md`:
- Around line 3-8: Make the schema-extension approval status consistent across
all references: in
pmoves/docs/specs/room-manifest-schema-extensions-2026-07-20.md lines 3-8,
establish whether operator signoff is complete and update the status
accordingly; in .claude/commands/p7/room.md line 118, remove “operator-approved”
or link to approval evidence; in .claude/skills/p7-stage/SKILL.md lines 175-178,
align the cross-reference with that authoritative status; and in
pmoves/docs/AGENTS/AGNOTE4482PHI.t1.md line 1236, retain the prior status only
when explicitly labeled historical.
In `@pmoves/services/p7-room-orchestrator/catalog.py`:
- Around line 74-91: Offload synchronous catalog disk I/O from the asyncio event
loop by wrapping calls to Catalog methods reload(), get_manifest(), and
_write_catalog_atomic() with asyncio.to_thread (or the existing executor
pattern) at their async call sites in TransitionEngine.transition() and the
get_room, list_rooms, and reload_catalog handlers in main.py. Preserve locking,
validation, return values, and exception behavior while ensuring reads, writes,
and fsync execute in a worker thread.
In `@pmoves/services/p7-room-orchestrator/main.py`:
- Around line 87-95: Replace the direct os.environ read in the PUBLISHER setup
with a required signing-key field on P7Settings, using the existing centralized
environment and *_FILE secret-loading helpers. Document the corresponding
environment variable in config.py, and ensure missing configuration fails loudly
rather than passing an empty key to NATSPublisher.
In `@pmoves/services/p7-room-orchestrator/nats_pub.py`:
- Around line 57-95: Update NATSClient.connect to use self._retry_max and
self._retry_backoff for bounded connection retries with backoff, returning
success immediately when connected and log-only failure only after all attempts
are exhausted. Ensure each failed attempt cleans up any partially created or
stale self._nc client before the next attempt, including mid-stream reconnects,
without changing the existing connected-state contract.
In `@pmoves/services/p7-room-orchestrator/README.md`:
- Around line 89-95: Update the Docker run example to set the P7_NATS_URL
environment variable instead of NATS_URL, preserving the host.docker.internal
endpoint so the service uses the configured NATS address.
- Around line 58-73: The P7 signing configuration documentation and
implementation must support file-based secrets and avoid defaulting to
unsigned-local operation. Update the signing-key configuration around P7's
signing setup to honor P7_SIGNING_KEY_FILE via the shared secret helper, and
change P7_ALLOW_UNSIGNED_LOCAL so it is explicitly enabled only for local
development rather than defaulting to true; keep fail-closed signature
enforcement as the default.
In `@pmoves/services/p7-room-orchestrator/transition.py`:
- Around line 216-230: Update the checklist flow around the card_id validation
so a missing or invalid card_id records items 1-3 as skipped but does not return
early; allow execution to continue through independent items 4-7. Remove the
duplicate self._find_signing_card(card_id) lookup below and ensure
card-dependent validation remains safely skipped when no valid card_id exists.
- Around line 283-312: The configuration sources used by checklist items 6 and 7
are inconsistent with the canonical sidecar config naming. Update the relevant
transition validation symbols, including _settings.resolved("pmoves/env.shared")
and _load_sidecar_env, so they use the documented canonical sidecar.env path, or
explicitly document and consistently support env.shared as its alias; ensure
item 7 validates the same source referenced by ROOM_MANIFEST_CONTRACT.md.
---
Outside diff comments:
In `@pmoves/docs/AGENTS/AGNOTE4482.md`:
- Around line 1256-1265: Remove or clearly archive the duplicated CHIT
activation checklist beneath the historical note in AGNOTE4482.md, leaving only
the note and canonical reference to ROOM_MANIFEST_CONTRACT.md. Ensure operators
cannot mistake this file for an actionable checklist and preserve
ROOM_MANIFEST_CONTRACT.md as the sole source of truth.
---
Nitpick comments:
In `@pmoves/services/a2ui-nats-bridge/bridge.py`:
- Around line 162-165: Update the module-level helper docstring near the
forward_* functions to reference tests/test_signature_gate.py instead of the
nonexistent tests/test_room_envelope.py, leaving the surrounding explanation
unchanged.
In `@pmoves/services/p7-room-orchestrator/main.py`:
- Around line 129-134: Update ManifestError and _manifest_error_handler to use a
structured status_code attribute instead of checking for the “not in catalog”
substring. Assign the appropriate status code at each ManifestError construction
site, defaulting to 422 where applicable, and return exc.status_code while
preserving the existing error response content.
In `@pmoves/services/p7-room-orchestrator/tests/test_nats_pub.py`:
- Around line 65-74: Remove the assertion-free test_publish_room_updated_shape,
since test_high_level_helpers_use_correct_subjects already verifies
publish_room_updated’s subject and payload through the mocked p.publish call. Do
not add redundant coverage; retain the existing meaningful subject/payload
assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5c48a34d-dae6-45f3-bb66-89cff7623b07
📒 Files selected for processing (32)
.claude/commands/p7/room.md.claude/commands/p7/stage.md.claude/skills/p7-stage/SKILL.mdpmoves/Makefilepmoves/config/rooms/catalog.jsonpmoves/config/rooms/tokenism.room.exchange.jsonpmoves/contracts/schemas/room/room.manifest.v1.schema.jsonpmoves/docker-compose.ymlpmoves/docs/AGENTS/AGNOTE4482.mdpmoves/docs/AGENTS/AGNOTE4482PHI.t1.mdpmoves/docs/ROOMS_ON_A_STAGE.mdpmoves/docs/ROOM_MANIFEST_CONTRACT.mdpmoves/docs/specs/p7-service-spec-2026-07-20.mdpmoves/docs/specs/room-manifest-schema-extensions-2026-07-20.mdpmoves/services/a2ui-nats-bridge/bridge.pypmoves/services/a2ui-nats-bridge/tests/test_signature_gate.pypmoves/services/p7-room-orchestrator/Dockerfilepmoves/services/p7-room-orchestrator/README.mdpmoves/services/p7-room-orchestrator/__init__.pypmoves/services/p7-room-orchestrator/app.pypmoves/services/p7-room-orchestrator/catalog.pypmoves/services/p7-room-orchestrator/config.pypmoves/services/p7-room-orchestrator/main.pypmoves/services/p7-room-orchestrator/nats_pub.pypmoves/services/p7-room-orchestrator/requirements.txtpmoves/services/p7-room-orchestrator/tests/__init__.pypmoves/services/p7-room-orchestrator/tests/conftest.pypmoves/services/p7-room-orchestrator/tests/test_api.pypmoves/services/p7-room-orchestrator/tests/test_catalog.pypmoves/services/p7-room-orchestrator/tests/test_nats_pub.pypmoves/services/p7-room-orchestrator/tests/test_transition.pypmoves/services/p7-room-orchestrator/transition.py
💤 Files with no reviewable changes (1)
- pmoves/services/p7-room-orchestrator/app.py
Addresses 5 P1 review threads from chatgpt-codex-connector + CodeQL:
- pmoves/services/p7-room-orchestrator/Dockerfile: COPY paths now assume
monorepo-root build context (matching the existing p7-room-orchestrator
compose definitions in docker-compose.yml and docker-compose.agents.yml).
Sets P7_HOST=0.0.0.0 so the container bind is correct.
- pmoves/services/p7-room-orchestrator/main.py: restore bearer auth on
/api/p7/rooms/{id}/transition via P7_CONTROL_TOKEN. Uses the shared
services/common/env.py get_secret helper so P7_CONTROL_TOKEN_FILE works
out of the box. Endpoints return 503 if the token is unconfigured
(fail-closed) or 401 for invalid bearer credentials. Mirrors the
original p7-room-orchestrator contract from origin/main app.py.
- pmoves/services/p7-room-orchestrator/main.py: add a global exception
handler that logs the full exception server-side but returns a generic
'internal_error' message to the client. Closes the CodeQL
'information exposure through an exception' finding.
- pmoves/services/p7-room-orchestrator/transition.py: signing_cards.yaml
parser now handles the canonical LIST shape used in
pmoves/config/signing_identity_cards.yaml. The previous code only
matched the DICT shape and silently failed for every rehearsal→live
transition. Legacy DICT shape still supported for backward compat.
- pmoves/Makefile: restore up-cipher-full and cipher-memory-smoke
targets. The 'feat(p7): align' commit accidentally dropped them when
inserting the P7 targets next to up-cipher. Restoring the existing
AGENT_TRAIL.md workflow.
- pmoves/docker-compose.yml: add P7_HOST=0.0.0.0 and P7_SIGNING_KEY_FILE
env vars to the p7 service. Use the same build-context convention as
the existing p7-room-orchestrator entries (context: ., dockerfile:
services/p7-room-orchestrator/Dockerfile).
- pmoves/services/p7-room-orchestrator/tests/conftest.py: write signing
cards in the canonical LIST shape (matches production) and set
P7_CONTROL_TOKEN=test-control-token so the new auth dependency
passes.
- pmoves/services/p7-room-orchestrator/tests/test_api.py: TestClient
default headers include the bearer token so existing transition tests
pass through the auth gate. Per-test override remains possible for
the 401/503 auth paths.
Validation: validate_room_manifests.py 9/9 OK, P7 pytest 46/46 passing.
- transition.py: items 4-7 now evaluated when card_id is missing (not
short-circuited), so the operator sees the full checklist state in a
single iteration.
- transition.py: rename 'sidecar.env' → 'env.shared' in items 6/7 to
match the canonical filename (per pmoves/env.shared).
- transition.py: _load_sidecar_env now merges process env (wins) over
the file contents so tests + runtime overrides work cleanly.
- main.py: offload CATALOG.reload() and CATALOG.get_manifest() (disk
I/O + JSON schema validation) via asyncio.to_thread so the event
loop isn't blocked during /api/p7/reload and /api/p7/rooms/{id}.
- transition.py: offload CATALOG.get_manifest() and CATALOG.update_stage()
in the transition path.
- main.py: P7_SIGNING_KEY now goes through get_secret() so the
P7_SIGNING_KEY_FILE secret-mount pattern works (mirrors the
P7_CONTROL_TOKEN handling).
- nats_pub.py: implement connect_with_retry() using retry_max_attempts
and retry_backoff_sec with exponential backoff (cap 60s). Lifespan
now calls connect_with_retry() for bounded boot-time retries; the
one-shot connect() is kept for lazy publish-time fallback.
- tests/test_api.py: add FakePublisher.connect_with_retry().
All tests green: 46/46 P7, 16/16 A2UI, 9/9 manifests.
- README.md: signing key FILE support documented (P7_SIGNING_KEY_FILE), Docker example uses P7_NATS_URL not NATS_URL, new 'Secret-aware env' section covers the *_FILE pattern with Docker + local-dev examples. - ROOMS_ON_A_STAGE.md: P7 deployment contract updated to reflect FastAPI service at pmoves/services/p7-room-orchestrator on port 8120 with the actual HTTP API + NATS subject table + signed/unsigned-local envelope contract (was 'port 8122' + 'p7.room.*.v1', both stale). - p7-service-spec-2026-07-20.md: status APPROVED, branch feat/open-room-lane, file layout corrected to p7-room-orchestrator dir, docker-compose snippet uses the real P7_*-prefixed env vars (was NATS_URL / PMOVES_ROOMS_CATALOG / PMOVES_ROOMS_DIR / CHIT_REQUIRE_SIGNATURE), open questions replaced with resolved decisions, signoff checklist marked all-checked. - room-manifest-schema-extensions-2026-07-20.md: status APPROVED, branch feat/open-room-lane, signoff checklist marked all-checked with the CLAIM + RELEASE references in AGNOTE4482PHI.t1.md. - .claude/commands/p7/stage.md: 'Common pitfalls' table row for NATS no-traffic fixed (the inline pipe in 'curl ... | jq ...' was being parsed as a column separator, breaking the 2-column table). - .claude/skills/p7-stage/SKILL.md: 'Signed NATS publish' wording relaxed to make the signed/unsigned-local distinction explicit (was misleading — implies all publishes are signed). All tests still green: 46/46 P7, 16/16 A2UI, 9/9 manifests.
Records the 3 review-iter commits (a3813dc / e1f6d15 / 249a38b) that addressed all 18 review threads on PR #2173. This AGNOTE entry is the trail-of-record for the review-iter-1 push. The intermediate entries that lived on the operator's auto worktree (operator signoff, align-plan, runtime slice RELEASE, A2UI consumer+skill) are summarized inline so the trail on this branch is self-contained.
… reconnect - get_room now logs the ManifestError (which embeds internal filesystem paths) and returns a generic manifest_error to the caller, closing the CodeQL py/stack-trace-exposure alert at main.py. - NATSPublisher.connect() drains any stale self._nc before creating a new client, so a mid-stream publish failure (_connected=False) no longer leaks the previous connection on reconnect (CodeRabbit Major). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The nats_connected check cell contained an unescaped ` | ` inside its code span, which GFM parses as a 4th column and breaks the 3-column table render (CodeRabbit Minor / table-column-count). stage.md's 2-col table was already clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Conflicts resolved: - tokenism.room.exchange.json: keep BOTH the branch's guardrails block (schema-completion work) and main's enabled:true (the W2 flip, #2169, live-verified — must not regress). - AGNOTE4482PHI.t1.md: append-collision, keep-both (branch's GRAPHITI mark + main's sections). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Docker Hardening ValidationHardening Validation ReportValidated: Sat Jul 25 12:23:34 UTC 2026Services CheckedPMOVES.AI Docker Hardening Validation[INFO] Checking: pmoves/docker-compose.hardened.yml [INFO] Validating: hi-rag-gateway-v2 [INFO] Validating: extract-worker [INFO] Validating: langextract [INFO] Validating: presign [INFO] Validating: render-webhook [INFO] Validating: retrieval-eval [INFO] Validating: pdf-ingest [INFO] Validating: jellyfin-bridge [INFO] Validating: invidious-companion-proxy [INFO] Validating: ffmpeg-whisper [INFO] Validating: media-video [INFO] Validating: media-audio [INFO] Validating: hi-rag-gateway-v2-gpu [INFO] Validating: hi-rag-gateway-gpu [INFO] Validating: deepresearch [INFO] Validating: supaserch [INFO] Validating: publisher-discord [INFO] Validating: mesh-agent [INFO] Validating: nats-echo-req [INFO] Validating: nats-echo-res [INFO] Validating: comfy-watcher [INFO] Validating: grayjay-plugin-host [INFO] Validating: agent-zero [INFO] Validating: p7-room-orchestrator [INFO] Validating: archon [INFO] Validating: channel-monitor [INFO] Validating: pmoves-yt [INFO] Validating: notebook-sync [INFO] Validating: supabase_service_role_key [INFO] Validating: supabase_jwt_secret [INFO] Validating: p7_control_token ====================================== |
…rofiles, broker auth (#2223) Landed #2173 and stood P7 up on 5090; five defects surfaced at first real boot, each verified fixed live (healthz ok, rooms_loaded=9, nats_connected=true, smoke 46/46): 1. main.py crashed in-container: Path(__file__).parents[3] raises IndexError at /app/main.py before the ImportError fallback can engage (restart loop). Now walks upward for services/common — and APPENDS to sys.path: services/common ships its own config.py which would otherwise shadow the service's config module. (The old fixed index accidentally never resolved in-repo either, so the shared env helper was silently unused everywhere; tests now import the real one.) 2. Fallback get_secret ignored the <KEY>_FILE convention — compose delivers P7_SIGNING_KEY_FILE, which would silently never load in-container. 3. up-p7 used --profile agents, activating the legacy p7-room-orchestrator whose supabase-kong dependency is profile-gated — compose fails the whole project as 'depends on undefined service'. New p7 needs only --profile p7. 4. Legacy service's kong dependency marked required:false so ANY --profile agents invocation resolves again without supabase-local. 5. P7_NATS_URL lacked broker credentials — P7 booted log-only (nats_connected:false, no signed lifecycle events). Also: p7-health now retries (up-p7 chains into it before the container finishes booting) and smoke-p7 runs pytest from the service dir (flat-module imports). Co-authored-by: Shaela Bello <slbello@uncg.edu> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Summary
Closes the open-room lane: a single PR that takes the rooms-on-a-stage model from "declarative doc + stale prototype" to "working service + aligned docs + A2UI consumer + operator skill + slash commands." P7 (the room-aware stage manager) goes from a concept with 3 inconsistent definitions (rooms doc / approved spec / pre-existing
p7-room-orchestratorwith its own vocabulary) to one canonical implementation.The work was originally in a
feat-auto-*worktree with unrelated changes piled on; this PR cherry-picks just the 8 open-room lane commits onto a cleanfeat/open-room-lanebranch offmain.What's in this PR
1. Schema + catalog + checklist (first slice,
feat(rooms): ...)pmoves/contracts/schemas/room/room.manifest.v1.schema.json(additive, provisional per AGNOTE4482 §1348):room_typeenum gains"exchange"(for the ToKenism floor)panels[].kindenum gains"browser"(for webview surfaces)metaobject added (free-form,additionalProperties: true) for inline CHIT signing-card references; the spec doc explicitly notes this is the interim path until a first-classchit_cardfield landscatalog.jsonbumped toschema_version 1.2.0; every room row now carriescurrent_stage(3 live / 5 rehearsal / 1 archive),stage_source,stage_verified_at— closes P7's fast-lookup gappmoves/docs/ROOM_MANIFEST_CONTRACT.mdas the single canonical source (7 items,rehearsal → livevocabulary corrected from the historicalplanned → active). The other two copies (ROOMS_ON_A_STAGE.md,AGNOTE4482.md) now link out — historical audit trail inAGNOTE4482.mdpreserved with a forward-pointer notesuits→overlaysrename in both room docs to break the term collision withpmoves/configs/model-suits/(per-model YAML profiles)pmoves/config/rooms/tokenism.room.exchange.jsonskill_bindings now have all required fields (skill_id,room_id,context.sources,outputs,guardrails)2. P7 runtime service (Option A — aligned the pre-existing
p7-room-orchestrator,feat(p7): align ...)The pre-existing 333-line
pmoves/services/p7-room-orchestrator/app.pywas an early prototype with a different vocabulary (planned/active/paused/ended/archived,chit.capability/handler/integration/trigger, NATSp7.room.session.*.v1, port 8092, no compose entry). Aligned it to the approved spec:app.pyinto 5 modules:config.py— Pydantic settings (P7_*-prefixed env vars)catalog.py—CatalogLoaderwith atomiccurrent_stagewriteback (write-temp + rename) and schema-validated manifest loads (withskill.binding.v1.schema.json$refresolution viareferencing.Registry)nats_pub.py—NATSPublisherwith HMAC-SHA256 signing + log-only fallback when NATS is unreachabletransition.py—TransitionEnginewith the canonical 7-item CHIT activation checklistmain.py— FastAPI app (port 8120) with/healthz,/api/p7/rooms,/api/p7/rooms/{id},/api/p7/rooms/{id}/transition,/api/p7/reloadrehearsal → live(gated) →review→archive; same → same is idempotent no-op;archiveis terminalp7.nats.launch,p7.nats.session,room.session.updated.v1,pmoves.config.rooms.reloaded.v1— all with signed envelopes (chit{kid, ts, status, signature})p7service onprofile=p7(port 8120, NATS dep, rooms+schemas+sigcards+agent_registry volumes)up-p7,p7-health,smoke-p73. A2UI consumer (
feat(p7): A2UI consumer ...+feat(p7): A2UI handler refactor ...)pmoves/services/a2ui-nats-bridge/bridge.py: per-client subscriptions in the/ws/clientWebSocket handler forroom.session.updated.v1andpmoves.config.rooms.reloaded.v1; payloads wrapped in{room: "p7-rooms", subject, data}envelopes (mirrors the geometry pattern)forward_room_session_event,forward_config_reloaded_event) so unit tests can call them directlya2ui_room_session_events_forwarded_total,a2ui_room_config_reloaded_events_forwarded_totalunsubscribe()in thefinallyblock4. Operator + agent skills / commands
.claude/skills/p7-stage/SKILL.md: new procedural skill with three operations (claim,transition,observe), common-pitfalls table, full cross-references.claude/commands/p7/stage.md: new operator slash command, sibling to/p7:room— thin wrapper over the P7 service HTTP API + NATS subjects.claude/commands/p7/room.md: rewritten to match the aligned state machine, port 8120, transition endpoint, NATS subjects5. Spec docs
pmoves/docs/specs/room-manifest-schema-extensions-2026-07-20.md(operator-approved)pmoves/docs/specs/p7-service-spec-2026-07-20.md(approved, design source-of-truth for the runtime)Validation
python pmoves/scripts/validate_room_manifests.pypytest pmoves/services/p7-room-orchestrator/tests/pytest pmoves/services/a2ui-nats-bridge/tests/python -c 'import main'(P7 service)ast.parseonbridge.pymake -C pmoves smoke-p7Operator gate
Mavis::OPEN-ROOM-LANE-SIGNOFF-RECEIVEDentry)room-manifest-schema-extensions-2026-07-20.md) lists the operator signoff checklist — already satisfiedp7-service-spec-2026-07-20.md) lists 5 open questions; the RELEASE entry documents the default resolutions taken (port 8120, P7's own CHIT card, catalog writeback directly, A2UI additive, worktree co-located)Test plan for reviewer
Risk + rollback
additionalProperties: trueon the newmetafield, new enum values). Existing 9 manifests validate without modification. Backward compatible.pdf-ingest+publisher-discord) → 8120 (chosen as the first free port in the agent tier). Confirmed free.p7-room-orchestrator: this PR rewrites it. Theapp.pydeletion is the only "breaking" change; consumers of the oldp7.room.session.*.v1NATS subjects and/api/v1/rooms/{id}/start|pause|resume|endendpoints must migrate to the new shape. Per the AGNOTE, no in-fleet consumers were identified.chit.status: "unsigned-local"if P7_SERVICE_CARD_ID is unset. CHIT checklist item 4 still passes becauseP7_ALLOW_UNSIGNED_LOCAL=trueis the operator-acknowledged default. Production rollout should setP7_SERVICE_CARD_ID+P7_SIGNING_KEY.feat(rooms): open-room-lane first slice) is independently revertable — the catalog and schema changes are non-destructive (catalogschema_versionbump is the only outward-facing signal).Follow-ups (NOT in this PR — separately tracked)
litbundle picks up the newp7-roomsenvelope, the renderer can show a "Live" / "Rehearsal" badge on each room card. The bridge is wired; the renderer side is a small follow-up.p7-stageskill is in.claude/skills/; the auto-invocation path can be enabled by Claude Code's skill-discovery (already on by default).chit_cardfirst-class field on the room manifest, deprecating the free-formmetaobject: spec'd in the extension doc §3.3; not implemented.Cross-references
pmoves/docs/AGENTS/AGNOTE4482PHI.t1.md(searchMavis::OPEN-ROOM-LANE— 7 entries: CLAIM + RELEASE + signoff-received + align-plan + consumer+skill + COMPLETE)pmoves/docs/specs/p7-service-spec-2026-07-20.mdpmoves/docs/specs/room-manifest-schema-extensions-2026-07-20.mdpmoves/docs/ROOM_MANIFEST_CONTRACT.md§ CHIT Signing-Card Activation Checklistpmoves/docs/ROOMS_ON_A_STAGE.md.claude/commands/p7/{room,stage}.md.claude/skills/p7-stage/SKILL.mdpmoves/services/a2ui-nats-bridge/bridge.py(function:forward_room_session_event/forward_config_reloaded_event)Commits in this PR
8 commits authored by the same local git identity (PMOVES auto-mode operator), 9 total in the PR (1 fixup for the clean branch).
CHIT trail unsigned-local (no CHIT_PASSPHRASE loaded in the Mavis session). agent_signature:
ACK::Mavis::OPEN-ROOM-LANE-2026-07-20.Summary by CodeRabbit
New Features
Documentation
Bug Fixes